Skip to content

Commit 1ed0278

Browse files
authored
[9.x] Revert "Enable madvise by default for all builds (#110159)" (#127921)
9.x port of: Revert "Enable madvise by default for all builds (#110159)" #126308 This change did not apply cleanly. In fact this is not strictly a revert, since the change was never actually in 9.x post the Lucene 10 upgrade. However, the semantics of the change still apply - avoid RANDOM everywhere. Even though in 9.x we do set -Dorg.apache.lucene.store.defaultReadAdvice=normal, it is not enough to avoid RANDOM when random is explicitly requested by code.
1 parent a0b0aca commit 1ed0278

File tree

3 files changed

+63
-2
lines changed

3 files changed

+63
-2
lines changed

docs/changelog/127921.yaml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
pr: 127921
2+
summary: "[9.x] Revert \"Enable madvise by default for all builds\""
3+
area: Vector Search
4+
type: bug
5+
issues: []

server/src/main/java/org/elasticsearch/index/store/FsDirectoryFactory.java

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,11 @@
2020
import org.apache.lucene.store.MMapDirectory;
2121
import org.apache.lucene.store.NIOFSDirectory;
2222
import org.apache.lucene.store.NativeFSLockFactory;
23+
import org.apache.lucene.store.ReadAdvice;
2324
import org.apache.lucene.store.SimpleFSLockFactory;
2425
import org.elasticsearch.common.settings.Setting;
2526
import org.elasticsearch.common.settings.Setting.Property;
27+
import org.elasticsearch.common.util.FeatureFlag;
2628
import org.elasticsearch.core.IOUtils;
2729
import org.elasticsearch.index.IndexModule;
2830
import org.elasticsearch.index.IndexSettings;
@@ -43,6 +45,7 @@
4345
public class FsDirectoryFactory implements IndexStorePlugin.DirectoryFactory {
4446

4547
private static final Logger Log = LogManager.getLogger(FsDirectoryFactory.class);
48+
private static final FeatureFlag MADV_RANDOM_FEATURE_FLAG = new FeatureFlag("madv_random");
4649

4750
public static final Setting<LockFactory> INDEX_LOCK_FACTOR_SETTING = new Setting<>("index.store.fs.fs_lock", "native", (s) -> {
4851
return switch (s) {
@@ -75,12 +78,20 @@ protected Directory newFSDirectory(Path location, LockFactory lockFactory, Index
7578
// Use Lucene defaults
7679
final FSDirectory primaryDirectory = FSDirectory.open(location, lockFactory);
7780
if (primaryDirectory instanceof MMapDirectory mMapDirectory) {
78-
return new HybridDirectory(lockFactory, setPreload(mMapDirectory, preLoadExtensions));
81+
Directory dir = new HybridDirectory(lockFactory, setPreload(mMapDirectory, preLoadExtensions));
82+
if (MADV_RANDOM_FEATURE_FLAG.isEnabled() == false) {
83+
dir = disableRandomAdvice(dir);
84+
}
85+
return dir;
7986
} else {
8087
return primaryDirectory;
8188
}
8289
case MMAPFS:
83-
return setPreload(new MMapDirectory(location, lockFactory), preLoadExtensions);
90+
Directory dir = setPreload(new MMapDirectory(location, lockFactory), preLoadExtensions);
91+
if (MADV_RANDOM_FEATURE_FLAG.isEnabled() == false) {
92+
dir = disableRandomAdvice(dir);
93+
}
94+
return dir;
8495
case SIMPLEFS:
8596
case NIOFS:
8697
return new NIOFSDirectory(location, lockFactory);
@@ -108,6 +119,23 @@ static BiPredicate<String, IOContext> getPreloadFunc(Set<String> preLoadExtensio
108119
return MMapDirectory.NO_FILES;
109120
}
110121

122+
/**
123+
* Return a {@link FilterDirectory} around the provided {@link Directory} that forcefully disables {@link IOContext#readAdvice random
124+
* access}.
125+
*/
126+
static Directory disableRandomAdvice(Directory dir) {
127+
return new FilterDirectory(dir) {
128+
@Override
129+
public IndexInput openInput(String name, IOContext context) throws IOException {
130+
if (context.readAdvice() == ReadAdvice.RANDOM) {
131+
context = context.withReadAdvice(ReadAdvice.NORMAL);
132+
}
133+
assert context.readAdvice() != ReadAdvice.RANDOM;
134+
return super.openInput(name, context);
135+
}
136+
};
137+
}
138+
111139
/**
112140
* Returns true iff the directory is a hybrid fs directory
113141
*/

server/src/test/java/org/elasticsearch/index/store/FsDirectoryFactoryTests.java

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,16 @@
99
package org.elasticsearch.index.store;
1010

1111
import org.apache.lucene.store.AlreadyClosedException;
12+
import org.apache.lucene.store.ByteBuffersDirectory;
1213
import org.apache.lucene.store.Directory;
1314
import org.apache.lucene.store.FilterDirectory;
1415
import org.apache.lucene.store.IOContext;
16+
import org.apache.lucene.store.IndexInput;
17+
import org.apache.lucene.store.IndexOutput;
1518
import org.apache.lucene.store.MMapDirectory;
1619
import org.apache.lucene.store.NIOFSDirectory;
1720
import org.apache.lucene.store.NoLockFactory;
21+
import org.apache.lucene.store.ReadAdvice;
1822
import org.apache.lucene.store.SleepingLockWrapper;
1923
import org.apache.lucene.util.Constants;
2024
import org.elasticsearch.cluster.metadata.IndexMetadata;
@@ -34,6 +38,7 @@
3438
import java.nio.file.Path;
3539
import java.util.Arrays;
3640
import java.util.HashMap;
41+
import java.util.List;
3742
import java.util.Locale;
3843
import java.util.Map;
3944
import java.util.Set;
@@ -74,6 +79,29 @@ public void testPreload() throws IOException {
7479
}
7580
}
7681

82+
public void testDisableRandomAdvice() throws IOException {
83+
Directory dir = new FilterDirectory(new ByteBuffersDirectory()) {
84+
@Override
85+
public IndexInput openInput(String name, IOContext context) throws IOException {
86+
assertFalse(context.readAdvice() == ReadAdvice.RANDOM);
87+
return super.openInput(name, context);
88+
}
89+
};
90+
Directory noRandomAccessDir = FsDirectoryFactory.disableRandomAdvice(dir);
91+
try (IndexOutput out = noRandomAccessDir.createOutput("foo", IOContext.DEFAULT)) {
92+
out.writeInt(42);
93+
}
94+
// Test the tester
95+
expectThrows(AssertionError.class, () -> dir.openInput("foo", IOContext.DEFAULT.withReadAdvice(ReadAdvice.RANDOM)));
96+
97+
// The wrapped directory shouldn't fail regardless of the IOContext
98+
for (IOContext context : List.of(IOContext.DEFAULT, IOContext.READONCE, IOContext.DEFAULT.withReadAdvice(ReadAdvice.RANDOM))) {
99+
try (IndexInput in = noRandomAccessDir.openInput("foo", context)) {
100+
assertEquals(42, in.readInt());
101+
}
102+
}
103+
}
104+
77105
private Directory newDirectory(Settings settings) throws IOException {
78106
IndexSettings idxSettings = IndexSettingsModule.newIndexSettings("foo", settings);
79107
Path tempDir = createTempDir().resolve(idxSettings.getUUID()).resolve("0");

0 commit comments

Comments
 (0)