From c59fb74fa9618c0fd0434bebaae4d674f0f7f424 Mon Sep 17 00:00:00 2001 From: Martin Gaievski Date: Fri, 6 Jan 2023 10:11:06 -0800 Subject: [PATCH 1/2] Allow additional settings for hybridfs from plugin Signed-off-by: Martin Gaievski --- CHANGELOG.md | 1 + .../smbmmapfs/SmbMmapFsDirectoryFactory.java | 9 ++- .../smbniofs/SmbNIOFsDirectoryFactory.java | 9 ++- .../opensearch/index/shard/IndexShardIT.java | 4 +- .../org/opensearch/index/IndexModule.java | 33 +++++++- .../org/opensearch/index/IndexService.java | 10 ++- .../opensearch/index/shard/IndexShard.java | 5 +- .../index/store/FsDirectoryFactory.java | 19 ++++- .../RemoteSnapshotDirectoryFactory.java | 8 +- .../opensearch/plugins/IndexStorePlugin.java | 7 +- .../opensearch/index/IndexModuleTests.java | 72 ++++++++++++++++- .../index/store/FsDirectoryFactoryTests.java | 79 +++++++++++++++++-- .../index/shard/IndexShardTestCase.java | 3 +- .../test/store/MockFSDirectoryFactory.java | 6 +- 14 files changed, 239 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 37f9fc821a5c0..8d29c4a43d9d0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - Add support for discovered cluster manager and remove local weights ([#5680](https://github.com/opensearch-project/OpenSearch/pull/5680)) - Added support for feature flags in opensearch.yml ([#4959](https://github.com/opensearch-project/OpenSearch/pull/4959)) - Add query for initialized extensions ([#5658](https://github.com/opensearch-project/OpenSearch/pull/5658)) +- Allow additional settings for hybridfs from plugin ([#5735](https://github.com/opensearch-project/OpenSearch/pull/5735)) ### Dependencies - Bumps `log4j-core` from 2.18.0 to 2.19.0 diff --git a/plugins/store-smb/src/main/java/org/opensearch/index/store/smbmmapfs/SmbMmapFsDirectoryFactory.java b/plugins/store-smb/src/main/java/org/opensearch/index/store/smbmmapfs/SmbMmapFsDirectoryFactory.java index 286688f61e658..516481870bd00 100644 --- a/plugins/store-smb/src/main/java/org/opensearch/index/store/smbmmapfs/SmbMmapFsDirectoryFactory.java +++ b/plugins/store-smb/src/main/java/org/opensearch/index/store/smbmmapfs/SmbMmapFsDirectoryFactory.java @@ -43,11 +43,18 @@ import java.io.IOException; import java.nio.file.Path; import java.util.HashSet; +import java.util.List; +import java.util.Map; public final class SmbMmapFsDirectoryFactory extends FsDirectoryFactory { @Override - protected Directory newFSDirectory(Path location, LockFactory lockFactory, IndexSettings indexSettings) throws IOException { + protected Directory newFSDirectory( + Path location, + LockFactory lockFactory, + IndexSettings indexSettings, + Map> additionalSettingProviders + ) throws IOException { return new SmbDirectoryWrapper( setPreload( new MMapDirectory(location, lockFactory), diff --git a/plugins/store-smb/src/main/java/org/opensearch/index/store/smbniofs/SmbNIOFsDirectoryFactory.java b/plugins/store-smb/src/main/java/org/opensearch/index/store/smbniofs/SmbNIOFsDirectoryFactory.java index 200f72dd66d89..038b106efd55b 100644 --- a/plugins/store-smb/src/main/java/org/opensearch/index/store/smbniofs/SmbNIOFsDirectoryFactory.java +++ b/plugins/store-smb/src/main/java/org/opensearch/index/store/smbniofs/SmbNIOFsDirectoryFactory.java @@ -17,6 +17,8 @@ import java.io.IOException; import java.nio.file.Path; +import java.util.List; +import java.util.Map; /** * Factory to create a new NIO File System type directory accessible as a SMB share @@ -24,7 +26,12 @@ public final class SmbNIOFsDirectoryFactory extends FsDirectoryFactory { @Override - protected Directory newFSDirectory(Path location, LockFactory lockFactory, IndexSettings indexSettings) throws IOException { + protected Directory newFSDirectory( + Path location, + LockFactory lockFactory, + IndexSettings indexSettings, + Map> additionalSettingProviders + ) throws IOException { return new SmbDirectoryWrapper(new NIOFSDirectory(location, lockFactory)); } } diff --git a/server/src/internalClusterTest/java/org/opensearch/index/shard/IndexShardIT.java b/server/src/internalClusterTest/java/org/opensearch/index/shard/IndexShardIT.java index d8476e6284d98..3af641ef1b0fd 100644 --- a/server/src/internalClusterTest/java/org/opensearch/index/shard/IndexShardIT.java +++ b/server/src/internalClusterTest/java/org/opensearch/index/shard/IndexShardIT.java @@ -107,6 +107,7 @@ import java.util.Comparator; import java.util.List; import java.util.Locale; +import java.util.Map; import java.util.concurrent.BrokenBarrierException; import java.util.concurrent.CountDownLatch; import java.util.concurrent.CyclicBarrier; @@ -701,7 +702,8 @@ public static final IndexShard newIndexShard( cbs, (indexSettings, shardRouting) -> new InternalTranslogFactory(), SegmentReplicationCheckpointPublisher.EMPTY, - null + null, + Map.of() ); } diff --git a/server/src/main/java/org/opensearch/index/IndexModule.java b/server/src/main/java/org/opensearch/index/IndexModule.java index e9125256438a5..58bcd179cbce3 100644 --- a/server/src/main/java/org/opensearch/index/IndexModule.java +++ b/server/src/main/java/org/opensearch/index/IndexModule.java @@ -198,6 +198,7 @@ public final class IndexModule { private final AtomicBoolean frozen = new AtomicBoolean(false); private final BooleanSupplier allowExpensiveQueries; private final Map recoveryStateFactories; + private final List>> fsExtensions = new ArrayList<>(); /** * Construct the index module for the index with the specified index settings. The index module contains extension points for plugins @@ -495,7 +496,7 @@ public IndexService newIndexService( BooleanSupplier idFieldDataEnabled, ValuesSourceRegistry valuesSourceRegistry, IndexStorePlugin.RemoteDirectoryFactory remoteDirectoryFactory, - BiFunction translogFactorySupplier + Supplier repositoriesServiceSupplier ) throws IOException { final IndexEventListener eventListener = freeze(); Function> readerWrapperFactory = indexReaderWrapper @@ -520,6 +521,14 @@ public IndexService newIndexService( if (IndexService.needsMapperService(indexSettings, indexCreationContext)) { indexAnalyzers = analysisRegistry.build(indexSettings); } + final Map> fsExtensionsByType = new HashMap<>(); + for (Type type : Type.values()) { + fsExtensionsByType.put(type, new HashSet<>()); + } + for (Map> extensions : fsExtensions) { + extensions.forEach((key, value) -> fsExtensionsByType.get(key).addAll(value)); + } + final IndexService indexService = new IndexService( indexSettings, indexCreationContext, @@ -551,7 +560,8 @@ public IndexService newIndexService( expressionResolver, valuesSourceRegistry, recoveryStateFactory, - translogFactorySupplier + translogFactorySupplier, + fsExtensionsByType ); success = true; return indexService; @@ -674,4 +684,23 @@ public static Map createBuiltInDirect } return factories; } + + /** + * Adds a supplier of extension's list. All listeners added here are maintained for the entire index lifecycle on + * this node. Once an index is closed or deleted these listeners go out of scope. + *

+ * Note: an index might be created on a node multiple times. For instance if the last shard from an index is + * relocated to another node the internal representation will be destroyed which includes the registered listeners. + * Once the node holds at least one shard of an index all modules are reloaded and listeners are registered again. + * Listeners can't be unregistered they will stay alive for the entire time the index is allocated on a node. + *

+ */ + public void addFSTypeFileExtensions(final Map> extensionsByType) { + ensureNotFrozen(); + if (extensionsByType == null) { + throw new IllegalArgumentException("supplier must not be null"); + } + + this.fsExtensions.add(extensionsByType); + } } diff --git a/server/src/main/java/org/opensearch/index/IndexService.java b/server/src/main/java/org/opensearch/index/IndexService.java index 78211f12f71ad..fab728ceedc06 100644 --- a/server/src/main/java/org/opensearch/index/IndexService.java +++ b/server/src/main/java/org/opensearch/index/IndexService.java @@ -175,6 +175,7 @@ public class IndexService extends AbstractIndexComponent implements IndicesClust private final Supplier indexSortSupplier; private final ValuesSourceRegistry valuesSourceRegistry; private final BiFunction translogFactorySupplier; + private final Map> fsExtensions; public IndexService( IndexSettings indexSettings, @@ -207,7 +208,8 @@ public IndexService( IndexNameExpressionResolver expressionResolver, ValuesSourceRegistry valuesSourceRegistry, IndexStorePlugin.RecoveryStateFactory recoveryStateFactory, - BiFunction translogFactorySupplier + BiFunction translogFactorySupplier, + Map> fsExtensions ) { super(indexSettings); this.allowExpensiveQueries = allowExpensiveQueries; @@ -280,6 +282,7 @@ public IndexService( this.globalCheckpointTask = new AsyncGlobalCheckpointTask(this); this.retentionLeaseSyncTask = new AsyncRetentionLeaseSyncTask(this); this.translogFactorySupplier = translogFactorySupplier; + this.fsExtensions = Objects.requireNonNull(fsExtensions); updateFsyncTaskIfNecessary(); } @@ -522,7 +525,7 @@ public synchronized IndexShard createShard( remoteStore = new Store(shardId, this.indexSettings, remoteDirectory, lock, Store.OnClose.EMPTY); } - Directory directory = directoryFactory.newDirectory(this.indexSettings, path); + Directory directory = directoryFactory.newDirectory(this.indexSettings, path, this.fsExtensions); store = new Store( shardId, this.indexSettings, @@ -554,7 +557,8 @@ public synchronized IndexShard createShard( circuitBreakerService, translogFactorySupplier, this.indexSettings.isSegRepEnabled() ? checkpointPublisher : null, - remoteStore + remoteStore, + fsExtensions ); eventListener.indexShardStateChanged(indexShard, null, indexShard.state(), "shard created"); eventListener.afterIndexShardCreated(indexShard); diff --git a/server/src/main/java/org/opensearch/index/shard/IndexShard.java b/server/src/main/java/org/opensearch/index/shard/IndexShard.java index 4be11badd0879..72088ce283025 100644 --- a/server/src/main/java/org/opensearch/index/shard/IndexShard.java +++ b/server/src/main/java/org/opensearch/index/shard/IndexShard.java @@ -323,6 +323,7 @@ Runnable getGlobalCheckpointSyncer() { private volatile boolean useRetentionLeasesInPeerRecovery; private final Store remoteStore; private final BiFunction translogFactorySupplier; + private final Map> fsExtensions; public IndexShard( final ShardRouting shardRouting, @@ -347,7 +348,8 @@ public IndexShard( final CircuitBreakerService circuitBreakerService, final BiFunction translogFactorySupplier, @Nullable final SegmentReplicationCheckpointPublisher checkpointPublisher, - @Nullable final Store remoteStore + @Nullable final Store remoteStore, + final Map> fsExtensions ) throws IOException { super(shardRouting.shardId(), indexSettings); assert shardRouting.initializing(); @@ -430,6 +432,7 @@ public boolean shouldCache(Query query) { persistMetadata(path, indexSettings, shardRouting, null, logger); this.useRetentionLeasesInPeerRecovery = replicationTracker.hasAllPeerRecoveryRetentionLeases(); this.refreshPendingLocationListener = new RefreshPendingLocationListener(); + this.fsExtensions = fsExtensions; this.checkpointPublisher = checkpointPublisher; this.remoteStore = remoteStore; this.translogFactorySupplier = translogFactorySupplier; diff --git a/server/src/main/java/org/opensearch/index/store/FsDirectoryFactory.java b/server/src/main/java/org/opensearch/index/store/FsDirectoryFactory.java index 88c7e12632863..d926e63cf9a64 100644 --- a/server/src/main/java/org/opensearch/index/store/FsDirectoryFactory.java +++ b/server/src/main/java/org/opensearch/index/store/FsDirectoryFactory.java @@ -55,6 +55,9 @@ import java.nio.file.Files; import java.nio.file.Path; import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Objects; import java.util.Set; /** @@ -76,14 +79,20 @@ public class FsDirectoryFactory implements IndexStorePlugin.DirectoryFactory { }, Property.IndexScope, Property.NodeScope); @Override - public Directory newDirectory(IndexSettings indexSettings, ShardPath path) throws IOException { + public Directory newDirectory(IndexSettings indexSettings, ShardPath path, Map> additionalSettingProviders) + throws IOException { final Path location = path.resolveIndex(); final LockFactory lockFactory = indexSettings.getValue(INDEX_LOCK_FACTOR_SETTING); Files.createDirectories(location); - return newFSDirectory(location, lockFactory, indexSettings); + return newFSDirectory(location, lockFactory, indexSettings, additionalSettingProviders); } - protected Directory newFSDirectory(Path location, LockFactory lockFactory, IndexSettings indexSettings) throws IOException { + protected Directory newFSDirectory( + Path location, + LockFactory lockFactory, + IndexSettings indexSettings, + Map> additionalSettingProviders + ) throws IOException { final String storeType = indexSettings.getSettings() .get(IndexModule.INDEX_STORE_TYPE_SETTING.getKey(), IndexModule.Type.FS.getSettingsKey()); IndexModule.Type type; @@ -98,6 +107,10 @@ protected Directory newFSDirectory(Path location, LockFactory lockFactory, Index // Use Lucene defaults final FSDirectory primaryDirectory = FSDirectory.open(location, lockFactory); final Set mmapExtensions = new HashSet<>(indexSettings.getValue(IndexModule.INDEX_STORE_HYBRID_MMAP_EXTENSIONS)); + Objects.requireNonNull(additionalSettingProviders); + if (additionalSettingProviders.containsKey(IndexModule.INDEX_STORE_HYBRID_MMAP_EXTENSIONS.getKey())) { + mmapExtensions.addAll(additionalSettingProviders.get(IndexModule.INDEX_STORE_HYBRID_MMAP_EXTENSIONS.getKey())); + } if (primaryDirectory instanceof MMapDirectory) { MMapDirectory mMapDirectory = (MMapDirectory) primaryDirectory; return new HybridDirectory(lockFactory, setPreload(mMapDirectory, lockFactory, preLoadExtensions), mmapExtensions); diff --git a/server/src/main/java/org/opensearch/index/store/remote/directory/RemoteSnapshotDirectoryFactory.java b/server/src/main/java/org/opensearch/index/store/remote/directory/RemoteSnapshotDirectoryFactory.java index fed1f127d113f..8fed5a876d7de 100644 --- a/server/src/main/java/org/opensearch/index/store/remote/directory/RemoteSnapshotDirectoryFactory.java +++ b/server/src/main/java/org/opensearch/index/store/remote/directory/RemoteSnapshotDirectoryFactory.java @@ -11,6 +11,8 @@ import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; +import java.util.List; +import java.util.Map; import java.util.concurrent.ExecutionException; import java.util.concurrent.Future; import java.util.function.Supplier; @@ -48,7 +50,11 @@ public RemoteSnapshotDirectoryFactory(Supplier repositories } @Override - public Directory newDirectory(IndexSettings indexSettings, ShardPath localShardPath) throws IOException { + public Directory newDirectory( + IndexSettings indexSettings, + ShardPath localShardPath, + Map> additionalSettingProviders + ) throws IOException { final String repositoryName = IndexSettings.SEARCHABLE_SNAPSHOT_REPOSITORY.get(indexSettings.getSettings()); final Repository repository = repositoriesService.get().repository(repositoryName); assert repository instanceof BlobStoreRepository : "repository should be instance of BlobStoreRepository"; diff --git a/server/src/main/java/org/opensearch/plugins/IndexStorePlugin.java b/server/src/main/java/org/opensearch/plugins/IndexStorePlugin.java index 1dc90a21c2f70..9f36597f23ba4 100644 --- a/server/src/main/java/org/opensearch/plugins/IndexStorePlugin.java +++ b/server/src/main/java/org/opensearch/plugins/IndexStorePlugin.java @@ -42,6 +42,7 @@ import java.io.IOException; import java.util.Collections; +import java.util.List; import java.util.Map; /** @@ -58,12 +59,14 @@ public interface IndexStorePlugin { interface DirectoryFactory { /** * Creates a new directory per shard. This method is called once per shard on shard creation. + * * @param indexSettings the shards index settings - * @param shardPath the path the shard is using + * @param shardPath the path the shard is using * @return a new lucene directory instance * @throws IOException if an IOException occurs while opening the directory */ - Directory newDirectory(IndexSettings indexSettings, ShardPath shardPath) throws IOException; + Directory newDirectory(IndexSettings indexSettings, ShardPath shardPath, Map> additionalSettingProviders) + throws IOException; } /** diff --git a/server/src/test/java/org/opensearch/index/IndexModuleTests.java b/server/src/test/java/org/opensearch/index/IndexModuleTests.java index d2374e767639c..4fbc81c648053 100644 --- a/server/src/test/java/org/opensearch/index/IndexModuleTests.java +++ b/server/src/test/java/org/opensearch/index/IndexModuleTests.java @@ -52,6 +52,8 @@ import org.opensearch.cluster.node.DiscoveryNode; import org.opensearch.cluster.routing.RecoverySource; import org.opensearch.cluster.routing.ShardRouting; +import org.opensearch.cluster.routing.ShardRoutingState; +import org.opensearch.cluster.routing.TestShardRouting; import org.opensearch.cluster.routing.UnassignedInfo; import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.CheckedFunction; @@ -82,6 +84,7 @@ import org.opensearch.index.fielddata.IndexFieldDataCache; import org.opensearch.index.mapper.ParsedDocument; import org.opensearch.index.mapper.Uid; +import org.opensearch.index.seqno.RetentionLeaseSyncer; import org.opensearch.index.shard.IndexEventListener; import org.opensearch.index.shard.IndexingOperationListener; import org.opensearch.index.shard.SearchOperationListener; @@ -103,6 +106,7 @@ import org.opensearch.indices.fielddata.cache.IndicesFieldDataCache; import org.opensearch.indices.mapper.MapperRegistry; import org.opensearch.indices.recovery.RecoveryState; +import org.opensearch.indices.replication.checkpoint.SegmentReplicationCheckpointPublisher; import org.opensearch.plugins.IndexStorePlugin; import org.opensearch.repositories.RepositoriesService; import org.opensearch.script.ScriptService; @@ -120,6 +124,7 @@ import java.io.IOException; import java.util.Collections; import java.util.HashSet; +import java.util.List; import java.util.Map; import java.util.Set; import java.util.concurrent.TimeUnit; @@ -128,6 +133,11 @@ import static java.util.Collections.emptyMap; import static java.util.Collections.singletonMap; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; import static org.opensearch.index.IndexService.IndexCreationContext.CREATE_INDEX; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.empty; @@ -632,6 +642,60 @@ public void testRegisterCustomRecoveryStateFactory() throws IOException { indexService.close("closing", false); } + public void testHybridFSExtensionsList() throws IOException { + final IndexStorePlugin.DirectoryFactory directoryFactory = spy(new FsDirectoryFactory()); + + final Settings settings = Settings.builder() + .put(Environment.PATH_HOME_SETTING.getKey(), createTempDir()) + .put("index.store.type", IndexModule.Type.HYBRIDFS.getSettingsKey()) + .build(); + + final IndexSettings indexSettings = IndexSettingsModule.newIndexSettings(index, settings); + final Map indexStoreFactories = singletonMap( + IndexModule.Type.HYBRIDFS.getSettingsKey(), + directoryFactory + ); + final IndexModule module = new IndexModule( + indexSettings, + emptyAnalysisRegistry, + new InternalEngineFactory(), + new EngineConfigFactory(indexSettings), + indexStoreFactories, + () -> true, + new IndexNameExpressionResolver(new ThreadContext(Settings.EMPTY)), + Collections.emptyMap() + ); + + try { + module.addHybridFSExtensions(() -> List.of("vec", "vem")); + module.addHybridFSExtensions(() -> List.of("txt", "vem")); + } catch (Exception ex) { + fail("not registered"); + } + + final IndexService indexService = newIndexService(module); + + final Index index = new Index("test", UUIDs.randomBase64UUID()); + final ShardId shardId = new ShardId(index, 0); + final ShardRouting shardRouting = TestShardRouting.newShardRouting( + shardId, + UUIDs.randomBase64UUID(), + false, + ShardRoutingState.INITIALIZING + ); + + indexService.createShard(shardRouting, s -> {}, RetentionLeaseSyncer.EMPTY, SegmentReplicationCheckpointPublisher.EMPTY); + + List expectedListOfExtensions = List.of("vec", "vem", "txt"); + verify(directoryFactory, times(1)).newDirectory( + any(), + any(), + eq(Map.of("index.store.hybrid.mmap.extensions", expectedListOfExtensions)) + ); + + indexService.close("simon says", false); + } + private ShardRouting createInitializedShardRouting() { ShardRouting shard = ShardRouting.newUnassigned( new ShardId("test", "_na_", 0), @@ -708,8 +772,12 @@ public SimScorer scorer(float boost, CollectionStatistics collectionStats, TermS public static final class FooFunction implements IndexStorePlugin.DirectoryFactory { @Override - public Directory newDirectory(IndexSettings indexSettings, ShardPath shardPath) throws IOException { - return new FsDirectoryFactory().newDirectory(indexSettings, shardPath); + public Directory newDirectory( + IndexSettings indexSettings, + ShardPath shardPath, + Map> additionalSettingProviders + ) throws IOException { + return newDirectory(indexSettings, shardPath, additionalSettingProviders); } } diff --git a/server/src/test/java/org/opensearch/index/store/FsDirectoryFactoryTests.java b/server/src/test/java/org/opensearch/index/store/FsDirectoryFactoryTests.java index ce40de0e9aa71..0bfc5a5894644 100644 --- a/server/src/test/java/org/opensearch/index/store/FsDirectoryFactoryTests.java +++ b/server/src/test/java/org/opensearch/index/store/FsDirectoryFactoryTests.java @@ -55,7 +55,9 @@ import java.nio.file.Files; import java.nio.file.Path; import java.util.Arrays; +import java.util.List; import java.util.Locale; +import java.util.Map; import static org.opensearch.test.store.MockFSDirectoryFactory.FILE_SYSTEM_BASED_STORE_TYPES; @@ -69,7 +71,7 @@ public void testPreload() throws IOException { .put(IndexModule.INDEX_STORE_TYPE_SETTING.getKey(), IndexModule.Type.HYBRIDFS.name().toLowerCase(Locale.ROOT)) .putList(IndexModule.INDEX_STORE_PRE_LOAD_SETTING.getKey(), "dvd", "bar") .build(); - try (Directory directory = newDirectory(build)) { + try (Directory directory = newDirectory(build, List.of())) { assertTrue(FsDirectoryFactory.isHybridFs(directory)); FsDirectoryFactory.HybridDirectory hybridDirectory = (FsDirectoryFactory.HybridDirectory) directory; // test default hybrid mmap extensions @@ -96,7 +98,7 @@ public void testPreload() throws IOException { .putList(IndexModule.INDEX_STORE_PRE_LOAD_SETTING.getKey(), "nvd", "dvd", "cfs") .putList(IndexModule.INDEX_STORE_HYBRID_MMAP_EXTENSIONS.getKey(), "nvd", "dvd", "tim", "pos", "pay") .build(); - try (Directory directory = newDirectory(build)) { + try (Directory directory = newDirectory(build, List.of())) { assertTrue(FsDirectoryFactory.isHybridFs(directory)); FsDirectoryFactory.HybridDirectory hybridDirectory = (FsDirectoryFactory.HybridDirectory) directory; // test custom hybrid mmap extensions @@ -121,12 +123,16 @@ public void testPreload() throws IOException { } } - private Directory newDirectory(Settings settings) throws IOException { + private Directory newDirectory(Settings settings, List additionalExtensions) throws IOException { IndexSettings idxSettings = IndexSettingsModule.newIndexSettings("foo", settings); Path tempDir = createTempDir().resolve(idxSettings.getUUID()).resolve("0"); Files.createDirectories(tempDir); ShardPath path = new ShardPath(false, tempDir, tempDir, new ShardId(idxSettings.getIndex(), 0)); - return new FsDirectoryFactory().newDirectory(idxSettings, path); + return new FsDirectoryFactory().newDirectory( + idxSettings, + path, + Map.of(IndexModule.INDEX_STORE_HYBRID_MMAP_EXTENSIONS.getKey(), additionalExtensions) + ); } private void doTestPreload(String... preload) throws IOException { @@ -134,7 +140,7 @@ private void doTestPreload(String... preload) throws IOException { .put(IndexModule.INDEX_STORE_TYPE_SETTING.getKey(), "mmapfs") .putList(IndexModule.INDEX_STORE_PRE_LOAD_SETTING.getKey(), preload) .build(); - Directory directory = newDirectory(build); + Directory directory = newDirectory(build, List.of()); try (Directory dir = directory) { assertSame(dir, directory); // prevent warnings assertFalse(directory instanceof SleepingLockWrapper); @@ -185,7 +191,7 @@ private void doTestStoreDirectory(Path tempDir, String typeSettingValue, IndexMo Settings settings = settingsBuilder.build(); IndexSettings indexSettings = IndexSettingsModule.newIndexSettings("foo", settings); FsDirectoryFactory service = new FsDirectoryFactory(); - try (Directory directory = service.newFSDirectory(tempDir, NoLockFactory.INSTANCE, indexSettings)) { + try (Directory directory = service.newFSDirectory(tempDir, NoLockFactory.INSTANCE, indexSettings, Map.of())) { switch (type) { case HYBRIDFS: assertTrue(FsDirectoryFactory.isHybridFs(directory)); @@ -210,4 +216,65 @@ private void doTestStoreDirectory(Path tempDir, String typeSettingValue, IndexMo } } } + + public void testAdditionalExtensionsForHybridFS() throws IOException { + Settings build = Settings.builder() + .put(IndexModule.INDEX_STORE_TYPE_SETTING.getKey(), IndexModule.Type.HYBRIDFS.name().toLowerCase(Locale.ROOT)) + .putList(IndexModule.INDEX_STORE_PRE_LOAD_SETTING.getKey(), "dvd", "bar") + .build(); + try (Directory directory = newDirectory(build, List.of("vec", "vex"))) { + assertTrue(FsDirectoryFactory.isHybridFs(directory)); + FsDirectoryFactory.HybridDirectory hybridDirectory = (FsDirectoryFactory.HybridDirectory) directory; + // test default hybrid mmap extensions + assertTrue(hybridDirectory.useDelegate("foo.nvd")); + assertTrue(hybridDirectory.useDelegate("foo.dvd")); + assertTrue(hybridDirectory.useDelegate("foo.tim")); + assertTrue(hybridDirectory.useDelegate("foo.tip")); + assertTrue(hybridDirectory.useDelegate("foo.dim")); + assertTrue(hybridDirectory.useDelegate("foo.kdd")); + assertTrue(hybridDirectory.useDelegate("foo.kdi")); + assertTrue(hybridDirectory.useDelegate("foo.cfs")); + assertTrue(hybridDirectory.useDelegate("foo.doc")); + assertFalse(hybridDirectory.useDelegate("foo.pos")); + assertFalse(hybridDirectory.useDelegate("foo.pay")); + assertTrue(hybridDirectory.useDelegate("foo.vec")); + assertTrue(hybridDirectory.useDelegate("foo.vex")); + MMapDirectory delegate = hybridDirectory.getDelegate(); + assertThat(delegate, Matchers.instanceOf(FsDirectoryFactory.PreLoadMMapDirectory.class)); + FsDirectoryFactory.PreLoadMMapDirectory preLoadMMapDirectory = (FsDirectoryFactory.PreLoadMMapDirectory) delegate; + assertTrue(preLoadMMapDirectory.useDelegate("foo.dvd")); + assertTrue(preLoadMMapDirectory.useDelegate("foo.bar")); + assertFalse(preLoadMMapDirectory.useDelegate("foo.cfs")); + } + build = Settings.builder() + .put(IndexModule.INDEX_STORE_TYPE_SETTING.getKey(), IndexModule.Type.HYBRIDFS.name().toLowerCase(Locale.ROOT)) + .putList(IndexModule.INDEX_STORE_PRE_LOAD_SETTING.getKey(), "nvd", "dvd", "cfs") + .putList(IndexModule.INDEX_STORE_HYBRID_MMAP_EXTENSIONS.getKey(), "nvd", "dvd", "tim", "pos", "pay") + .build(); + try (Directory directory = newDirectory(build, List.of("nvd", "vec", "vex"))) { + assertTrue(FsDirectoryFactory.isHybridFs(directory)); + FsDirectoryFactory.HybridDirectory hybridDirectory = (FsDirectoryFactory.HybridDirectory) directory; + // test custom hybrid mmap extensions + assertTrue(hybridDirectory.useDelegate("foo.nvd")); + assertTrue(hybridDirectory.useDelegate("foo.dvd")); + assertTrue(hybridDirectory.useDelegate("foo.tim")); + assertFalse(hybridDirectory.useDelegate("foo.tip")); + assertFalse(hybridDirectory.useDelegate("foo.dim")); + assertFalse(hybridDirectory.useDelegate("foo.kdd")); + assertFalse(hybridDirectory.useDelegate("foo.kdi")); + assertFalse(hybridDirectory.useDelegate("foo.cfs")); + assertFalse(hybridDirectory.useDelegate("foo.doc")); + assertTrue(hybridDirectory.useDelegate("foo.pos")); + assertTrue(hybridDirectory.useDelegate("foo.pay")); + assertTrue(hybridDirectory.useDelegate("foo.vec")); + assertTrue(hybridDirectory.useDelegate("foo.vex")); + MMapDirectory delegate = hybridDirectory.getDelegate(); + assertThat(delegate, Matchers.instanceOf(FsDirectoryFactory.PreLoadMMapDirectory.class)); + FsDirectoryFactory.PreLoadMMapDirectory preLoadMMapDirectory = (FsDirectoryFactory.PreLoadMMapDirectory) delegate; + assertTrue(preLoadMMapDirectory.useDelegate("foo.dvd")); + assertFalse(preLoadMMapDirectory.useDelegate("foo.bar")); + assertTrue(preLoadMMapDirectory.useDelegate("foo.cfs")); + assertTrue(preLoadMMapDirectory.useDelegate("foo.nvd")); + } + } } diff --git a/test/framework/src/main/java/org/opensearch/index/shard/IndexShardTestCase.java b/test/framework/src/main/java/org/opensearch/index/shard/IndexShardTestCase.java index 3ae79a8a17879..4197cbaa7a3eb 100644 --- a/test/framework/src/main/java/org/opensearch/index/shard/IndexShardTestCase.java +++ b/test/framework/src/main/java/org/opensearch/index/shard/IndexShardTestCase.java @@ -590,7 +590,8 @@ protected IndexShard newShard( breakerService, translogFactorySupplier, checkpointPublisher, - remoteStore + remoteStore, + Map.of() ); indexShard.addShardFailureCallback(DEFAULT_SHARD_FAILURE_HANDLER); success = true; diff --git a/test/framework/src/main/java/org/opensearch/test/store/MockFSDirectoryFactory.java b/test/framework/src/main/java/org/opensearch/test/store/MockFSDirectoryFactory.java index e38b62c419334..93ad394964e58 100644 --- a/test/framework/src/main/java/org/opensearch/test/store/MockFSDirectoryFactory.java +++ b/test/framework/src/main/java/org/opensearch/test/store/MockFSDirectoryFactory.java @@ -64,6 +64,7 @@ import java.nio.charset.StandardCharsets; import java.util.Arrays; import java.util.List; +import java.util.Map; import java.util.Random; import java.util.Set; import java.util.stream.Collectors; @@ -95,7 +96,8 @@ public class MockFSDirectoryFactory implements IndexStorePlugin.DirectoryFactory ); @Override - public Directory newDirectory(IndexSettings idxSettings, ShardPath path) throws IOException { + public Directory newDirectory(IndexSettings idxSettings, ShardPath path, Map> additionalSettingProviders) + throws IOException { Settings indexSettings = idxSettings.getSettings(); Random random = new Random(idxSettings.getValue(OpenSearchIntegTestCase.INDEX_TEST_SEED_SETTING)); return wrap(randomDirectoryService(random, idxSettings, path), random, indexSettings, path.getShardId()); @@ -178,7 +180,7 @@ private Directory randomDirectoryService(Random random, IndexSettings indexSetti ) .build(); final IndexSettings newIndexSettings = new IndexSettings(build, indexSettings.getNodeSettings()); - return new FsDirectoryFactory().newDirectory(newIndexSettings, path); + return new FsDirectoryFactory().newDirectory(newIndexSettings, path, Map.of()); } public static final class OpenSearchMockDirectoryWrapper extends MockDirectoryWrapper { From 03da78228f5160fc756f49c7a070aaa1757f675f Mon Sep 17 00:00:00 2001 From: Martin Gaievski Date: Mon, 9 Jan 2023 21:06:07 -0800 Subject: [PATCH 2/2] Focus on extensions by type, refactor code Signed-off-by: Martin Gaievski --- .../smbmmapfs/SmbMmapFsDirectoryFactory.java | 4 +-- .../smbniofs/SmbNIOFsDirectoryFactory.java | 5 ++-- .../org/opensearch/index/IndexModule.java | 2 +- .../index/store/FsDirectoryFactory.java | 18 +++++++----- .../RemoteSnapshotDirectoryFactory.java | 8 +----- .../opensearch/plugins/IndexStorePlugin.java | 22 +++++++++++---- .../opensearch/index/IndexModuleTests.java | 28 ++++++++----------- .../index/store/FsDirectoryFactoryTests.java | 20 ++++++------- .../test/store/MockFSDirectoryFactory.java | 6 ++-- 9 files changed, 56 insertions(+), 57 deletions(-) diff --git a/plugins/store-smb/src/main/java/org/opensearch/index/store/smbmmapfs/SmbMmapFsDirectoryFactory.java b/plugins/store-smb/src/main/java/org/opensearch/index/store/smbmmapfs/SmbMmapFsDirectoryFactory.java index 516481870bd00..6adaa6ae22eba 100644 --- a/plugins/store-smb/src/main/java/org/opensearch/index/store/smbmmapfs/SmbMmapFsDirectoryFactory.java +++ b/plugins/store-smb/src/main/java/org/opensearch/index/store/smbmmapfs/SmbMmapFsDirectoryFactory.java @@ -43,8 +43,8 @@ import java.io.IOException; import java.nio.file.Path; import java.util.HashSet; -import java.util.List; import java.util.Map; +import java.util.Set; public final class SmbMmapFsDirectoryFactory extends FsDirectoryFactory { @@ -53,7 +53,7 @@ protected Directory newFSDirectory( Path location, LockFactory lockFactory, IndexSettings indexSettings, - Map> additionalSettingProviders + Map> additionalExtensions ) throws IOException { return new SmbDirectoryWrapper( setPreload( diff --git a/plugins/store-smb/src/main/java/org/opensearch/index/store/smbniofs/SmbNIOFsDirectoryFactory.java b/plugins/store-smb/src/main/java/org/opensearch/index/store/smbniofs/SmbNIOFsDirectoryFactory.java index 038b106efd55b..c9764f69d22bd 100644 --- a/plugins/store-smb/src/main/java/org/opensearch/index/store/smbniofs/SmbNIOFsDirectoryFactory.java +++ b/plugins/store-smb/src/main/java/org/opensearch/index/store/smbniofs/SmbNIOFsDirectoryFactory.java @@ -11,14 +11,15 @@ import org.apache.lucene.store.Directory; import org.apache.lucene.store.LockFactory; import org.apache.lucene.store.NIOFSDirectory; +import org.opensearch.index.IndexModule; import org.opensearch.index.IndexSettings; import org.opensearch.index.store.FsDirectoryFactory; import org.opensearch.index.store.SmbDirectoryWrapper; import java.io.IOException; import java.nio.file.Path; -import java.util.List; import java.util.Map; +import java.util.Set; /** * Factory to create a new NIO File System type directory accessible as a SMB share @@ -30,7 +31,7 @@ protected Directory newFSDirectory( Path location, LockFactory lockFactory, IndexSettings indexSettings, - Map> additionalSettingProviders + Map> additionalExtensions ) throws IOException { return new SmbDirectoryWrapper(new NIOFSDirectory(location, lockFactory)); } diff --git a/server/src/main/java/org/opensearch/index/IndexModule.java b/server/src/main/java/org/opensearch/index/IndexModule.java index 58bcd179cbce3..e5fcf50ba096b 100644 --- a/server/src/main/java/org/opensearch/index/IndexModule.java +++ b/server/src/main/java/org/opensearch/index/IndexModule.java @@ -496,7 +496,7 @@ public IndexService newIndexService( BooleanSupplier idFieldDataEnabled, ValuesSourceRegistry valuesSourceRegistry, IndexStorePlugin.RemoteDirectoryFactory remoteDirectoryFactory, - Supplier repositoriesServiceSupplier + BiFunction translogFactorySupplier ) throws IOException { final IndexEventListener eventListener = freeze(); Function> readerWrapperFactory = indexReaderWrapper diff --git a/server/src/main/java/org/opensearch/index/store/FsDirectoryFactory.java b/server/src/main/java/org/opensearch/index/store/FsDirectoryFactory.java index d926e63cf9a64..3c5dcdf59e347 100644 --- a/server/src/main/java/org/opensearch/index/store/FsDirectoryFactory.java +++ b/server/src/main/java/org/opensearch/index/store/FsDirectoryFactory.java @@ -55,7 +55,6 @@ import java.nio.file.Files; import java.nio.file.Path; import java.util.HashSet; -import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Set; @@ -79,19 +78,24 @@ public class FsDirectoryFactory implements IndexStorePlugin.DirectoryFactory { }, Property.IndexScope, Property.NodeScope); @Override - public Directory newDirectory(IndexSettings indexSettings, ShardPath path, Map> additionalSettingProviders) + public Directory newDirectory(IndexSettings indexSettings, ShardPath shardPath) throws IOException { + return newDirectory(indexSettings, shardPath, Map.of()); + } + + @Override + public Directory newDirectory(IndexSettings indexSettings, ShardPath path, Map> additionalExtensions) throws IOException { final Path location = path.resolveIndex(); final LockFactory lockFactory = indexSettings.getValue(INDEX_LOCK_FACTOR_SETTING); Files.createDirectories(location); - return newFSDirectory(location, lockFactory, indexSettings, additionalSettingProviders); + return newFSDirectory(location, lockFactory, indexSettings, additionalExtensions); } protected Directory newFSDirectory( Path location, LockFactory lockFactory, IndexSettings indexSettings, - Map> additionalSettingProviders + Map> additionalExtensions ) throws IOException { final String storeType = indexSettings.getSettings() .get(IndexModule.INDEX_STORE_TYPE_SETTING.getKey(), IndexModule.Type.FS.getSettingsKey()); @@ -107,9 +111,9 @@ protected Directory newFSDirectory( // Use Lucene defaults final FSDirectory primaryDirectory = FSDirectory.open(location, lockFactory); final Set mmapExtensions = new HashSet<>(indexSettings.getValue(IndexModule.INDEX_STORE_HYBRID_MMAP_EXTENSIONS)); - Objects.requireNonNull(additionalSettingProviders); - if (additionalSettingProviders.containsKey(IndexModule.INDEX_STORE_HYBRID_MMAP_EXTENSIONS.getKey())) { - mmapExtensions.addAll(additionalSettingProviders.get(IndexModule.INDEX_STORE_HYBRID_MMAP_EXTENSIONS.getKey())); + Objects.requireNonNull(additionalExtensions); + if (additionalExtensions.containsKey(IndexModule.Type.HYBRIDFS)) { + mmapExtensions.addAll(additionalExtensions.get(IndexModule.Type.HYBRIDFS)); } if (primaryDirectory instanceof MMapDirectory) { MMapDirectory mMapDirectory = (MMapDirectory) primaryDirectory; diff --git a/server/src/main/java/org/opensearch/index/store/remote/directory/RemoteSnapshotDirectoryFactory.java b/server/src/main/java/org/opensearch/index/store/remote/directory/RemoteSnapshotDirectoryFactory.java index 8fed5a876d7de..fed1f127d113f 100644 --- a/server/src/main/java/org/opensearch/index/store/remote/directory/RemoteSnapshotDirectoryFactory.java +++ b/server/src/main/java/org/opensearch/index/store/remote/directory/RemoteSnapshotDirectoryFactory.java @@ -11,8 +11,6 @@ import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; -import java.util.List; -import java.util.Map; import java.util.concurrent.ExecutionException; import java.util.concurrent.Future; import java.util.function.Supplier; @@ -50,11 +48,7 @@ public RemoteSnapshotDirectoryFactory(Supplier repositories } @Override - public Directory newDirectory( - IndexSettings indexSettings, - ShardPath localShardPath, - Map> additionalSettingProviders - ) throws IOException { + public Directory newDirectory(IndexSettings indexSettings, ShardPath localShardPath) throws IOException { final String repositoryName = IndexSettings.SEARCHABLE_SNAPSHOT_REPOSITORY.get(indexSettings.getSettings()); final Repository repository = repositoriesService.get().repository(repositoryName); assert repository instanceof BlobStoreRepository : "repository should be instance of BlobStoreRepository"; diff --git a/server/src/main/java/org/opensearch/plugins/IndexStorePlugin.java b/server/src/main/java/org/opensearch/plugins/IndexStorePlugin.java index 9f36597f23ba4..24677b2d09c81 100644 --- a/server/src/main/java/org/opensearch/plugins/IndexStorePlugin.java +++ b/server/src/main/java/org/opensearch/plugins/IndexStorePlugin.java @@ -36,14 +36,15 @@ import org.opensearch.cluster.node.DiscoveryNode; import org.opensearch.cluster.routing.ShardRouting; import org.opensearch.common.Nullable; +import org.opensearch.index.IndexModule; import org.opensearch.index.IndexSettings; import org.opensearch.index.shard.ShardPath; import org.opensearch.indices.recovery.RecoveryState; import java.io.IOException; import java.util.Collections; -import java.util.List; import java.util.Map; +import java.util.Set; /** * A plugin that provides alternative directory implementations. @@ -59,14 +60,25 @@ public interface IndexStorePlugin { interface DirectoryFactory { /** * Creates a new directory per shard. This method is called once per shard on shard creation. - * * @param indexSettings the shards index settings - * @param shardPath the path the shard is using + * @param shardPath the path the shard is using + * @return a new lucene directory instance + * @throws IOException if an IOException occurs while opening the directory + */ + Directory newDirectory(IndexSettings indexSettings, ShardPath shardPath) throws IOException; + + /** + * Creates a new directory per shard. This method is called once per shard on shard creation. + * @param indexSettings the shards index settings + * @param shardPath the path the shard is using + * @param extensions map of providers for additional settings where key is a setting name * @return a new lucene directory instance * @throws IOException if an IOException occurs while opening the directory */ - Directory newDirectory(IndexSettings indexSettings, ShardPath shardPath, Map> additionalSettingProviders) - throws IOException; + default Directory newDirectory(IndexSettings indexSettings, ShardPath shardPath, Map> extensions) + throws IOException { + return newDirectory(indexSettings, shardPath); + } } /** diff --git a/server/src/test/java/org/opensearch/index/IndexModuleTests.java b/server/src/test/java/org/opensearch/index/IndexModuleTests.java index 4fbc81c648053..a7c4216695b59 100644 --- a/server/src/test/java/org/opensearch/index/IndexModuleTests.java +++ b/server/src/test/java/org/opensearch/index/IndexModuleTests.java @@ -46,6 +46,7 @@ import org.apache.lucene.store.Directory; import org.apache.lucene.util.SetOnce; import org.apache.lucene.util.SetOnce.AlreadySetException; +import org.mockito.ArgumentCaptor; import org.opensearch.Version; import org.opensearch.cluster.metadata.IndexMetadata; import org.opensearch.cluster.metadata.IndexNameExpressionResolver; @@ -124,7 +125,6 @@ import java.io.IOException; import java.util.Collections; import java.util.HashSet; -import java.util.List; import java.util.Map; import java.util.Set; import java.util.concurrent.TimeUnit; @@ -134,7 +134,6 @@ import static java.util.Collections.emptyMap; import static java.util.Collections.singletonMap; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -667,8 +666,8 @@ public void testHybridFSExtensionsList() throws IOException { ); try { - module.addHybridFSExtensions(() -> List.of("vec", "vem")); - module.addHybridFSExtensions(() -> List.of("txt", "vem")); + module.addFSTypeFileExtensions(Map.of(IndexModule.Type.HYBRIDFS, Set.of("vec", "vem"))); + module.addFSTypeFileExtensions(Map.of(IndexModule.Type.HYBRIDFS, Set.of("txt", "vem"))); } catch (Exception ex) { fail("not registered"); } @@ -685,13 +684,12 @@ public void testHybridFSExtensionsList() throws IOException { ); indexService.createShard(shardRouting, s -> {}, RetentionLeaseSyncer.EMPTY, SegmentReplicationCheckpointPublisher.EMPTY); - - List expectedListOfExtensions = List.of("vec", "vem", "txt"); - verify(directoryFactory, times(1)).newDirectory( - any(), - any(), - eq(Map.of("index.store.hybrid.mmap.extensions", expectedListOfExtensions)) - ); + ArgumentCaptor>> extensionsCaptor = ArgumentCaptor.forClass(Map.class); + Set expectedListOfExtensions = Set.of("vec", "vem", "txt"); + verify(directoryFactory, times(1)).newDirectory(any(), any(), extensionsCaptor.capture()); + assertNotNull(extensionsCaptor.getValue()); + assertTrue(extensionsCaptor.getValue().containsKey(IndexModule.Type.HYBRIDFS)); + assertEquals(expectedListOfExtensions, extensionsCaptor.getValue().get(IndexModule.Type.HYBRIDFS)); indexService.close("simon says", false); } @@ -772,12 +770,8 @@ public SimScorer scorer(float boost, CollectionStatistics collectionStats, TermS public static final class FooFunction implements IndexStorePlugin.DirectoryFactory { @Override - public Directory newDirectory( - IndexSettings indexSettings, - ShardPath shardPath, - Map> additionalSettingProviders - ) throws IOException { - return newDirectory(indexSettings, shardPath, additionalSettingProviders); + public Directory newDirectory(IndexSettings indexSettings, ShardPath shardPath) throws IOException { + return newDirectory(indexSettings, shardPath); } } diff --git a/server/src/test/java/org/opensearch/index/store/FsDirectoryFactoryTests.java b/server/src/test/java/org/opensearch/index/store/FsDirectoryFactoryTests.java index 0bfc5a5894644..11acdf7bf4fac 100644 --- a/server/src/test/java/org/opensearch/index/store/FsDirectoryFactoryTests.java +++ b/server/src/test/java/org/opensearch/index/store/FsDirectoryFactoryTests.java @@ -55,9 +55,9 @@ import java.nio.file.Files; import java.nio.file.Path; import java.util.Arrays; -import java.util.List; import java.util.Locale; import java.util.Map; +import java.util.Set; import static org.opensearch.test.store.MockFSDirectoryFactory.FILE_SYSTEM_BASED_STORE_TYPES; @@ -71,7 +71,7 @@ public void testPreload() throws IOException { .put(IndexModule.INDEX_STORE_TYPE_SETTING.getKey(), IndexModule.Type.HYBRIDFS.name().toLowerCase(Locale.ROOT)) .putList(IndexModule.INDEX_STORE_PRE_LOAD_SETTING.getKey(), "dvd", "bar") .build(); - try (Directory directory = newDirectory(build, List.of())) { + try (Directory directory = newDirectory(build, Set.of())) { assertTrue(FsDirectoryFactory.isHybridFs(directory)); FsDirectoryFactory.HybridDirectory hybridDirectory = (FsDirectoryFactory.HybridDirectory) directory; // test default hybrid mmap extensions @@ -98,7 +98,7 @@ public void testPreload() throws IOException { .putList(IndexModule.INDEX_STORE_PRE_LOAD_SETTING.getKey(), "nvd", "dvd", "cfs") .putList(IndexModule.INDEX_STORE_HYBRID_MMAP_EXTENSIONS.getKey(), "nvd", "dvd", "tim", "pos", "pay") .build(); - try (Directory directory = newDirectory(build, List.of())) { + try (Directory directory = newDirectory(build, Set.of())) { assertTrue(FsDirectoryFactory.isHybridFs(directory)); FsDirectoryFactory.HybridDirectory hybridDirectory = (FsDirectoryFactory.HybridDirectory) directory; // test custom hybrid mmap extensions @@ -123,16 +123,12 @@ public void testPreload() throws IOException { } } - private Directory newDirectory(Settings settings, List additionalExtensions) throws IOException { + private Directory newDirectory(Settings settings, Set additionalExtensions) throws IOException { IndexSettings idxSettings = IndexSettingsModule.newIndexSettings("foo", settings); Path tempDir = createTempDir().resolve(idxSettings.getUUID()).resolve("0"); Files.createDirectories(tempDir); ShardPath path = new ShardPath(false, tempDir, tempDir, new ShardId(idxSettings.getIndex(), 0)); - return new FsDirectoryFactory().newDirectory( - idxSettings, - path, - Map.of(IndexModule.INDEX_STORE_HYBRID_MMAP_EXTENSIONS.getKey(), additionalExtensions) - ); + return new FsDirectoryFactory().newDirectory(idxSettings, path, Map.of(IndexModule.Type.HYBRIDFS, additionalExtensions)); } private void doTestPreload(String... preload) throws IOException { @@ -140,7 +136,7 @@ private void doTestPreload(String... preload) throws IOException { .put(IndexModule.INDEX_STORE_TYPE_SETTING.getKey(), "mmapfs") .putList(IndexModule.INDEX_STORE_PRE_LOAD_SETTING.getKey(), preload) .build(); - Directory directory = newDirectory(build, List.of()); + Directory directory = newDirectory(build, Set.of()); try (Directory dir = directory) { assertSame(dir, directory); // prevent warnings assertFalse(directory instanceof SleepingLockWrapper); @@ -222,7 +218,7 @@ public void testAdditionalExtensionsForHybridFS() throws IOException { .put(IndexModule.INDEX_STORE_TYPE_SETTING.getKey(), IndexModule.Type.HYBRIDFS.name().toLowerCase(Locale.ROOT)) .putList(IndexModule.INDEX_STORE_PRE_LOAD_SETTING.getKey(), "dvd", "bar") .build(); - try (Directory directory = newDirectory(build, List.of("vec", "vex"))) { + try (Directory directory = newDirectory(build, Set.of("vec", "vex"))) { assertTrue(FsDirectoryFactory.isHybridFs(directory)); FsDirectoryFactory.HybridDirectory hybridDirectory = (FsDirectoryFactory.HybridDirectory) directory; // test default hybrid mmap extensions @@ -251,7 +247,7 @@ public void testAdditionalExtensionsForHybridFS() throws IOException { .putList(IndexModule.INDEX_STORE_PRE_LOAD_SETTING.getKey(), "nvd", "dvd", "cfs") .putList(IndexModule.INDEX_STORE_HYBRID_MMAP_EXTENSIONS.getKey(), "nvd", "dvd", "tim", "pos", "pay") .build(); - try (Directory directory = newDirectory(build, List.of("nvd", "vec", "vex"))) { + try (Directory directory = newDirectory(build, Set.of("nvd", "vec", "vex"))) { assertTrue(FsDirectoryFactory.isHybridFs(directory)); FsDirectoryFactory.HybridDirectory hybridDirectory = (FsDirectoryFactory.HybridDirectory) directory; // test custom hybrid mmap extensions diff --git a/test/framework/src/main/java/org/opensearch/test/store/MockFSDirectoryFactory.java b/test/framework/src/main/java/org/opensearch/test/store/MockFSDirectoryFactory.java index 93ad394964e58..e38b62c419334 100644 --- a/test/framework/src/main/java/org/opensearch/test/store/MockFSDirectoryFactory.java +++ b/test/framework/src/main/java/org/opensearch/test/store/MockFSDirectoryFactory.java @@ -64,7 +64,6 @@ import java.nio.charset.StandardCharsets; import java.util.Arrays; import java.util.List; -import java.util.Map; import java.util.Random; import java.util.Set; import java.util.stream.Collectors; @@ -96,8 +95,7 @@ public class MockFSDirectoryFactory implements IndexStorePlugin.DirectoryFactory ); @Override - public Directory newDirectory(IndexSettings idxSettings, ShardPath path, Map> additionalSettingProviders) - throws IOException { + public Directory newDirectory(IndexSettings idxSettings, ShardPath path) throws IOException { Settings indexSettings = idxSettings.getSettings(); Random random = new Random(idxSettings.getValue(OpenSearchIntegTestCase.INDEX_TEST_SEED_SETTING)); return wrap(randomDirectoryService(random, idxSettings, path), random, indexSettings, path.getShardId()); @@ -180,7 +178,7 @@ private Directory randomDirectoryService(Random random, IndexSettings indexSetti ) .build(); final IndexSettings newIndexSettings = new IndexSettings(build, indexSettings.getNodeSettings()); - return new FsDirectoryFactory().newDirectory(newIndexSettings, path, Map.of()); + return new FsDirectoryFactory().newDirectory(newIndexSettings, path); } public static final class OpenSearchMockDirectoryWrapper extends MockDirectoryWrapper {