IndexShard - only flush on primary activation if it's a relocation ta…

…rget from an old node #27580 added extra flushes when a shard transitions to primary to make sure that we never replay translog ops without seq# during recovery. The current logic causes an extra flush when a primary starts when it's recovering from the store. This is not needed as we also flush in the engine itself (to add sequence numbers info into the commit). This double flushing confuses tests and is unneeded. Fixes #27649
elastic · Dec 5, 2017 · e8c34c2 · e8c34c2
1 parent a7c2def
commit e8c34c2
Show file tree

Hide file tree

Showing 2 changed files with 6 additions and 6 deletions.
diff --git a/core/src/main/java/org/elasticsearch/index/shard/IndexShard.java b/core/src/main/java/org/elasticsearch/index/shard/IndexShard.java
@@ -66,7 +66,6 @@
 import org.elasticsearch.common.util.BigArrays;
 import org.elasticsearch.common.util.concurrent.AbstractRunnable;
 import org.elasticsearch.common.util.concurrent.AsyncIOProcessor;
-import org.elasticsearch.common.util.concurrent.ThreadContext;
 import org.elasticsearch.common.xcontent.XContentFactory;
 import org.elasticsearch.index.Index;
 import org.elasticsearch.index.IndexModule;
@@ -142,7 +141,6 @@
 import java.nio.channels.ClosedByInterruptException;
 import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.Collections;
 import java.util.EnumSet;
 import java.util.List;
@@ -408,10 +406,12 @@ public void updateShardState(final ShardRouting newRouting,
 
                 if (newRouting.primary()) {
                     final DiscoveryNode recoverySourceNode = recoveryState.getSourceNode();
+                    final Engine engine = getEngine();
                     if (currentRouting.isRelocationTarget() == false || recoverySourceNode.getVersion().before(Version.V_6_0_0_alpha1)) {
                         // there was no primary context hand-off in < 6.0.0, need to manually activate the shard
-                        final Engine engine = getEngine();
                         engine.seqNoService().activatePrimaryMode(getEngine().seqNoService().getLocalCheckpoint());
+                    }
+                    if (currentRouting.isRelocationTarget() == true && recoverySourceNode.getVersion().before(Version.V_6_0_0_alpha1)) {
                         // Flush the translog as it may contain operations with no sequence numbers. We want to make sure those
                         // operations will never be replayed as part of peer recovery to avoid an arbitrary mixture of operations with seq#
                         // (due to active indexing) and operations without a seq# coming from the translog. We therefore flush

diff --git a/qa/full-cluster-restart/src/test/java/org/elasticsearch/upgrades/FullClusterRestartIT.java b/qa/full-cluster-restart/src/test/java/org/elasticsearch/upgrades/FullClusterRestartIT.java
@@ -613,7 +613,6 @@ public void testEmptyShard() throws IOException {
      * Tests recovery of an index with or without a translog and the
      * statistics we gather about that.
      */
-    @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/27649")
     public void testRecovery() throws IOException {
         int count;
         boolean shouldHaveTranslog;
@@ -694,8 +693,9 @@ public void testRecovery() throws IOException {
                     fail("expected version to be one of [" + currentLuceneVersion + "," + bwcLuceneVersion + "] but was " + line);
                 }
             }
-            assertNotEquals("expected at least 1 current segment after translog recovery", 0, numCurrentVersion);
-            assertNotEquals("expected at least 1 old segment", 0, numBwcVersion);}
+            assertNotEquals("expected at least 1 current segment after translog recovery. segments:\n" + segmentsResponse,
+                0, numCurrentVersion);
+            assertNotEquals("expected at least 1 old segment. segments:\n" + segmentsResponse, 0, numBwcVersion);}
         }
     }