Skip to content

Commit dabea23

Browse files
authored
Revert "Enable madvise by default for all builds (#110159)" (#126308)
This reverts commit 4a77e06. We've seen a significant performance degradation in merging vectors resulting from the use of MADV_RANDOM and MGLRU ( and LRU in recent Linux kernels ) For the 8.x release train, then we will revert the change that enabled MADV_RANDOM. And backport to all shipping 8.x bugfix releases. relates: #124499
1 parent 43b10cf commit dabea23

File tree

2 files changed

+56
-2
lines changed

2 files changed

+56
-2
lines changed

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

+30-2
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import org.apache.lucene.store.SimpleFSLockFactory;
2323
import org.elasticsearch.common.settings.Setting;
2424
import org.elasticsearch.common.settings.Setting.Property;
25+
import org.elasticsearch.common.util.FeatureFlag;
2526
import org.elasticsearch.core.IOUtils;
2627
import org.elasticsearch.index.IndexModule;
2728
import org.elasticsearch.index.IndexSettings;
@@ -36,6 +37,8 @@
3637

3738
public class FsDirectoryFactory implements IndexStorePlugin.DirectoryFactory {
3839

40+
private static final FeatureFlag MADV_RANDOM_FEATURE_FLAG = new FeatureFlag("madv_random");
41+
3942
public static final Setting<LockFactory> INDEX_LOCK_FACTOR_SETTING = new Setting<>("index.store.fs.fs_lock", "native", (s) -> {
4043
return switch (s) {
4144
case "native" -> NativeFSLockFactory.INSTANCE;
@@ -67,12 +70,20 @@ protected Directory newFSDirectory(Path location, LockFactory lockFactory, Index
6770
// Use Lucene defaults
6871
final FSDirectory primaryDirectory = FSDirectory.open(location, lockFactory);
6972
if (primaryDirectory instanceof MMapDirectory mMapDirectory) {
70-
return new HybridDirectory(lockFactory, setPreload(mMapDirectory, lockFactory, preLoadExtensions));
73+
Directory dir = new HybridDirectory(lockFactory, setPreload(mMapDirectory, lockFactory, preLoadExtensions));
74+
if (MADV_RANDOM_FEATURE_FLAG.isEnabled() == false) {
75+
dir = disableRandomAdvice(dir);
76+
}
77+
return dir;
7178
} else {
7279
return primaryDirectory;
7380
}
7481
case MMAPFS:
75-
return setPreload(new MMapDirectory(location, lockFactory), lockFactory, preLoadExtensions);
82+
Directory dir = setPreload(new MMapDirectory(location, lockFactory), lockFactory, preLoadExtensions);
83+
if (MADV_RANDOM_FEATURE_FLAG.isEnabled() == false) {
84+
dir = disableRandomAdvice(dir);
85+
}
86+
return dir;
7687
case SIMPLEFS:
7788
case NIOFS:
7889
return new NIOFSDirectory(location, lockFactory);
@@ -94,6 +105,23 @@ public static MMapDirectory setPreload(MMapDirectory mMapDirectory, LockFactory
94105
return mMapDirectory;
95106
}
96107

108+
/**
109+
* Return a {@link FilterDirectory} around the provided {@link Directory} that forcefully disables {@link IOContext#RANDOM random
110+
* access}.
111+
*/
112+
static Directory disableRandomAdvice(Directory dir) {
113+
return new FilterDirectory(dir) {
114+
@Override
115+
public IndexInput openInput(String name, IOContext context) throws IOException {
116+
if (context.randomAccess) {
117+
context = IOContext.READ;
118+
}
119+
assert context.randomAccess == false;
120+
return super.openInput(name, context);
121+
}
122+
};
123+
}
124+
97125
/**
98126
* Returns true iff the directory is a hybrid fs directory
99127
*/

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

+26
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,12 @@
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;
@@ -67,6 +70,29 @@ public void testPreload() throws IOException {
6770
}
6871
}
6972

73+
public void testDisableRandomAdvice() throws IOException {
74+
Directory dir = new FilterDirectory(new ByteBuffersDirectory()) {
75+
@Override
76+
public IndexInput openInput(String name, IOContext context) throws IOException {
77+
assertFalse(context.randomAccess);
78+
return super.openInput(name, context);
79+
}
80+
};
81+
Directory noRandomAccessDir = FsDirectoryFactory.disableRandomAdvice(dir);
82+
try (IndexOutput out = noRandomAccessDir.createOutput("foo", IOContext.DEFAULT)) {
83+
out.writeInt(42);
84+
}
85+
// Test the tester
86+
expectThrows(AssertionError.class, () -> dir.openInput("foo", IOContext.RANDOM));
87+
88+
// The wrapped directory shouldn't fail regardless of the IOContext
89+
for (IOContext context : Arrays.asList(IOContext.READ, IOContext.DEFAULT, IOContext.READONCE, IOContext.RANDOM)) {
90+
try (IndexInput in = noRandomAccessDir.openInput("foo", context)) {
91+
assertEquals(42, in.readInt());
92+
}
93+
}
94+
}
95+
7096
private Directory newDirectory(Settings settings) throws IOException {
7197
IndexSettings idxSettings = IndexSettingsModule.newIndexSettings("foo", settings);
7298
Path tempDir = createTempDir().resolve(idxSettings.getUUID()).resolve("0");

0 commit comments

Comments
 (0)