Remove isRecovering method from Engine (#47039)

We already prevent flushing in Engine if it's recovering. Hence, we can remove the protection in IndexShard.
elastic · Sep 25, 2019 · 9df6cbe · 9df6cbe
1 parent de88249
commit 9df6cbe
Show file tree

Hide file tree

Showing 4 changed files with 12 additions and 32 deletions.
diff --git a/server/src/main/java/org/elasticsearch/index/engine/Engine.java b/server/src/main/java/org/elasticsearch/index/engine/Engine.java
@@ -1869,13 +1869,6 @@ public interface Warmer {
      */
     public abstract void skipTranslogRecovery();
 
-    /**
-     * Returns <code>true</code> iff this engine is currently recovering from translog.
-     */
-    public boolean isRecovering() {
-        return false;
-    }
-
     /**
      * Tries to prune buffered deletes from the version map.
      */

diff --git a/server/src/main/java/org/elasticsearch/index/engine/InternalEngine.java b/server/src/main/java/org/elasticsearch/index/engine/InternalEngine.java
@@ -2457,7 +2457,7 @@ protected void commitIndexWriter(final IndexWriter writer, final Translog transl
         }
     }
 
-    private void ensureCanFlush() {
+    final void ensureCanFlush() {
         // translog recover happens after the engine is fully constructed
         // if we are in this stage we have to prevent flushes from this
         // engine otherwise we might loose documents if the flush succeeds
@@ -2649,11 +2649,6 @@ public Closeable acquireRetentionLock() {
         }
     }
 
-    @Override
-    public boolean isRecovering() {
-        return pendingTranslogRecovery.get();
-    }
-
     /**
      * Gets the commit data from {@link IndexWriter} as a map.
      */

diff --git a/server/src/main/java/org/elasticsearch/index/shard/IndexShard.java b/server/src/main/java/org/elasticsearch/index/shard/IndexShard.java
@@ -1054,12 +1054,7 @@ public CompletionStats completionStats(String... fields) {
     public Engine.SyncedFlushResult syncFlush(String syncId, Engine.CommitId expectedCommitId) {
         verifyNotClosed();
         logger.trace("trying to sync flush. sync id [{}]. expected commit id [{}]]", syncId, expectedCommitId);
-        Engine engine = getEngine();
-        if (engine.isRecovering()) {
-            throw new IllegalIndexShardStateException(shardId(), state, "syncFlush is only allowed if the engine is not recovery" +
-                " from translog");
-        }
-        return engine.syncFlush(syncId, expectedCommitId);
+        return getEngine().syncFlush(syncId, expectedCommitId);
     }
 
     /**
@@ -1078,15 +1073,8 @@ public Engine.CommitId flush(FlushRequest request) {
          * since we use Engine#writeIndexingBuffer for this now.
          */
         verifyNotClosed();
-        final Engine engine = getEngine();
-        if (engine.isRecovering()) {
-            throw new IllegalIndexShardStateException(
-                    shardId(),
-                    state,
-                    "flush is only allowed if the engine is not recovery from translog");
-        }
         final long time = System.nanoTime();
-        final Engine.CommitId commitId = engine.flush(force, waitIfOngoing);
+        final Engine.CommitId commitId = getEngine().flush(force, waitIfOngoing);
         flushMetric.inc(System.nanoTime() - time);
         return commitId;
     }

diff --git a/server/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java b/server/src/test/java/org/elasticsearch/index/engine/InternalEngineTests.java
@@ -731,16 +731,20 @@ public long getProcessedCheckpoint() {
     }
 
     public void testFlushIsDisabledDuringTranslogRecovery() throws IOException {
-        assertFalse(engine.isRecovering());
+        engine.ensureCanFlush(); // recovered already
         ParsedDocument doc = testParsedDocument("1", null, testDocumentWithTextField(), SOURCE, null);
         engine.index(indexForDoc(doc));
         engine.close();
 
         engine = new InternalEngine(engine.config());
+        expectThrows(IllegalStateException.class, engine::ensureCanFlush);
         expectThrows(IllegalStateException.class, () -> engine.flush(true, true));
-        assertTrue(engine.isRecovering());
-        engine.recoverFromTranslog(translogHandler, Long.MAX_VALUE);
-        assertFalse(engine.isRecovering());
+        if (randomBoolean()) {
+            engine.recoverFromTranslog(translogHandler, Long.MAX_VALUE);
+        } else {
+            engine.skipTranslogRecovery();
+        }
+        engine.ensureCanFlush(); // ready
         doc = testParsedDocument("2", null, testDocumentWithTextField(), SOURCE, null);
         engine.index(indexForDoc(doc));
         engine.flush();
@@ -2825,7 +2829,7 @@ public void testCurrentTranslogIDisCommitted() throws IOException {
             {
                 for (int i = 0; i < 2; i++) {
                     try (InternalEngine engine = new InternalEngine(config)) {
-                        assertTrue(engine.isRecovering());
+                        expectThrows(IllegalStateException.class, engine::ensureCanFlush);
                         Map<String, String> userData = engine.getLastCommittedSegmentInfos().getUserData();
                         if (i == 0) {
                             assertEquals("1", userData.get(Translog.TRANSLOG_GENERATION_KEY));