From 9ba66d87e4858e7185aa200f0f1ca938b806c2d8 Mon Sep 17 00:00:00 2001 From: Boaz Leskes Date: Tue, 5 Dec 2017 10:37:10 +0100 Subject: [PATCH] IndexShard - only flush on primary activation if it's a relocation target 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 # Conflicts: # core/src/main/java/org/elasticsearch/index/shard/IndexShard.java --- .../main/java/org/elasticsearch/index/shard/IndexShard.java | 5 +++-- .../org/elasticsearch/upgrades/FullClusterRestartIT.java | 6 +++--- 2 files changed, 6 insertions(+), 5 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 index e8aa9540e9822..de4cce5746491 100644 --- a/core/src/main/java/org/elasticsearch/index/shard/IndexShard.java +++ b/core/src/main/java/org/elasticsearch/index/shard/IndexShard.java @@ -63,7 +63,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; @@ -421,10 +420,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 index 1fcb4fc4dbeb5..8ec33dfbfc9b7 100644 --- 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);} } }