Skip to content

Commit

Permalink
Remove mmap.extensions setting
Browse files Browse the repository at this point in the history 
Signed-off-by: David Zane <[email protected]>
  • Loading branch information
@dzane17
dzane17 committed Aug 16, 2023
1 parent 8d153ab commit a18b958
Show file tree
Hide file tree
Showing 4 changed files with 4 additions and 168 deletions.
1 change: 0 additions & 1 deletion server/src/main/java/org/opensearch/common/settings/IndexScopedSettings.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
76 changes: 1 addition & 75 deletions server/src/main/java/org/opensearch/index/IndexModule.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 <a href="https://lucene.apache.org/core/9_5_0/core/org/apache/lucene/codecs/lucene95/package-summary.html#file-names">Lucene File Extensions</a>.
*
* @deprecated This setting will be removed in OpenSearch 3.x. Use {@link #INDEX_STORE_HYBRID_NIO_EXTENSIONS} instead.
*/
@Deprecated
public static final Setting<List<String>> 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<List<String>>() {

@Override
public void validate(final List<String> value) {}

@Override
public void validate(final List<String> value, final Map<Setting<?>, Object> settings) {
if (value.equals(INDEX_STORE_HYBRID_MMAP_EXTENSIONS.getDefault(Settings.EMPTY)) == false) {
final List<String> nioExtensions = (List<String>) settings.get(INDEX_STORE_HYBRID_NIO_EXTENSIONS);
final List<String> 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<Setting<?>> settings() {
return List.<Setting<?>>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 <a href="https://lucene.apache.org/core/9_5_0/core/org/apache/lucene/codecs/lucene95/package-summary.html#file-names">Lucene File Extensions</a>.
*/
Expand All @@ -226,35 +181,6 @@ public Iterator<Setting<?>> settings() {
"vem"
),
Function.identity(),
new Setting.Validator<List<String>>() {

@Override
public void validate(final List<String> value) {}

@Override
public void validate(final List<String> value, final Map<Setting<?>, Object> settings) {
if (value.equals(INDEX_STORE_HYBRID_NIO_EXTENSIONS.getDefault(Settings.EMPTY)) == false) {
final List<String> mmapExtensions = (List<String>) settings.get(INDEX_STORE_HYBRID_MMAP_EXTENSIONS);
final List<String> 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<Setting<?>> settings() {
return List.<Setting<?>>of(INDEX_STORE_HYBRID_MMAP_EXTENSIONS).iterator();
}
},
Property.IndexScope,
Property.NodeScope
);
Expand Down
16 changes: 1 addition & 15 deletions server/src/main/java/org/opensearch/index/store/FsDirectoryFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> nioExtensions;
final Set<String> 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<String> 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);
Expand Down
79 changes: 2 additions & 77 deletions server/src/test/java/org/opensearch/index/store/FsDirectoryFactoryTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand All @@ -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"));
Expand All @@ -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")
Expand All @@ -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 {
Expand Down

0 comments on commit a18b958

Please sign in to comment.