Put MADV_RANDOM behind a feature flag.

This puts `MADV_RANDOM` behind a feature flag to decouple this new feature from Lucene from the Lucene upgrade itself. As a feature flag, snapshot builds will have support for `MADV_RANDOM` (unless explicitly disabled) and release builds will not support `MADV_RANDOM` (unless explicitly enabled).
jpountz · Apr 9, 2024 · 899f86c · 899f86c
1 parent b23f7d0
commit 899f86c
Show file tree

Hide file tree

Showing 2 changed files with 57 additions and 2 deletions.
diff --git a/server/src/main/java/org/elasticsearch/index/store/FsDirectoryFactory.java b/server/src/main/java/org/elasticsearch/index/store/FsDirectoryFactory.java
@@ -21,6 +21,7 @@
 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;
@@ -35,6 +36,8 @@
 
 public class FsDirectoryFactory implements IndexStorePlugin.DirectoryFactory {
 
+    public 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;
@@ -66,12 +69,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, lockFactory, preLoadExtensions));
+                    Directory dir = new HybridDirectory(lockFactory, setPreload(mMapDirectory, lockFactory, preLoadExtensions));
+                    if (MADV_RANDOM_FEATURE_FLAG.isEnabled() == false) {
+                        dir = disableRandomAdvice(dir);
+                    }
+                    return dir;
                 } else {
                     return primaryDirectory;
                 }
             case MMAPFS:
-                return setPreload(new MMapDirectory(location, lockFactory), lockFactory, preLoadExtensions);
+                Directory dir = setPreload(new MMapDirectory(location, lockFactory), lockFactory, preLoadExtensions);
+                if (MADV_RANDOM_FEATURE_FLAG.isEnabled() == false) {
+                    dir = disableRandomAdvice(dir);
+                }
+                return dir;
             case SIMPLEFS:
             case NIOFS:
                 return new NIOFSDirectory(location, lockFactory);
@@ -93,6 +104,23 @@ public static MMapDirectory setPreload(MMapDirectory mMapDirectory, LockFactory
         return mMapDirectory;
     }
 
+    /**
+     * Return a {@link FilterDirectory} around the provided {@link Directory} that forcefully disables {@link IOContext#RANDOM random
+     * access}.
+     */
+    static Directory disableRandomAdvice(Directory dir) {
+        return new FilterDirectory(dir) {
+            @Override
+            public IndexInput openInput(String name, IOContext context) throws IOException {
+                if (context.randomAccess) {
+                    context = IOContext.READ;
+                }
+                assert context.randomAccess == false;
+                return super.openInput(name, context);
+            }
+        };
+    }
+
     /**
      * Returns true iff the directory is a hybrid fs directory
      */

diff --git a/server/src/test/java/org/elasticsearch/index/store/FsDirectoryFactoryTests.java b/server/src/test/java/org/elasticsearch/index/store/FsDirectoryFactoryTests.java
@@ -8,8 +8,12 @@
 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;
@@ -65,6 +69,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.randomAccess);
+                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.RANDOM));
+
+        // The wrapped directory shouldn't fail regardless of the IOContext
+        for (IOContext context : Arrays.asList(IOContext.READ, IOContext.DEFAULT, IOContext.READONCE, IOContext.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");