Skip to content

Commit

Permalink
Default to mmapfs within hybridfs (opensearch-project#8508)
Browse files Browse the repository at this point in the history 
* Default to mmapfs within hybridfs

Signed-off-by: David Zane <davizane@amazon.com>

* Add index setting validation func

Signed-off-by: David Zane <davizane@amazon.com>

* Reviewer comments

Signed-off-by: David Zane <davizane@amazon.com>

* Clean up, mmap.extensions validation

Signed-off-by: David Zane <davizane@amazon.com>

* Deprecation flag, build all_ext list

Signed-off-by: David Zane <davizane@amazon.com>

* Make nioExtensions unmodifiable

Signed-off-by: David Zane <davizane@amazon.com>

---------

Signed-off-by: David Zane <davizane@amazon.com>
Signed-off-by: Kaushal Kumar <ravi.kaushal97@gmail.com>
  • Loading branch information
@dzane17 @kaushalmahi12
dzane17 authored and kaushalmahi12 committed Sep 12, 2023
1 parent 9ed5792 commit 1d3943c
Show file tree
Hide file tree
Showing 5 changed files with 222 additions and 13 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
- Bump `aws-actions/configure-aws-credentials` from 1 to 2 ([#9302](https://github.com/opensearch-project/OpenSearch/pull/9302))

### Changed
- Default to mmapfs within hybridfs ([#8508](https://github.com/opensearch-project/OpenSearch/pull/8508))
- Perform aggregation postCollection in ContextIndexSearcher after searching leaves ([#8303](https://github.com/opensearch-project/OpenSearch/pull/8303))
- Make Span exporter configurable ([#8620](https://github.com/opensearch-project/OpenSearch/issues/8620))
- Perform aggregation postCollection in ContextIndexSearcher after searching leaves ([#8303](https://github.com/opensearch-project/OpenSearch/pull/8303))
Expand Down
1 change: 1 addition & 0 deletions server/src/main/java/org/opensearch/common/settings/IndexScopedSettings.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,7 @@ public final class IndexScopedSettings extends AbstractScopedSettings {
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,
FsDirectoryFactory.INDEX_LOCK_FACTOR_SETTING,
Expand Down
97 changes: 95 additions & 2 deletions server/src/main/java/org/opensearch/index/IndexModule.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.Iterator;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.function.BiFunction;
import java.util.function.BooleanSupplier;
Expand Down Expand Up @@ -154,14 +155,106 @@ public final class IndexModule {
Property.NodeScope
);

/** Which lucene file extensions to load with the mmap directory when using hybridfs store.
/** 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_2_0/core/org/apache/lucene/codecs/lucene92/package-summary.html#file-names">Lucene File Extensions</a>.
* @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}.
* 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>.
*/
public static final Setting<List<String>> INDEX_STORE_HYBRID_NIO_EXTENSIONS = Setting.listSetting(
"index.store.hybrid.nio.extensions",
List.of(
"segments_N",
"write.lock",
"si",
"cfe",
"fnm",
"fdx",
"fdt",
"pos",
"pay",
"nvm",
"dvm",
"tvx",
"tvd",
"liv",
"dii",
"vec",
"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
29 changes: 23 additions & 6 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 @@ -44,6 +44,7 @@
import org.apache.lucene.store.NativeFSLockFactory;
import org.apache.lucene.store.SimpleFSLockFactory;
import org.opensearch.common.settings.Setting;
import org.opensearch.common.settings.Settings;
import org.opensearch.common.settings.Setting.Property;
import org.opensearch.common.util.io.IOUtils;
import org.opensearch.index.IndexModule;
Expand All @@ -56,6 +57,8 @@
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 @@ -97,10 +100,24 @@ protected Directory newFSDirectory(Path location, LockFactory lockFactory, Index
case HYBRIDFS:
// Use Lucene defaults
final FSDirectory primaryDirectory = FSDirectory.open(location, lockFactory);
final Set<String> mmapExtensions = new HashSet<>(indexSettings.getValue(IndexModule.INDEX_STORE_HYBRID_MMAP_EXTENSIONS));
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));
}
if (primaryDirectory instanceof MMapDirectory) {
MMapDirectory mMapDirectory = (MMapDirectory) primaryDirectory;
return new HybridDirectory(lockFactory, setPreload(mMapDirectory, lockFactory, preLoadExtensions), mmapExtensions);
return new HybridDirectory(lockFactory, setPreload(mMapDirectory, lockFactory, preLoadExtensions), nioExtensions);
} else {
return primaryDirectory;
}
Expand Down Expand Up @@ -143,12 +160,12 @@ public static boolean isHybridFs(Directory directory) {
*/
static final class HybridDirectory extends NIOFSDirectory {
private final MMapDirectory delegate;
private final Set<String> mmapExtensions;
private final Set<String> nioExtensions;

HybridDirectory(LockFactory lockFactory, MMapDirectory delegate, Set<String> mmapExtensions) throws IOException {
HybridDirectory(LockFactory lockFactory, MMapDirectory delegate, Set<String> nioExtensions) throws IOException {
super(delegate.getDirectory(), lockFactory);
this.delegate = delegate;
this.mmapExtensions = mmapExtensions;
this.nioExtensions = nioExtensions;
}

@Override
Expand All @@ -169,7 +186,7 @@ public IndexInput openInput(String name, IOContext context) throws IOException {

boolean useDelegate(String name) {
final String extension = FileSwitchDirectory.getExtension(name);
return mmapExtensions.contains(extension);
return nioExtensions.contains(extension) == false;
}

@Override
Expand Down
107 changes: 102 additions & 5 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 @@ -72,7 +72,8 @@ public void testPreload() throws IOException {
try (Directory directory = newDirectory(build)) {
assertTrue(FsDirectoryFactory.isHybridFs(directory));
FsDirectoryFactory.HybridDirectory hybridDirectory = (FsDirectoryFactory.HybridDirectory) directory;
// test default hybrid mmap extensions
// test default hybrid extensions
// true->mmap, false->nio
assertTrue(hybridDirectory.useDelegate("foo.nvd"));
assertTrue(hybridDirectory.useDelegate("foo.dvd"));
assertTrue(hybridDirectory.useDelegate("foo.tim"));
Expand All @@ -82,6 +83,7 @@ public void testPreload() throws IOException {
assertTrue(hybridDirectory.useDelegate("foo.kdi"));
assertTrue(hybridDirectory.useDelegate("foo.cfs"));
assertTrue(hybridDirectory.useDelegate("foo.doc"));
assertTrue(hybridDirectory.useDelegate("foo.new"));
assertFalse(hybridDirectory.useDelegate("foo.pos"));
assertFalse(hybridDirectory.useDelegate("foo.pay"));
MMapDirectory delegate = hybridDirectory.getDelegate();
Expand All @@ -94,23 +96,25 @@ 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_MMAP_EXTENSIONS.getKey(), "nvd", "dvd", "tim", "pos", "pay")
.putList(IndexModule.INDEX_STORE_HYBRID_NIO_EXTENSIONS.getKey(), "tip", "dim", "kdd", "kdi", "cfs", "doc")
.build();
try (Directory directory = newDirectory(build)) {
assertTrue(FsDirectoryFactory.isHybridFs(directory));
FsDirectoryFactory.HybridDirectory hybridDirectory = (FsDirectoryFactory.HybridDirectory) directory;
// test custom hybrid mmap extensions
// test custom hybrid nio 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.pay"));
assertTrue(hybridDirectory.useDelegate("foo.new"));
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"));
MMapDirectory delegate = hybridDirectory.getDelegate();
assertThat(delegate, Matchers.instanceOf(FsDirectoryFactory.PreLoadMMapDirectory.class));
FsDirectoryFactory.PreLoadMMapDirectory preLoadMMapDirectory = (FsDirectoryFactory.PreLoadMMapDirectory) delegate;
Expand All @@ -119,6 +123,99 @@ 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")
.putList(IndexModule.INDEX_STORE_HYBRID_NIO_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"));
assertTrue(hybridDirectory.useDelegate("foo.nvd"));
assertTrue(hybridDirectory.useDelegate("foo.dvd"));
assertTrue(hybridDirectory.useDelegate("foo.cfs"));
assertTrue(hybridDirectory.useDelegate("foo.doc"));
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 1d3943c

Please sign in to comment.