From e8c34c2c627c4addda921536a3516c5c604945bf 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 --- .../main/java/org/elasticsearch/index/shard/IndexShard.java | 6 +++--- .../org/elasticsearch/upgrades/FullClusterRestartIT.java | 6 +++--- 2 files changed, 6 insertions(+), 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 index 23b7b0c9a5a79..7362bd1be90c8 100644 --- 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 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);} } }