Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove mmap.extensions setting #9392

Merged
merged 2 commits into from
Jul 29, 2024
Merged

Remove mmap.extensions setting #9392

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bc0d3dd
Remove mmap.extensions setting in favor of nio.extensions
dzane17 Jul 29, 2024
4c59590
Update CHANGELOG-3.0.md
dzane17 Jul 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG-3.0.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
- Remove LegacyESVersion.V_7_10_ Constants ([#5018](https://github.com/opensearch-project/OpenSearch/pull/5018))
- Remove Version.V_1_ Constants ([#5021](https://github.com/opensearch-project/OpenSearch/pull/5021))
- Remove custom Map, List and Set collection classes ([#6871](https://github.com/opensearch-project/OpenSearch/pull/6871))
- Remove mmap.extensions setting in favor of nio.extensions ([#9392](https://github.com/opensearch-project/OpenSearch/pull/9392))
dzane17 marked this conversation as resolved.
Show resolved Hide resolved

### Fixed
- Fix 'org.apache.hc.core5.http.ParseException: Invalid protocol version' under JDK 16+ ([#4827](https://github.com/opensearch-project/OpenSearch/pull/4827))
Expand Down
1 change: 0 additions & 1 deletion server/src/main/java/org/opensearch/common/settings/IndexScopedSettings.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,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
77 changes: 1 addition & 76 deletions server/src/main/java/org/opensearch/index/IndexModule.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,6 @@
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Objects;
Expand Down Expand Up @@ -183,52 +182,7 @@ public final class IndexModule {
Property.PrivateIndex
);

/** 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_9_0/core/org/apache/lucene/codecs/lucene99/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(
andrross marked this conversation as resolved.
Show resolved Hide resolved
"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_9_0/core/org/apache/lucene/codecs/lucene99/package-summary.html#file-names">Lucene File Extensions</a>.
*/
Expand All @@ -253,35 +207,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
19 changes: 1 addition & 18 deletions server/src/main/java/org/opensearch/index/store/FsDirectoryFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@
import org.apache.lucene.store.SimpleFSLockFactory;
import org.opensearch.common.settings.Setting;
import org.opensearch.common.settings.Setting.Property;
import org.opensearch.common.settings.Settings;
import org.opensearch.common.util.io.IOUtils;
import org.opensearch.index.IndexModule;
import org.opensearch.index.IndexSettings;
Expand All @@ -57,8 +56,6 @@
import java.nio.file.Path;
import java.util.HashSet;
import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.Stream;

/**
* Factory for a filesystem directory
Expand Down Expand Up @@ -100,21 +97,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
Open in desktop
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")
dzane17 marked this conversation as resolved.
Show resolved Hide resolved
.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
Loading