Update MMapDirectory to use ADVISE_BY_CONTEXT rather than default (opensearch-project#21031)

Signed-off-by: Navneet Verma <navneev@amazon.com>

Author: navneet1v

CHANGELOG.md

@@ -39,6 +39,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - Move pull-based ingestion classes from experimental to publicAPI ([#20704](https://github.com/opensearch-project/OpenSearch/pull/20704))
 - Lazy init stored field reader in SourceLookup ([#20827](https://github.com/opensearch-project/OpenSearch/pull/20827))
 * Improved error message when trying to open an index originally created with Elasticsearch on OpenSearch ([#20512](https://github.com/opensearch-project/OpenSearch/pull/20512))
+- Updated MMapDirectory to use ReadAdviseByContext rather than default readadvise of Lucene([#21031](https://github.com/opensearch-project/OpenSearch/pull/21031))
 
 ### Fixed
 - Relax index template pattern overlap check to use minimum-string heuristic, allowing distinguishable multi-wildcard patterns at the same priority ([#20702](https://github.com/opensearch-project/OpenSearch/pull/20702))

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

@@ -100,12 +100,17 @@ public Directory newFSDirectory(Path location, LockFactory lockFactory, IndexSet
                 final FSDirectory primaryDirectory = FSDirectory.open(location, lockFactory);
                 final Set<String> nioExtensions = new HashSet<>(indexSettings.getValue(IndexModule.INDEX_STORE_HYBRID_NIO_EXTENSIONS));
                 if (primaryDirectory instanceof MMapDirectory mMapDirectory) {
+                    // Setting the read advise by context: REF: https://github.com/opensearch-project/OpenSearch/issues/21012
+                    mMapDirectory.setReadAdvice(MMapDirectory.ADVISE_BY_CONTEXT);
                     return new HybridDirectory(lockFactory, setPreload(mMapDirectory, preLoadExtensions), nioExtensions);
                 } else {
                     return primaryDirectory;
                 }
             case MMAPFS:
-                return setPreload(new MMapDirectory(location, lockFactory), preLoadExtensions);
+                final MMapDirectory mMapDirectory = new MMapDirectory(location, lockFactory);
+                // Setting the read advise by context: REF: https://github.com/opensearch-project/OpenSearch/issues/21012
+                mMapDirectory.setReadAdvice(MMapDirectory.ADVISE_BY_CONTEXT);
+                return setPreload(mMapDirectory, preLoadExtensions);
             // simplefs was removed in Lucene 9; support for enum is maintained for bwc
             case SIMPLEFS:
             case NIOFS:

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

@@ -31,14 +31,18 @@
 
 package org.opensearch.index.store;
 
+import org.apache.lucene.store.DataAccessHint;
 import org.apache.lucene.store.Directory;
 import org.apache.lucene.store.IOContext;
 import org.apache.lucene.store.MMapDirectory;
+import org.apache.lucene.store.MergeInfo;
 import org.apache.lucene.store.NIOFSDirectory;
 import org.apache.lucene.store.NoLockFactory;
+import org.apache.lucene.store.ReadAdvice;
 import org.apache.lucene.util.Constants;
 import org.opensearch.Version;
 import org.opensearch.cluster.metadata.IndexMetadata;
+import org.opensearch.common.SuppressForbidden;
 import org.opensearch.common.settings.Settings;
 import org.opensearch.core.index.Index;
 import org.opensearch.core.index.shard.ShardId;
@@ -49,11 +53,14 @@
 import org.opensearch.test.OpenSearchTestCase;
 
 import java.io.IOException;
+import java.lang.reflect.Field;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.Arrays;
 import java.util.Locale;
+import java.util.Optional;
 import java.util.Set;
+import java.util.function.BiFunction;
 import java.util.function.BiPredicate;
 
 import static org.opensearch.test.store.MockFSDirectoryFactory.FILE_SYSTEM_BASED_STORE_TYPES;
@@ -173,6 +180,7 @@ private void doTestStoreDirectory(Path tempDir, String typeSettingValue, IndexMo
             switch (type) {
                 case HYBRIDFS:
                     assertTrue(FsDirectoryFactory.isHybridFs(directory));
+                    mMapDirectoryHasReadAdviceByContext(((FsDirectoryFactory.HybridDirectory) directory).getDelegate());
                     break;
                 // simplefs was removed in Lucene 9; support for enum is maintained for bwc
                 case SIMPLEFS:
@@ -181,6 +189,7 @@ private void doTestStoreDirectory(Path tempDir, String typeSettingValue, IndexMo
                     break;
                 case MMAPFS:
                     assertTrue(type + " " + directory.toString(), directory instanceof MMapDirectory);
+                    mMapDirectoryHasReadAdviceByContext((MMapDirectory) directory);
                     break;
                 case FS:
                     if (Constants.JRE_IS_64BIT) {
@@ -194,4 +203,52 @@ private void doTestStoreDirectory(Path tempDir, String typeSettingValue, IndexMo
             }
         }
     }
+
+    @SuppressForbidden(reason = "Need to check the readAdvise as there is no getter on read advise")
+    private void mMapDirectoryHasReadAdviceByContext(MMapDirectory mapDirectory) {
+        try {
+            @SuppressWarnings("unchecked")
+            BiFunction<String, IOContext, Optional<ReadAdvice>> readAdvice = (BiFunction<
+                String,
+                IOContext,
+                Optional<ReadAdvice>>) getReadAdviceField(mapDirectory);
+
+            // Verify the function behaves identically to ADVISE_BY_CONTEXT
+            // ADVISE_BY_CONTEXT returns the ReadAdvice from the IOContext
+            assertEquals(
+                "Advise By context is not set",
+                MMapDirectory.ADVISE_BY_CONTEXT.apply("test.dvd", IOContext.DEFAULT),
+                readAdvice.apply("test.dvd", IOContext.DEFAULT)
+            );
+            assertEquals(
+                "Advise By context is not set",
+                MMapDirectory.ADVISE_BY_CONTEXT.apply("test.tim", IOContext.READONCE),
+                readAdvice.apply("test.tim", IOContext.READONCE)
+            );
+            MergeInfo mergeInfo = new MergeInfo(100, 100L, false, 1);
+            assertEquals(
+                "Advise By context is not set",
+                MMapDirectory.ADVISE_BY_CONTEXT.apply("test.vec", IOContext.merge(mergeInfo)),
+                readAdvice.apply("test.vec", IOContext.merge(mergeInfo).withHints())
+            );
+
+            assertEquals(
+                "Advise By context is not set",
+                MMapDirectory.ADVISE_BY_CONTEXT.apply("test.vec", IOContext.DEFAULT.withHints(DataAccessHint.RANDOM)),
+                readAdvice.apply("test.vec", IOContext.DEFAULT.withHints(DataAccessHint.RANDOM))
+            );
+
+        } catch (Exception e) {
+            throw new RuntimeException("Failed to verify read advice is set to ADVISE_BY_CONTEXT: ", e);
+        }
+
+    }
+
+    @SuppressForbidden(reason = "need reflection to access private readAdvice field for testing")
+    private Object getReadAdviceField(MMapDirectory mMapDirectory) throws Exception {
+        Field readAdviceField = MMapDirectory.class.getDeclaredField("readAdvice");
+        readAdviceField.setAccessible(true);
+        return readAdviceField.get(mMapDirectory);
+    }
+
 }