[RCI] Add NoOpEngine for closed indices

[tlrx]

Note: this pull request is against the replicated-closed-indices branch

This pull request adds the NoOpEngine implementation reviewed in #31163 and adapts it to the latest Engine changes. It also makes this engine the default for closed indices, so that it will be automatically picked up by IndexService when instantiating the shards of a closed index (which will be done in a another PR).

As a side note, I'd like to use the NoOpEngine for closed index replication as early as possible in order to catch more bugs.

[elasticmachine]

Pinging @elastic/es-distributed

[dnhatn]

@tlrx I've merged a PR which changes the Engine class. You may have to update your PR. Thank you!

[s1monw]

left a detailed comment

[s1monw]

I wonder if we can subclass ReadOnlyEngine here if we do this:

diff --git a/server/src/main/java/org/elasticsearch/index/engine/ReadOnlyEngine.java b/server/src/main/java/org/elasticsearch/index/engine/ReadOnlyEngine.java
index 9f7ceb61474..43a77b7e83e 100644
--- a/server/src/main/java/org/elasticsearch/index/engine/ReadOnlyEngine.java
+++ b/server/src/main/java/org/elasticsearch/index/engine/ReadOnlyEngine.java
@@ -20,7 +20,9 @@ package org.elasticsearch.index.engine;
 
 import org.apache.lucene.index.DirectoryReader;
 import org.apache.lucene.index.IndexCommit;
+import org.apache.lucene.index.IndexReader;
 import org.apache.lucene.index.IndexWriter;
+import org.apache.lucene.index.LeafReader;
 import org.apache.lucene.index.SegmentInfos;
 import org.apache.lucene.index.SoftDeletesDirectoryReaderWrapper;
 import org.apache.lucene.search.IndexSearcher;
@@ -56,7 +58,7 @@ import java.util.stream.Stream;
  *
  * @see #ReadOnlyEngine(EngineConfig, SeqNoStats, TranslogStats, boolean, Function)
  */
-public final class ReadOnlyEngine extends Engine {
+public class ReadOnlyEngine extends Engine {
 
     private final SegmentInfos lastCommittedSegmentInfos;
     private final SeqNoStats seqNoStats;
@@ -95,7 +97,7 @@ public final class ReadOnlyEngine extends Engine {
                 this.lastCommittedSegmentInfos = Lucene.readSegmentInfos(directory);
                 this.translogStats = translogStats == null ? new TranslogStats(0, 0, 0, 0, 0) : translogStats;
                 this.seqNoStats = seqNoStats == null ? buildSeqNoStats(lastCommittedSegmentInfos) : seqNoStats;
-                reader = ElasticsearchDirectoryReader.wrap(DirectoryReader.open(directory), config.getShardId());
+                reader = ElasticsearchDirectoryReader.wrap(open(directory), config.getShardId());
                 if (config.getIndexSettings().isSoftDeleteEnabled()) {
                     reader = new SoftDeletesDirectoryReaderWrapper(reader, Lucene.SOFT_DELETES_FIELD);
                 }
@@ -116,6 +118,10 @@ public final class ReadOnlyEngine extends Engine {
         }
     }
 
+    protected DirectoryReader open(Directory directory) throws IOException {
+        return DirectoryReader.open(directory);
+    }
+

and then in the NoOpEngine we can simply override the methods that should throw UOE.
For the reader we simply use a dummy empty directory reader:

    protected DirectoryReader open(Directory directory) throws IOException {
        List<IndexCommit> indexCommits = DirectoryReader.listCommits(directory);
        IndexCommit indexCommit = indexCommits.get(indexCommits.size() - 1);
        return new DirectoryReader(directory, new LeafReader[0]) {
            @Override
            protected DirectoryReader doOpenIfChanged() throws IOException {
                return null;
            }

            @Override
            protected DirectoryReader doOpenIfChanged(IndexCommit commit) throws IOException {
                return null;
            }

            @Override
            protected DirectoryReader doOpenIfChanged(IndexWriter writer, boolean applyAllDeletes) throws IOException {
                return null;
            }

            @Override
            public long getVersion() {
                return 0;
            }

            @Override
            public boolean isCurrent() throws IOException {
                return true;
            }

            @Override
            public IndexCommit getIndexCommit() throws IOException {
                return indexCommit;
            }

            @Override
            protected void doClose() throws IOException {

            }

            @Override
            public CacheHelper getReaderCacheHelper() {
                return null;
            }
        };
  }

this way we would share quite some code and behavior which I think is good?

[tlrx]

It makes sense. By extending ReadOnlyEngine, NoOpEngine will also obtain the index writer lock which I think is good too.

[tlrx]

Thanks @s1monw, I updated the code.

[bleskes]

LGTM.

[s1monw]

looks good. left some suggestions

[tlrx]

Thanks @bleskes and @s1monw. I updated the code and it now also takes care of doc stats.

[bleskes]

Still LGTM

[s1monw]

left some nits and suggestions.

[tlrx]

@elasticmachine test this please

[s1monw]

LGTM