diff --git a/CHANGELOG.md b/CHANGELOG.md index 7af3c171e8c6a..4a4452b1b43b6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -50,6 +50,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), - [Remote Store] Add Segment download stats to remotestore stats API ([#8718](https://github.com/opensearch-project/OpenSearch/pull/8718)) - [Remote Store] Add remote segment transfer stats on NodesStats API ([#9168](https://github.com/opensearch-project/OpenSearch/pull/9168)) - Return 409 Conflict HTTP status instead of 503 on failure to concurrently execute snapshots ([#8986](https://github.com/opensearch-project/OpenSearch/pull/5855)) +- Remove mmap.extensions setting ([#9392](https://github.com/opensearch-project/OpenSearch/pull/9392)) ### Deprecated diff --git a/server/src/main/java/org/opensearch/common/settings/IndexScopedSettings.java b/server/src/main/java/org/opensearch/common/settings/IndexScopedSettings.java index f14db4354f196..2c077268df8fd 100644 --- a/server/src/main/java/org/opensearch/common/settings/IndexScopedSettings.java +++ b/server/src/main/java/org/opensearch/common/settings/IndexScopedSettings.java @@ -182,7 +182,6 @@ public final class IndexScopedSettings extends AbstractScopedSettings { BitsetFilterCache.INDEX_LOAD_RANDOM_ACCESS_FILTERS_EAGERLY_SETTING, IndexModule.INDEX_STORE_TYPE_SETTING, IndexModule.INDEX_STORE_PRE_LOAD_SETTING, - IndexModule.INDEX_STORE_HYBRID_MMAP_EXTENSIONS, IndexModule.INDEX_STORE_HYBRID_NIO_EXTENSIONS, IndexModule.INDEX_RECOVERY_TYPE_SETTING, IndexModule.INDEX_QUERY_CACHE_ENABLED_SETTING, diff --git a/server/src/main/java/org/opensearch/index/IndexModule.java b/server/src/main/java/org/opensearch/index/IndexModule.java index ff91fd65b6f4e..d648be0ea3567 100644 --- a/server/src/main/java/org/opensearch/index/IndexModule.java +++ b/server/src/main/java/org/opensearch/index/IndexModule.java @@ -155,52 +155,7 @@ public final class IndexModule { Property.NodeScope ); - /** Which lucene file extensions to load with the mmap directory when using hybridfs store. This settings is ignored if {@link #INDEX_STORE_HYBRID_NIO_EXTENSIONS} is set. - * This is an expert setting. - * @see Lucene File Extensions. - * - * @deprecated This setting will be removed in OpenSearch 3.x. Use {@link #INDEX_STORE_HYBRID_NIO_EXTENSIONS} instead. - */ - @Deprecated - public static final Setting> INDEX_STORE_HYBRID_MMAP_EXTENSIONS = Setting.listSetting( - "index.store.hybrid.mmap.extensions", - List.of("nvd", "dvd", "tim", "tip", "dim", "kdd", "kdi", "cfs", "doc"), - Function.identity(), - new Setting.Validator>() { - - @Override - public void validate(final List value) {} - - @Override - public void validate(final List value, final Map, Object> settings) { - if (value.equals(INDEX_STORE_HYBRID_MMAP_EXTENSIONS.getDefault(Settings.EMPTY)) == false) { - final List nioExtensions = (List) settings.get(INDEX_STORE_HYBRID_NIO_EXTENSIONS); - final List defaultNioExtensions = INDEX_STORE_HYBRID_NIO_EXTENSIONS.getDefault(Settings.EMPTY); - if (nioExtensions.equals(defaultNioExtensions) == false) { - throw new IllegalArgumentException( - "Settings " - + INDEX_STORE_HYBRID_NIO_EXTENSIONS.getKey() - + " & " - + INDEX_STORE_HYBRID_MMAP_EXTENSIONS.getKey() - + " cannot both be set. Use " - + INDEX_STORE_HYBRID_NIO_EXTENSIONS.getKey() - + " only." - ); - } - } - } - - @Override - public Iterator> settings() { - return List.>of(INDEX_STORE_HYBRID_NIO_EXTENSIONS).iterator(); - } - }, - Property.IndexScope, - Property.NodeScope, - Property.Deprecated - ); - - /** Which lucene file extensions to load with nio. All others will default to mmap. Takes precedence over {@link #INDEX_STORE_HYBRID_MMAP_EXTENSIONS}. + /** Which lucene file extensions to load with nio. All others will default to mmap. * This is an expert setting. * @see Lucene File Extensions. */ @@ -226,35 +181,6 @@ public Iterator> settings() { "vem" ), Function.identity(), - new Setting.Validator>() { - - @Override - public void validate(final List value) {} - - @Override - public void validate(final List value, final Map, Object> settings) { - if (value.equals(INDEX_STORE_HYBRID_NIO_EXTENSIONS.getDefault(Settings.EMPTY)) == false) { - final List mmapExtensions = (List) settings.get(INDEX_STORE_HYBRID_MMAP_EXTENSIONS); - final List defaultMmapExtensions = INDEX_STORE_HYBRID_MMAP_EXTENSIONS.getDefault(Settings.EMPTY); - if (mmapExtensions.equals(defaultMmapExtensions) == false) { - throw new IllegalArgumentException( - "Settings " - + INDEX_STORE_HYBRID_NIO_EXTENSIONS.getKey() - + " & " - + INDEX_STORE_HYBRID_MMAP_EXTENSIONS.getKey() - + " cannot both be set. Use " - + INDEX_STORE_HYBRID_NIO_EXTENSIONS.getKey() - + " only." - ); - } - } - } - - @Override - public Iterator> settings() { - return List.>of(INDEX_STORE_HYBRID_MMAP_EXTENSIONS).iterator(); - } - }, Property.IndexScope, Property.NodeScope ); 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 9b5bc8f94ce35..8051360d31327 100644 --- a/server/src/main/java/org/opensearch/index/store/FsDirectoryFactory.java +++ b/server/src/main/java/org/opensearch/index/store/FsDirectoryFactory.java @@ -100,21 +100,7 @@ protected Directory newFSDirectory(Path location, LockFactory lockFactory, Index case HYBRIDFS: // Use Lucene defaults final FSDirectory primaryDirectory = FSDirectory.open(location, lockFactory); - final Set nioExtensions; - final Set mmapExtensions = Set.copyOf(indexSettings.getValue(IndexModule.INDEX_STORE_HYBRID_MMAP_EXTENSIONS)); - if (mmapExtensions.equals( - new HashSet(IndexModule.INDEX_STORE_HYBRID_MMAP_EXTENSIONS.getDefault(Settings.EMPTY)) - ) == false) { - // If the mmap extension setting was defined, then compute nio extensions by subtracting out the - // mmap extensions from the set of all extensions. - nioExtensions = Stream.concat( - IndexModule.INDEX_STORE_HYBRID_NIO_EXTENSIONS.getDefault(Settings.EMPTY).stream(), - IndexModule.INDEX_STORE_HYBRID_MMAP_EXTENSIONS.getDefault(Settings.EMPTY).stream() - ).filter(e -> mmapExtensions.contains(e) == false).collect(Collectors.toUnmodifiableSet()); - } else { - // Otherwise, get the list of nio extensions from the nio setting - nioExtensions = Set.copyOf(indexSettings.getValue(IndexModule.INDEX_STORE_HYBRID_NIO_EXTENSIONS)); - } + final Set nioExtensions = new HashSet<>(indexSettings.getValue(IndexModule.INDEX_STORE_HYBRID_NIO_EXTENSIONS)); if (primaryDirectory instanceof MMapDirectory) { MMapDirectory mMapDirectory = (MMapDirectory) primaryDirectory; return new HybridDirectory(lockFactory, setPreload(mMapDirectory, lockFactory, preLoadExtensions), nioExtensions); 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 bc9ee9d3d4fb6..980eaf4434120 100644 --- a/server/src/test/java/org/opensearch/index/store/FsDirectoryFactoryTests.java +++ b/server/src/test/java/org/opensearch/index/store/FsDirectoryFactoryTests.java @@ -96,7 +96,7 @@ public void testPreload() throws IOException { 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_NIO_EXTENSIONS.getKey(), "tip", "dim", "kdd", "kdi", "cfs", "doc") + .putList(IndexModule.INDEX_STORE_HYBRID_NIO_EXTENSIONS.getKey(), "tip", "dim", "kdd", "kdi", "cfs", "doc", "new") .build(); try (Directory directory = newDirectory(build)) { assertTrue(FsDirectoryFactory.isHybridFs(directory)); @@ -108,7 +108,7 @@ public void testPreload() throws IOException { assertTrue(hybridDirectory.useDelegate("foo.tim")); assertTrue(hybridDirectory.useDelegate("foo.pos")); assertTrue(hybridDirectory.useDelegate("foo.pay")); - assertTrue(hybridDirectory.useDelegate("foo.new")); + assertFalse(hybridDirectory.useDelegate("foo.new")); assertFalse(hybridDirectory.useDelegate("foo.tip")); assertFalse(hybridDirectory.useDelegate("foo.dim")); assertFalse(hybridDirectory.useDelegate("foo.kdd")); @@ -123,63 +123,6 @@ public void testPreload() throws IOException { assertTrue(preLoadMMapDirectory.useDelegate("foo.cfs")); assertTrue(preLoadMMapDirectory.useDelegate("foo.nvd")); } - 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") - .build(); - try (Directory directory = newDirectory(build)) { - assertTrue(FsDirectoryFactory.isHybridFs(directory)); - FsDirectoryFactory.HybridDirectory hybridDirectory = (FsDirectoryFactory.HybridDirectory) directory; - // test custom hybrid mmap extensions - // true->mmap, false->nio - assertTrue(hybridDirectory.useDelegate("foo.nvd")); - assertTrue(hybridDirectory.useDelegate("foo.dvd")); - assertTrue(hybridDirectory.useDelegate("foo.tim")); - assertTrue(hybridDirectory.useDelegate("foo.pos")); - assertTrue(hybridDirectory.useDelegate("foo.new")); - assertFalse(hybridDirectory.useDelegate("foo.pay")); - 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")); - MMapDirectory delegate = hybridDirectory.getDelegate(); - assertThat(delegate, Matchers.instanceOf(FsDirectoryFactory.PreLoadMMapDirectory.class)); - assertWarnings( - "[index.store.hybrid.mmap.extensions] setting was deprecated in OpenSearch and will be removed in a future release!" - + " See the breaking changes documentation for the next major version." - ); - } - 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") - .putList(IndexModule.INDEX_STORE_HYBRID_NIO_EXTENSIONS.getKey(), "nvd", "dvd", "tim", "pos") - .build(); - try { - newDirectory(build); - } catch (final Exception e) { - assertEquals( - "Settings index.store.hybrid.nio.extensions & index.store.hybrid.mmap.extensions cannot both be set. Use index.store.hybrid.nio.extensions only.", - e.getMessage() - ); - } - 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_NIO_EXTENSIONS.getKey(), "nvd", "dvd", "tim", "pos") - .putList(IndexModule.INDEX_STORE_HYBRID_MMAP_EXTENSIONS.getKey(), "nvd", "dvd", "tim", "pos") - .build(); - try { - newDirectory(build); - } catch (final Exception e) { - assertEquals( - "Settings index.store.hybrid.nio.extensions & index.store.hybrid.mmap.extensions cannot both be set. Use index.store.hybrid.nio.extensions only.", - e.getMessage() - ); - } 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") @@ -198,24 +141,6 @@ public void testPreload() throws IOException { MMapDirectory delegate = hybridDirectory.getDelegate(); assertThat(delegate, Matchers.instanceOf(FsDirectoryFactory.PreLoadMMapDirectory.class)); } - 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()) - .build(); - try (Directory directory = newDirectory(build)) { - assertTrue(FsDirectoryFactory.isHybridFs(directory)); - FsDirectoryFactory.HybridDirectory hybridDirectory = (FsDirectoryFactory.HybridDirectory) directory; - // test custom hybrid mmap extensions - // true->mmap, false->nio - assertTrue(hybridDirectory.useDelegate("foo.new")); - assertFalse(hybridDirectory.useDelegate("foo.nvd")); - assertFalse(hybridDirectory.useDelegate("foo.dvd")); - assertFalse(hybridDirectory.useDelegate("foo.cfs")); - assertFalse(hybridDirectory.useDelegate("foo.doc")); - MMapDirectory delegate = hybridDirectory.getDelegate(); - assertThat(delegate, Matchers.instanceOf(FsDirectoryFactory.PreLoadMMapDirectory.class)); - } } private Directory newDirectory(Settings settings) throws IOException {