[9.0.x] Revert "Enable madvise by default for all builds (elastic#110159)" (elastic#127921) (elastic#127961)

9.x port of: Revert "Enable madvise by default for all builds (elastic#110159)" elastic#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.

Author: ChrisHegarty

docs/changelog/127921.yaml

@@ -0,0 +1,5 @@
+pr: 127921
+summary: "[9.x] Revert \"Enable madvise by default for all builds\""
+area: Vector Search
+type: bug
+issues: []

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

@@ -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;
@@ -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;
@@ -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);
@@ -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
      */

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

@@ -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;
@@ -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;
@@ -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");