Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions docs/changelog/127921.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pr: 127921
summary: "[9.x] Revert \"Enable madvise by default for all builds\""
area: Vector Search
type: bug
issues: []
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,11 @@
import org.apache.lucene.store.MMapDirectory;
import org.apache.lucene.store.NIOFSDirectory;
import org.apache.lucene.store.NativeFSLockFactory;
import org.apache.lucene.store.ReadAdvice;
import org.apache.lucene.store.SimpleFSLockFactory;
import org.elasticsearch.common.settings.Setting;
import org.elasticsearch.common.settings.Setting.Property;
import org.elasticsearch.common.util.FeatureFlag;
import org.elasticsearch.core.IOUtils;
import org.elasticsearch.index.IndexModule;
import org.elasticsearch.index.IndexSettings;
Expand All @@ -37,6 +39,8 @@

public class FsDirectoryFactory implements IndexStorePlugin.DirectoryFactory {

private static final FeatureFlag MADV_RANDOM_FEATURE_FLAG = new FeatureFlag("madv_random");

public static final Setting<LockFactory> INDEX_LOCK_FACTOR_SETTING = new Setting<>("index.store.fs.fs_lock", "native", (s) -> {
return switch (s) {
case "native" -> NativeFSLockFactory.INSTANCE;
Expand Down Expand Up @@ -68,12 +72,20 @@ protected Directory newFSDirectory(Path location, LockFactory lockFactory, Index
// Use Lucene defaults
final FSDirectory primaryDirectory = FSDirectory.open(location, lockFactory);
if (primaryDirectory instanceof MMapDirectory mMapDirectory) {
return new HybridDirectory(lockFactory, setPreload(mMapDirectory, preLoadExtensions));
Directory dir = new HybridDirectory(lockFactory, setPreload(mMapDirectory, preLoadExtensions));
if (MADV_RANDOM_FEATURE_FLAG.isEnabled() == false) {
dir = disableRandomAdvice(dir);
}
return dir;
} else {
return primaryDirectory;
}
case MMAPFS:
return setPreload(new MMapDirectory(location, lockFactory), preLoadExtensions);
Directory dir = setPreload(new MMapDirectory(location, lockFactory), preLoadExtensions);
if (MADV_RANDOM_FEATURE_FLAG.isEnabled() == false) {
dir = disableRandomAdvice(dir);
}
return dir;
case SIMPLEFS:
case NIOFS:
return new NIOFSDirectory(location, lockFactory);
Expand Down Expand Up @@ -101,6 +113,23 @@ static BiPredicate<String, IOContext> getPreloadFunc(Set<String> preLoadExtensio
return MMapDirectory.NO_FILES;
}

/**
* Return a {@link FilterDirectory} around the provided {@link Directory} that forcefully disables {@link IOContext#readAdvice random
* access}.
*/
static Directory disableRandomAdvice(Directory dir) {
return new FilterDirectory(dir) {
@Override
public IndexInput openInput(String name, IOContext context) throws IOException {
if (context.readAdvice() == ReadAdvice.RANDOM) {
context = context.withReadAdvice(ReadAdvice.NORMAL);
}
assert context.readAdvice() != ReadAdvice.RANDOM;
return super.openInput(name, context);
}
};
}

/**
* Returns true iff the directory is a hybrid fs directory
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,16 @@
package org.elasticsearch.index.store;

import org.apache.lucene.store.AlreadyClosedException;
import org.apache.lucene.store.ByteBuffersDirectory;
import org.apache.lucene.store.Directory;
import org.apache.lucene.store.FilterDirectory;
import org.apache.lucene.store.IOContext;
import org.apache.lucene.store.IndexInput;
import org.apache.lucene.store.IndexOutput;
import org.apache.lucene.store.MMapDirectory;
import org.apache.lucene.store.NIOFSDirectory;
import org.apache.lucene.store.NoLockFactory;
import org.apache.lucene.store.ReadAdvice;
import org.apache.lucene.store.SleepingLockWrapper;
import org.apache.lucene.util.Constants;
import org.elasticsearch.cluster.metadata.IndexMetadata;
Expand All @@ -34,6 +38,7 @@
import java.nio.file.Path;
import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Set;
Expand Down Expand Up @@ -74,6 +79,29 @@ public void testPreload() throws IOException {
}
}

public void testDisableRandomAdvice() throws IOException {
Directory dir = new FilterDirectory(new ByteBuffersDirectory()) {
@Override
public IndexInput openInput(String name, IOContext context) throws IOException {
assertFalse(context.readAdvice() == ReadAdvice.RANDOM);
return super.openInput(name, context);
}
};
Directory noRandomAccessDir = FsDirectoryFactory.disableRandomAdvice(dir);
try (IndexOutput out = noRandomAccessDir.createOutput("foo", IOContext.DEFAULT)) {
out.writeInt(42);
}
// Test the tester
expectThrows(AssertionError.class, () -> dir.openInput("foo", IOContext.DEFAULT.withReadAdvice(ReadAdvice.RANDOM)));

// The wrapped directory shouldn't fail regardless of the IOContext
for (IOContext context : List.of(IOContext.DEFAULT, IOContext.READONCE, IOContext.DEFAULT.withReadAdvice(ReadAdvice.RANDOM))) {
try (IndexInput in = noRandomAccessDir.openInput("foo", context)) {
assertEquals(42, in.readInt());
}
}
}

private Directory newDirectory(Settings settings) throws IOException {
IndexSettings idxSettings = IndexSettingsModule.newIndexSettings("foo", settings);
Path tempDir = createTempDir().resolve(idxSettings.getUUID()).resolve("0");
Expand Down