From a2676b1b26aab5a8bb655031f2ce0299e5207e00 Mon Sep 17 00:00:00 2001
From: Adrien Grand <jpountz@gmail.com>
Date: Thu, 4 Apr 2024 16:45:41 +0200
Subject: [PATCH] Use ReadAdvice.RANDOM by default. (#13244)

This switches the default `ReadAdvice` from `NORMAL` to `RANDOM`, which is a better fit for the kind of access pattern that Lucene has. This is expected to reduce page cache trashing and contention on the page table.

`NORMAL` is still available, but never used by any of the file formats.
---
 lucene/CHANGES.txt                                  |  9 ++++++++-
 .../src/java/org/apache/lucene/store/IOContext.java | 13 +++++--------
 .../org/apache/lucene/store/TestMMapDirectory.java  |  6 +++---
 3 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt
index b381ec3d218..c79579a61e5 100644
--- a/lucene/CHANGES.txt
+++ b/lucene/CHANGES.txt
@@ -157,7 +157,14 @@ Bug Fixes
 
 * GITHUB#12878: Fix the declared Exceptions of Expression#evaluate() to match those
   of DoubleValues#doubleValue(). (Uwe Schindler)
-  
+
+Changes in Runtime Behavior
+---------------------
+
+* GITHUB#13244: IOContext now uses ReadAdvice#RANDOM by default for read
+  operations. An implication is that `MMapDirectory` will use POSIX_MADV_RANDOM
+  on POSIX systems. (Adrien Grand)
+
 Changes in Backwards Compatibility Policy
 -----------------------------------------
 
diff --git a/lucene/core/src/java/org/apache/lucene/store/IOContext.java b/lucene/core/src/java/org/apache/lucene/store/IOContext.java
index 459e9bf38b5..5b0d541dae9 100644
--- a/lucene/core/src/java/org/apache/lucene/store/IOContext.java
+++ b/lucene/core/src/java/org/apache/lucene/store/IOContext.java
@@ -49,7 +49,7 @@ public record IOContext(
    * A default context for normal reads/writes. Use {@link #withReadAdvice(ReadAdvice)} to specify
    * another {@link ReadAdvice}.
    */
-  public static final IOContext DEFAULT = new IOContext(ReadAdvice.NORMAL);
+  public static final IOContext DEFAULT = new IOContext(ReadAdvice.RANDOM);
 
   /** A default context for reads with {@link ReadAdvice#SEQUENTIAL}. */
   public static final IOContext READONCE = new IOContext(ReadAdvice.SEQUENTIAL);
@@ -67,13 +67,10 @@ public record IOContext(
       case FLUSH -> Objects.requireNonNull(
           flushInfo, "flushInfo must not be null if context is FLUSH");
     }
-    if (context == Context.MERGE && readAdvice != ReadAdvice.SEQUENTIAL) {
+    if ((context == Context.FLUSH || context == Context.MERGE)
+        && readAdvice != ReadAdvice.SEQUENTIAL) {
       throw new IllegalArgumentException(
-          "The MERGE context must use the SEQUENTIAL read access advice");
-    }
-    if (context == Context.FLUSH && readAdvice != ReadAdvice.NORMAL) {
-      throw new IllegalArgumentException(
-          "The FLUSH context must use the NORMAL read access advice");
+          "The FLUSH and MERGE contexts must use the SEQUENTIAL read access advice");
     }
   }
 
@@ -84,7 +81,7 @@ public record IOContext(
 
   /** Creates an {@link IOContext} for flushing. */
   public IOContext(FlushInfo flushInfo) {
-    this(Context.FLUSH, null, flushInfo, ReadAdvice.NORMAL);
+    this(Context.FLUSH, null, flushInfo, ReadAdvice.SEQUENTIAL);
   }
 
   /** Creates an {@link IOContext} for merging. */
diff --git a/lucene/core/src/test/org/apache/lucene/store/TestMMapDirectory.java b/lucene/core/src/test/org/apache/lucene/store/TestMMapDirectory.java
index b13f4f3cf34..39d3dbda9ac 100644
--- a/lucene/core/src/test/org/apache/lucene/store/TestMMapDirectory.java
+++ b/lucene/core/src/test/org/apache/lucene/store/TestMMapDirectory.java
@@ -98,8 +98,8 @@ public class TestMMapDirectory extends BaseDirectoryTestCase {
         MMapDirectory.supportsMadvise());
   }
 
-  // Opens the input with IOContext.RANDOM to ensure basic code path coverage for POSIX_MADV_RANDOM.
-  public void testWithRandom() throws Exception {
+  // Opens the input with ReadAdvice.NORMAL to ensure basic code path coverage.
+  public void testWithNormal() throws Exception {
     final int size = 8 * 1024;
     byte[] bytes = new byte[size];
     random().nextBytes(bytes);
@@ -110,7 +110,7 @@ public class TestMMapDirectory extends BaseDirectoryTestCase {
       }
 
       try (final IndexInput in =
-          dir.openInput("test", IOContext.DEFAULT.withReadAdvice(ReadAdvice.RANDOM))) {
+          dir.openInput("test", IOContext.DEFAULT.withReadAdvice(ReadAdvice.NORMAL))) {
         final byte[] readBytes = new byte[size];
         in.readBytes(readBytes, 0, readBytes.length);
         assertArrayEquals(bytes, readBytes);