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 {