Move up acquireSearcher logic to Engine (#33453)

By moving the logic to acquire the searcher up to the engine
it's simpler to build new engines that are for instance read-only.

Author: s1monw

server/src/main/java/org/elasticsearch/index/engine/Engine.java

@@ -33,6 +33,7 @@
 import org.apache.lucene.index.SegmentReader;
 import org.apache.lucene.index.Term;
 import org.apache.lucene.search.IndexSearcher;
+import org.apache.lucene.search.ReferenceManager;
 import org.apache.lucene.store.AlreadyClosedException;
 import org.apache.lucene.store.Directory;
 import org.apache.lucene.store.IOContext;
@@ -569,7 +570,31 @@ public final Searcher acquireSearcher(String source) throws EngineException {
      *
      * @see Searcher#close()
      */
-    public abstract Searcher acquireSearcher(String source, SearcherScope scope) throws EngineException;
+    public Searcher acquireSearcher(String source, SearcherScope scope) throws EngineException {
+        /* Acquire order here is store -> manager since we need
+         * to make sure that the store is not closed before
+         * the searcher is acquired. */
+        if (store.tryIncRef() == false) {
+            throw new AlreadyClosedException(shardId + " store is closed", failedEngine.get());
+        }
+        Releasable releasable = store::decRef;
+        try {
+            EngineSearcher engineSearcher = new EngineSearcher(source, getReferenceManager(scope), store, logger);
+            releasable = null; // success - hand over the reference to the engine searcher
+            return engineSearcher;
+        } catch (AlreadyClosedException ex) {
+            throw ex;
+        } catch (Exception ex) {
+            maybeFailEngine("acquire_searcher", ex);
+            ensureOpen(ex); // throw EngineCloseException here if we are already closed
+            logger.error(() -> new ParameterizedMessage("failed to acquire searcher, source {}", source), ex);
+            throw new EngineException(shardId, "failed to acquire searcher, source " + source, ex);
+        } finally {
+            Releasables.close(releasable);
+        }
+    }
+
+    protected abstract ReferenceManager<IndexSearcher> getReferenceManager(SearcherScope scope);
 
     public enum SearcherScope {
         EXTERNAL, INTERNAL

server/src/main/java/org/elasticsearch/index/engine/InternalEngine.java

@@ -20,7 +20,6 @@
 package org.elasticsearch.index.engine;
 
 import org.apache.logging.log4j.Logger;
-import org.apache.logging.log4j.message.ParameterizedMessage;
 import org.apache.lucene.document.Field;
 import org.apache.lucene.document.NumericDocValuesField;
 import org.apache.lucene.index.DirectoryReader;
@@ -53,7 +52,6 @@
 import org.elasticsearch.common.Nullable;
 import org.elasticsearch.common.SuppressForbidden;
 import org.elasticsearch.common.lease.Releasable;
-import org.elasticsearch.common.lease.Releasables;
 import org.elasticsearch.common.lucene.LoggerInfoStream;
 import org.elasticsearch.common.lucene.Lucene;
 import org.elasticsearch.common.lucene.index.ElasticsearchDirectoryReader;
@@ -1527,19 +1525,11 @@ final void refresh(String source, SearcherScope scope) throws EngineException {
             if (store.tryIncRef()) {
                 // increment the ref just to ensure nobody closes the store during a refresh
                 try {
-                    switch (scope) {
-                        case EXTERNAL:
-                            // even though we maintain 2 managers we really do the heavy-lifting only once.
-                            // the second refresh will only do the extra work we have to do for warming caches etc.
-                            externalSearcherManager.maybeRefreshBlocking();
-                            // the break here is intentional we never refresh both internal / external together
-                            break;
-                        case INTERNAL:
-                            internalSearcherManager.maybeRefreshBlocking();
-                            break;
-                        default:
-                            throw new IllegalArgumentException("unknown scope: " + scope);
-                    }
+                    // even though we maintain 2 managers we really do the heavy-lifting only once.
+                    // the second refresh will only do the extra work we have to do for warming caches etc.
+                    ReferenceManager<IndexSearcher> referenceManager = getReferenceManager(scope);
+                    // it is intentional that we never refresh both internal / external together
+                    referenceManager.maybeRefreshBlocking();
                 } finally {
                     store.decRef();
                 }
@@ -2090,37 +2080,14 @@ protected final void closeNoLock(String reason, CountDownLatch closedLatch) {
     }
 
     @Override
-    public Searcher acquireSearcher(String source, SearcherScope scope) {
-        /* Acquire order here is store -> manager since we need
-         * to make sure that the store is not closed before
-         * the searcher is acquired. */
-        if (store.tryIncRef() == false) {
-            throw new AlreadyClosedException(shardId + " store is closed", failedEngine.get());
-        }
-        Releasable releasable = store::decRef;
-        try {
-            final ReferenceManager<IndexSearcher> referenceManager;
-            switch (scope) {
-                case INTERNAL:
-                    referenceManager = internalSearcherManager;
-                    break;
-                case EXTERNAL:
-                    referenceManager = externalSearcherManager;
-                    break;
-                default:
-                    throw new IllegalStateException("unknown scope: " + scope);
-            }
-            EngineSearcher engineSearcher = new EngineSearcher(source, referenceManager, store, logger);
-            releasable = null; // success - hand over the reference to the engine searcher
-            return engineSearcher;
-        } catch (AlreadyClosedException ex) {
-            throw ex;
-        } catch (Exception ex) {
-            ensureOpen(ex); // throw EngineCloseException here if we are already closed
-            logger.error(() -> new ParameterizedMessage("failed to acquire searcher, source {}", source), ex);
-            throw new EngineException(shardId, "failed to acquire searcher, source " + source, ex);
-        } finally {
-            Releasables.close(releasable);
+    protected final ReferenceManager<IndexSearcher> getReferenceManager(SearcherScope scope) {
+        switch (scope) {
+            case INTERNAL:
+                return internalSearcherManager;
+            case EXTERNAL:
+                return externalSearcherManager;
+            default:
+                throw new IllegalStateException("unknown scope: " + scope);
         }
     }
 

server/src/main/java/org/elasticsearch/repositories/blobstore/BlobStoreRepository.java

@@ -1340,7 +1340,7 @@ private void snapshotFile(final BlobStoreIndexShardSnapshot.FileInfo fileInfo) t
         }
 
         private void failStoreIfCorrupted(Exception e) {
-            if (e instanceof CorruptIndexException || e instanceof IndexFormatTooOldException || e instanceof IndexFormatTooNewException) {
+            if (Lucene.isCorruptionException(e)) {
                 try {
                     store.markStoreCorrupted((IOException) e);
                 } catch (IOException inner) {