diff --git a/benchmarks/src/main/java/org/opensearch/benchmark/store/remote/filecache/FileCacheBenchmark.java b/benchmarks/src/main/java/org/opensearch/benchmark/store/remote/filecache/FileCacheBenchmark.java index 7cd8c672f45df..a43555393a4d5 100644 --- a/benchmarks/src/main/java/org/opensearch/benchmark/store/remote/filecache/FileCacheBenchmark.java +++ b/benchmarks/src/main/java/org/opensearch/benchmark/store/remote/filecache/FileCacheBenchmark.java @@ -9,8 +9,6 @@ package org.opensearch.benchmark.store.remote.filecache; import org.apache.lucene.store.IndexInput; -import org.opensearch.core.common.breaker.CircuitBreaker; -import org.opensearch.core.common.breaker.NoopCircuitBreaker; import org.opensearch.index.store.remote.filecache.CachedIndexInput; import org.opensearch.index.store.remote.filecache.FileCache; import org.opensearch.index.store.remote.filecache.FileCacheFactory; @@ -93,8 +91,7 @@ public static class CacheParameters { public void setup() { fileCache = FileCacheFactory.createConcurrentLRUFileCache( (long) maximumNumberOfEntries * INDEX_INPUT.length(), - concurrencyLevel, - new NoopCircuitBreaker(CircuitBreaker.REQUEST) + concurrencyLevel ); for (long i = 0; i < maximumNumberOfEntries; i++) { final Path key = Paths.get(Long.toString(i)); diff --git a/server/src/main/java/org/opensearch/index/store/remote/filecache/FileCache.java b/server/src/main/java/org/opensearch/index/store/remote/filecache/FileCache.java index a1b9e0785f2e0..2bd124543ed4e 100644 --- a/server/src/main/java/org/opensearch/index/store/remote/filecache/FileCache.java +++ b/server/src/main/java/org/opensearch/index/store/remote/filecache/FileCache.java @@ -13,8 +13,6 @@ import org.apache.lucene.store.IndexInput; import org.opensearch.common.SetOnce; import org.opensearch.common.annotation.PublicApi; -import org.opensearch.core.common.breaker.CircuitBreaker; -import org.opensearch.core.common.breaker.CircuitBreakingException; import org.opensearch.index.store.remote.filecache.AggregateFileCacheStats.FileCacheStatsType; import org.opensearch.index.store.remote.utils.cache.RefCountedCache; import org.opensearch.index.store.remote.utils.cache.SegmentedCache; @@ -61,11 +59,8 @@ public class FileCache implements RefCountedCache { private static final Logger logger = LogManager.getLogger(FileCache.class); private final SegmentedCache theCache; - private final CircuitBreaker circuitBreaker; - - public FileCache(SegmentedCache cache, CircuitBreaker circuitBreaker) { + public FileCache(SegmentedCache cache) { this.theCache = cache; - this.circuitBreaker = circuitBreaker; } public long capacity() { @@ -74,7 +69,6 @@ public long capacity() { @Override public CachedIndexInput put(Path filePath, CachedIndexInput indexInput) { - checkParentBreaker(); CachedIndexInput cachedIndexInput = theCache.put(filePath, indexInput); return cachedIndexInput; } @@ -84,7 +78,6 @@ public CachedIndexInput compute( Path key, BiFunction remappingFunction ) { - checkParentBreaker(); CachedIndexInput cachedIndexInput = theCache.compute(key, remappingFunction); return cachedIndexInput; } @@ -204,22 +197,6 @@ public void closeIndexInputReferences() { theCache.closeIndexInputReferences(); } - /** - * Ensures that the PARENT breaker is not tripped when an entry is added to the cache - */ - private void checkParentBreaker() { - try { - circuitBreaker.addEstimateBytesAndMaybeBreak(0, "filecache_entry"); - } catch (CircuitBreakingException ex) { - throw new CircuitBreakingException( - "Unable to create file cache entries", - ex.getBytesWanted(), - ex.getByteLimit(), - ex.getDurability() - ); - } - } - /** * Restores the file cache instance performing a folder scan of the * {@link org.opensearch.index.store.remote.directory.RemoteSnapshotDirectoryFactory#LOCAL_STORE_LOCATION} diff --git a/server/src/main/java/org/opensearch/index/store/remote/filecache/FileCacheFactory.java b/server/src/main/java/org/opensearch/index/store/remote/filecache/FileCacheFactory.java index 0b7f4df66c1b3..14531e6611286 100644 --- a/server/src/main/java/org/opensearch/index/store/remote/filecache/FileCacheFactory.java +++ b/server/src/main/java/org/opensearch/index/store/remote/filecache/FileCacheFactory.java @@ -9,7 +9,6 @@ package org.opensearch.index.store.remote.filecache; import org.opensearch.common.cache.RemovalReason; -import org.opensearch.core.common.breaker.CircuitBreaker; import org.opensearch.index.store.remote.utils.cache.SegmentedCache; import java.nio.file.Files; @@ -37,12 +36,12 @@ */ public class FileCacheFactory { - public static FileCache createConcurrentLRUFileCache(long capacity, CircuitBreaker circuitBreaker) { - return new FileCache(createDefaultBuilder().capacity(capacity).build(), circuitBreaker); + public static FileCache createConcurrentLRUFileCache(long capacity) { + return new FileCache(createDefaultBuilder().capacity(capacity).build()); } - public static FileCache createConcurrentLRUFileCache(long capacity, int concurrencyLevel, CircuitBreaker circuitBreaker) { - return new FileCache(createDefaultBuilder().capacity(capacity).concurrencyLevel(concurrencyLevel).build(), circuitBreaker); + public static FileCache createConcurrentLRUFileCache(long capacity, int concurrencyLevel) { + return new FileCache(createDefaultBuilder().capacity(capacity).concurrencyLevel(concurrencyLevel).build()); } private static SegmentedCache.Builder createDefaultBuilder() { diff --git a/server/src/main/java/org/opensearch/node/Node.java b/server/src/main/java/org/opensearch/node/Node.java index a88e595ca9a94..91141e6dd52bd 100644 --- a/server/src/main/java/org/opensearch/node/Node.java +++ b/server/src/main/java/org/opensearch/node/Node.java @@ -804,7 +804,7 @@ protected Node(final Environment initialEnvironment, Collection clas settingsModule.getClusterSettings() ); // File cache will be initialized by the node once circuit breakers are in place. - initializeFileCache(settings, circuitBreakerService.getBreaker(CircuitBreaker.REQUEST)); + initializeFileCache(settings); pluginsService.filterPlugins(CircuitBreakerPlugin.class).forEach(plugin -> { CircuitBreaker breaker = circuitBreakerService.getBreaker(plugin.getCircuitBreaker(settings).getName()); @@ -2353,7 +2353,7 @@ DiscoveryNode getNode() { * If the user doesn't configure the cache size, it fails if the node is a data + warm node. * Else it configures the size to 80% of total capacity for a dedicated warm node, if not explicitly defined. */ - private void initializeFileCache(Settings settings, CircuitBreaker circuitBreaker) throws IOException { + private void initializeFileCache(Settings settings) throws IOException { if (DiscoveryNode.isWarmNode(settings) == false) { return; } @@ -2378,7 +2378,7 @@ private void initializeFileCache(Settings settings, CircuitBreaker circuitBreake throw new SettingsException("Cache size must be larger than zero and less than total capacity"); } - this.fileCache = FileCacheFactory.createConcurrentLRUFileCache(capacity, circuitBreaker); + this.fileCache = FileCacheFactory.createConcurrentLRUFileCache(capacity); fileCacheNodePath.fileCacheReservedSize = new ByteSizeValue(this.fileCache.capacity(), ByteSizeUnit.BYTES); ForkJoinPool loadFileCacheThreadpool = new ForkJoinPool( Runtime.getRuntime().availableProcessors(), diff --git a/server/src/test/java/org/opensearch/action/admin/indices/cache/clear/TransportClearIndicesCacheActionTests.java b/server/src/test/java/org/opensearch/action/admin/indices/cache/clear/TransportClearIndicesCacheActionTests.java index 1abd4c06e50e7..57ab30cd8e0f7 100644 --- a/server/src/test/java/org/opensearch/action/admin/indices/cache/clear/TransportClearIndicesCacheActionTests.java +++ b/server/src/test/java/org/opensearch/action/admin/indices/cache/clear/TransportClearIndicesCacheActionTests.java @@ -18,7 +18,6 @@ import org.opensearch.cluster.routing.ShardRouting; import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.settings.Settings; -import org.opensearch.core.common.breaker.NoopCircuitBreaker; import org.opensearch.core.index.shard.ShardId; import org.opensearch.core.rest.RestStatus; import org.opensearch.env.Environment; @@ -85,7 +84,7 @@ public void testOnShardOperation() throws IOException { when(shardRouting.shardId()).thenReturn(shardId); final ShardPath shardPath = ShardPath.loadFileCachePath(nodeEnvironment, shardId); final Path cacheEntryPath = shardPath.getDataPath(); - final FileCache fileCache = FileCacheFactory.createConcurrentLRUFileCache(1024 * 1024, 16, new NoopCircuitBreaker("")); + final FileCache fileCache = FileCacheFactory.createConcurrentLRUFileCache(1024 * 1024, 16); when(testNode.fileCache()).thenReturn(fileCache); when(testNode.getNodeEnvironment()).thenReturn(nodeEnvironment); diff --git a/server/src/test/java/org/opensearch/index/store/CompositeDirectoryTests.java b/server/src/test/java/org/opensearch/index/store/CompositeDirectoryTests.java index 54e2e8dacd0e5..9cbc2fdd12c97 100644 --- a/server/src/test/java/org/opensearch/index/store/CompositeDirectoryTests.java +++ b/server/src/test/java/org/opensearch/index/store/CompositeDirectoryTests.java @@ -15,8 +15,6 @@ import org.apache.lucene.store.IOContext; import org.apache.lucene.store.IndexInput; import org.apache.lucene.store.IndexOutput; -import org.opensearch.core.common.breaker.CircuitBreaker; -import org.opensearch.core.common.breaker.NoopCircuitBreaker; import org.opensearch.index.store.remote.file.CleanerDaemonThreadLeakFilter; import org.opensearch.index.store.remote.file.OnDemandBlockSnapshotIndexInput; import org.opensearch.index.store.remote.filecache.CachedIndexInput; @@ -65,7 +63,7 @@ public void setup() throws IOException { remoteSegmentStoreDirectory.init(); localDirectory = FSDirectory.open(createTempDir()); removeExtraFSFiles(); - fileCache = FileCacheFactory.createConcurrentLRUFileCache(FILE_CACHE_CAPACITY, new NoopCircuitBreaker(CircuitBreaker.REQUEST)); + fileCache = FileCacheFactory.createConcurrentLRUFileCache(FILE_CACHE_CAPACITY); compositeDirectory = new CompositeDirectory(localDirectory, remoteSegmentStoreDirectory, fileCache, threadPool); addFilesToDirectory(LOCAL_FILES); } diff --git a/server/src/test/java/org/opensearch/index/store/DefaultCompositeDirectoryFactoryTests.java b/server/src/test/java/org/opensearch/index/store/DefaultCompositeDirectoryFactoryTests.java index bb7368d64f38e..104ba38c7c7b2 100644 --- a/server/src/test/java/org/opensearch/index/store/DefaultCompositeDirectoryFactoryTests.java +++ b/server/src/test/java/org/opensearch/index/store/DefaultCompositeDirectoryFactoryTests.java @@ -11,8 +11,6 @@ import org.apache.lucene.store.Directory; import org.apache.lucene.store.FSDirectory; import org.opensearch.common.settings.Settings; -import org.opensearch.core.common.breaker.CircuitBreaker; -import org.opensearch.core.common.breaker.NoopCircuitBreaker; import org.opensearch.core.index.shard.ShardId; import org.opensearch.index.IndexSettings; import org.opensearch.index.shard.ShardPath; @@ -45,7 +43,7 @@ public void setup() throws IOException { shardPath = new ShardPath(false, tempDir, tempDir, new ShardId(indexSettings.getIndex(), 0)); localDirectoryFactory = mock(IndexStorePlugin.DirectoryFactory.class); localDirectory = FSDirectory.open(createTempDir()); - fileCache = FileCacheFactory.createConcurrentLRUFileCache(10000, new NoopCircuitBreaker(CircuitBreaker.REQUEST)); + fileCache = FileCacheFactory.createConcurrentLRUFileCache(10000); when(localDirectoryFactory.newDirectory(indexSettings, shardPath)).thenReturn(localDirectory); setupRemoteSegmentStoreDirectory(); populateMetadata(); diff --git a/server/src/test/java/org/opensearch/index/store/remote/filecache/FileCacheCleanerTests.java b/server/src/test/java/org/opensearch/index/store/remote/filecache/FileCacheCleanerTests.java index 84c8f179947fb..a6591a6960433 100644 --- a/server/src/test/java/org/opensearch/index/store/remote/filecache/FileCacheCleanerTests.java +++ b/server/src/test/java/org/opensearch/index/store/remote/filecache/FileCacheCleanerTests.java @@ -15,8 +15,6 @@ import org.opensearch.cluster.metadata.IndexMetadata; import org.opensearch.cluster.routing.AllocationId; import org.opensearch.common.settings.Settings; -import org.opensearch.core.common.breaker.CircuitBreaker; -import org.opensearch.core.common.breaker.NoopCircuitBreaker; import org.opensearch.core.index.shard.ShardId; import org.opensearch.env.NodeEnvironment; import org.opensearch.gateway.WriteStateException; @@ -77,11 +75,7 @@ public class FileCacheCleanerTests extends OpenSearchTestCase { private static final Logger logger = LogManager.getLogger(FileCache.class); - private final FileCache fileCache = FileCacheFactory.createConcurrentLRUFileCache( - 1024 * 1024, - 1, - new NoopCircuitBreaker(CircuitBreaker.REQUEST) - ); + private final FileCache fileCache = FileCacheFactory.createConcurrentLRUFileCache(1024 * 1024, 1); private final Map files = new HashMap<>(); private NodeEnvironment env; private FileCacheCleaner cleaner; diff --git a/server/src/test/java/org/opensearch/index/store/remote/filecache/FileCacheTests.java b/server/src/test/java/org/opensearch/index/store/remote/filecache/FileCacheTests.java index c9d71c6deaadc..45b4c10493156 100644 --- a/server/src/test/java/org/opensearch/index/store/remote/filecache/FileCacheTests.java +++ b/server/src/test/java/org/opensearch/index/store/remote/filecache/FileCacheTests.java @@ -15,10 +15,6 @@ import org.apache.lucene.store.IndexInput; import org.opensearch.common.SetOnce; import org.opensearch.common.SuppressForbidden; -import org.opensearch.common.breaker.TestCircuitBreaker; -import org.opensearch.core.common.breaker.CircuitBreaker; -import org.opensearch.core.common.breaker.CircuitBreakingException; -import org.opensearch.core.common.breaker.NoopCircuitBreaker; import org.opensearch.env.NodeEnvironment; import org.opensearch.index.store.remote.directory.RemoteSnapshotDirectoryFactory; import org.opensearch.index.store.remote.file.CleanerDaemonThreadLeakFilter; @@ -52,17 +48,7 @@ public void init() throws Exception { } private FileCache createFileCache(long capacity) { - return FileCacheFactory.createConcurrentLRUFileCache(capacity, CONCURRENCY_LEVEL, new NoopCircuitBreaker(CircuitBreaker.REQUEST)); - } - - private FileCache createFileCache(long capacity, CircuitBreaker circuitBreaker) { - return FileCacheFactory.createConcurrentLRUFileCache(capacity, CONCURRENCY_LEVEL, circuitBreaker); - } - - private FileCache createCircuitBreakingFileCache(long capacity) { - TestCircuitBreaker testCircuitBreaker = new TestCircuitBreaker(); - testCircuitBreaker.startBreaking(); - return FileCacheFactory.createConcurrentLRUFileCache(capacity, CONCURRENCY_LEVEL, testCircuitBreaker); + return FileCacheFactory.createConcurrentLRUFileCache(capacity, CONCURRENCY_LEVEL); } private Path createPath(String middle) { @@ -182,13 +168,6 @@ public void testPutThrowException() { }); } - public void testPutThrowCircuitBreakingException() { - FileCache fileCache = createCircuitBreakingFileCache(MEGA_BYTES); - Path path = createPath("0"); - assertThrows(CircuitBreakingException.class, () -> fileCache.put(path, new StubCachedIndexInput(8 * MEGA_BYTES))); - assertNull(fileCache.get(path)); - } - public void testCompute() { FileCache fileCache = createFileCache(MEGA_BYTES); Path path = createPath("0"); @@ -206,27 +185,6 @@ public void testComputeThrowException() { }); } - public void testComputeThrowCircuitBreakingException() { - FileCache fileCache = createCircuitBreakingFileCache(MEGA_BYTES); - Path path = createPath("0"); - assertThrows(CircuitBreakingException.class, () -> fileCache.compute(path, (p, i) -> new StubCachedIndexInput(8 * MEGA_BYTES))); - assertNull(fileCache.get(path)); - } - - public void testEntryNotRemovedCircuitBreaker() { - TestCircuitBreaker circuitBreaker = new TestCircuitBreaker(); - FileCache fileCache = createFileCache(MEGA_BYTES, circuitBreaker); - Path path = createPath("0"); - fileCache.put(path, new StubCachedIndexInput(8 * MEGA_BYTES)); - // put should succeed since circuit breaker hasn't tripped yet - assertEquals(fileCache.get(path).length(), 8 * MEGA_BYTES); - circuitBreaker.startBreaking(); - // compute should throw CircuitBreakingException but shouldn't remove entry already present - assertThrows(CircuitBreakingException.class, () -> fileCache.compute(path, (p, i) -> new StubCachedIndexInput(2 * MEGA_BYTES))); - assertNotNull(fileCache.get(path)); - assertEquals(fileCache.get(path).length(), 8 * MEGA_BYTES); - } - public void testRemove() { FileCache fileCache = createFileCache(MEGA_BYTES); for (int i = 0; i < 4; i++) { @@ -347,11 +305,7 @@ public void testPruneWithPredicate() { } public void testUsage() { - FileCache fileCache = FileCacheFactory.createConcurrentLRUFileCache( - 16 * MEGA_BYTES, - 1, - new NoopCircuitBreaker(CircuitBreaker.REQUEST) - ); + FileCache fileCache = FileCacheFactory.createConcurrentLRUFileCache(16 * MEGA_BYTES, 1); putAndDecRef(fileCache, 0, 16 * MEGA_BYTES); long expectedCacheUsage = 16 * MEGA_BYTES; diff --git a/server/src/test/java/org/opensearch/index/store/remote/filecache/FileCachedIndexInputTests.java b/server/src/test/java/org/opensearch/index/store/remote/filecache/FileCachedIndexInputTests.java index 41e76d0b762ea..51057cb279222 100644 --- a/server/src/test/java/org/opensearch/index/store/remote/filecache/FileCachedIndexInputTests.java +++ b/server/src/test/java/org/opensearch/index/store/remote/filecache/FileCachedIndexInputTests.java @@ -14,8 +14,6 @@ import org.apache.lucene.store.IOContext; import org.apache.lucene.store.IndexInput; import org.apache.lucene.store.IndexOutput; -import org.opensearch.core.common.breaker.CircuitBreaker; -import org.opensearch.core.common.breaker.NoopCircuitBreaker; import org.opensearch.index.store.remote.file.CleanerDaemonThreadLeakFilter; import org.opensearch.test.OpenSearchTestCase; import org.junit.Before; @@ -45,7 +43,7 @@ public void setup() throws IOException { indexOutput.close(); filePath = basePath.resolve(TEST_FILE); underlyingIndexInput = fsDirectory.openInput(TEST_FILE, IOContext.DEFAULT); - fileCache = FileCacheFactory.createConcurrentLRUFileCache(FILE_CACHE_CAPACITY, new NoopCircuitBreaker(CircuitBreaker.REQUEST)); + fileCache = FileCacheFactory.createConcurrentLRUFileCache(FILE_CACHE_CAPACITY); } protected void setupIndexInputAndAddToFileCache() { diff --git a/server/src/test/java/org/opensearch/index/store/remote/utils/TransferManagerTestCase.java b/server/src/test/java/org/opensearch/index/store/remote/utils/TransferManagerTestCase.java index 139a4031ddc99..69119bca9d631 100644 --- a/server/src/test/java/org/opensearch/index/store/remote/utils/TransferManagerTestCase.java +++ b/server/src/test/java/org/opensearch/index/store/remote/utils/TransferManagerTestCase.java @@ -13,8 +13,6 @@ import org.apache.lucene.store.IndexInput; import org.apache.lucene.store.MMapDirectory; import org.apache.lucene.store.SimpleFSLockFactory; -import org.opensearch.core.common.breaker.CircuitBreaker; -import org.opensearch.core.common.breaker.NoopCircuitBreaker; import org.opensearch.index.store.remote.file.CleanerDaemonThreadLeakFilter; import org.opensearch.index.store.remote.filecache.FileCache; import org.opensearch.index.store.remote.filecache.FileCacheFactory; @@ -44,11 +42,7 @@ @ThreadLeakFilters(filters = CleanerDaemonThreadLeakFilter.class) public abstract class TransferManagerTestCase extends OpenSearchTestCase { protected static final int EIGHT_MB = 1024 * 1024 * 8; - protected final FileCache fileCache = FileCacheFactory.createConcurrentLRUFileCache( - EIGHT_MB * 2, - 1, - new NoopCircuitBreaker(CircuitBreaker.REQUEST) - ); + protected final FileCache fileCache = FileCacheFactory.createConcurrentLRUFileCache(EIGHT_MB * 2, 1); protected MMapDirectory directory; protected TransferManager transferManager; protected ThreadPool threadPool; diff --git a/server/src/test/java/org/opensearch/monitor/fs/FsProbeTests.java b/server/src/test/java/org/opensearch/monitor/fs/FsProbeTests.java index c9ac3a8996f58..d3a7fa3e354fa 100644 --- a/server/src/test/java/org/opensearch/monitor/fs/FsProbeTests.java +++ b/server/src/test/java/org/opensearch/monitor/fs/FsProbeTests.java @@ -35,8 +35,6 @@ import org.apache.lucene.util.Constants; import org.opensearch.common.collect.Tuple; import org.opensearch.common.settings.Settings; -import org.opensearch.core.common.breaker.CircuitBreaker; -import org.opensearch.core.common.breaker.NoopCircuitBreaker; import org.opensearch.core.common.unit.ByteSizeUnit; import org.opensearch.core.common.unit.ByteSizeValue; import org.opensearch.env.NodeEnvironment; @@ -130,11 +128,7 @@ public void testFsCacheInfo() throws IOException { try (NodeEnvironment env = newNodeEnvironment(settings)) { ByteSizeValue gbByteSizeValue = new ByteSizeValue(1, ByteSizeUnit.GB); env.fileCacheNodePath().fileCacheReservedSize = gbByteSizeValue; - FileCache fileCache = FileCacheFactory.createConcurrentLRUFileCache( - gbByteSizeValue.getBytes(), - 16, - new NoopCircuitBreaker(CircuitBreaker.REQUEST) - ); + FileCache fileCache = FileCacheFactory.createConcurrentLRUFileCache(gbByteSizeValue.getBytes(), 16); FsProbe probe = new FsProbe(env, fileCache); FsInfo stats = probe.stats(null); assertNotNull(stats); @@ -170,11 +164,7 @@ public void testFsInfoWhenFileCacheOccupied() throws IOException { final long totalSpace = adjustForHugeFilesystems(env.fileCacheNodePath().fileStore.getTotalSpace()); ByteSizeValue gbByteSizeValue = new ByteSizeValue(totalSpace, ByteSizeUnit.BYTES); env.fileCacheNodePath().fileCacheReservedSize = gbByteSizeValue; - FileCache fileCache = FileCacheFactory.createConcurrentLRUFileCache( - gbByteSizeValue.getBytes(), - 16, - new NoopCircuitBreaker(CircuitBreaker.REQUEST) - ); + FileCache fileCache = FileCacheFactory.createConcurrentLRUFileCache(gbByteSizeValue.getBytes(), 16); FsProbe probe = new FsProbe(env, fileCache); FsInfo stats = probe.stats(null);