From 820eea1f3dba412f0a462809b554c8519886e8d3 Mon Sep 17 00:00:00 2001 From: Mark Robert Miller Date: Fri, 13 Nov 2015 13:40:52 +0000 Subject: [PATCH] SOLR-8075: Fix faulty implementation. git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1714204 13f79535-47bb-0310-9956-ffa450edef68 --- .../apache/solr/cloud/ElectionContext.java | 74 +++++++++++-------- ...erInitiatedRecoveryOnShardRestartTest.java | 34 ++++++++- 2 files changed, 74 insertions(+), 34 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/cloud/ElectionContext.java b/solr/core/src/java/org/apache/solr/cloud/ElectionContext.java index 70a29cc9d15..2e8c3790f54 100644 --- a/solr/core/src/java/org/apache/solr/cloud/ElectionContext.java +++ b/solr/core/src/java/org/apache/solr/cloud/ElectionContext.java @@ -393,14 +393,20 @@ final class ShardLeaderElectionContext extends ShardLeaderElectionContextBase { return; } - log.info("I am the new leader: " + ZkCoreNodeProps.getCoreUrl(leaderProps) + " " + shardId); - core.getCoreDescriptor().getCloudDescriptor().setLeader(true); } boolean isLeader = true; if (!isClosed) { try { + // we must check LIR before registering as leader + checkLIR(coreName, allReplicasInLine); + super.runLeaderProcess(weAreReplacement, 0); + try (SolrCore core = cc.getCore(coreName)) { + core.getCoreDescriptor().getCloudDescriptor().setLeader(true); + } + log.info("I am the new leader: " + ZkCoreNodeProps.getCoreUrl(leaderProps) + " " + shardId); + } catch (Exception e) { isLeader = false; SolrException.log(log, "There was a problem trying to register as the leader", e); @@ -420,23 +426,6 @@ final class ShardLeaderElectionContext extends ShardLeaderElectionContextBase { } if (isLeader) { - if (allReplicasInLine) { - // SOLR-8075: A bug may allow the proper leader to get marked as LIR DOWN and - // if we are marked as DOWN but were able to become the leader, we remove - // the DOWN entry here so that we don't fail publishing ACTIVE due to being in LIR. - // We only do this if all the replicas participated in the election just in case - // this was a valid LIR entry and the proper leader replica is missing. - try (SolrCore core = cc.getCore(coreName)) { - final Replica.State lirState = zkController.getLeaderInitiatedRecoveryState(collection, shardId, - core.getCoreDescriptor().getCloudDescriptor().getCoreNodeName()); - if (lirState == Replica.State.DOWN) { - zkController.updateLeaderInitiatedRecoveryState(collection, shardId, - leaderProps.getStr(ZkStateReader.CORE_NODE_NAME_PROP), Replica.State.ACTIVE, null, true); - } - } - - } - // check for any replicas in my shard that were set to down by the previous leader try { startLeaderInitiatedRecoveryOnReplicas(coreName); @@ -452,6 +441,41 @@ final class ShardLeaderElectionContext extends ShardLeaderElectionContextBase { MDCLoggingContext.clear(); } } + + public void checkLIR(String coreName, boolean allReplicasInLine) + throws InterruptedException, KeeperException, IOException { + if (allReplicasInLine) { + // SOLR-8075: A bug may allow the proper leader to get marked as LIR DOWN and + // if we are marked as DOWN but were able to become the leader, we remove + // the DOWN entry here so that we don't fail publishing ACTIVE due to being in LIR. + // We only do this if all the replicas participated in the election just in case + // this was a valid LIR entry and the proper leader replica is missing. + try (SolrCore core = cc.getCore(coreName)) { + final Replica.State lirState = zkController.getLeaderInitiatedRecoveryState(collection, shardId, + core.getCoreDescriptor().getCloudDescriptor().getCoreNodeName()); + if (lirState == Replica.State.DOWN) { + // We can do this before registering as leader because only setting DOWN requires that + // we are already registered as leader, and here we are setting ACTIVE + // The fact that we just won the zk leader election provides a quasi lock on setting this state, but + // we should improve this: see SOLR-8075 discussion + zkController.updateLeaderInitiatedRecoveryState(collection, shardId, + leaderProps.getStr(ZkStateReader.CORE_NODE_NAME_PROP), Replica.State.ACTIVE, core.getCoreDescriptor(), true); + } + } + + } else { + try (SolrCore core = cc.getCore(coreName)) { + final Replica.State lirState = zkController.getLeaderInitiatedRecoveryState(collection, shardId, + core.getCoreDescriptor().getCloudDescriptor().getCoreNodeName()); + if (lirState == Replica.State.DOWN || lirState == Replica.State.RECOVERING) { + log.warn("The previous leader marked me " + core.getName() + + " as " + lirState.toString() + " and I haven't recovered yet, so I shouldn't be the leader."); + + throw new SolrException(ErrorCode.SERVER_ERROR, "Leader Initiated Recovery prevented leadership"); + } + } + } + } private void startLeaderInitiatedRecoveryOnReplicas(String coreName) throws Exception { try (SolrCore core = cc.getCore(coreName)) { @@ -594,18 +618,6 @@ final class ShardLeaderElectionContext extends ShardLeaderElectionContextBase { } if (core.getCoreDescriptor().getCloudDescriptor().getLastPublished() == Replica.State.ACTIVE) { - - // maybe active but if the previous leader marked us as down and - // we haven't recovered, then can't be leader - final Replica.State lirState = zkController.getLeaderInitiatedRecoveryState(collection, shardId, - core.getCoreDescriptor().getCloudDescriptor().getCoreNodeName()); - if (lirState == Replica.State.DOWN || lirState == Replica.State.RECOVERING) { - log.warn("Although my last published state is Active, the previous leader marked me "+core.getName() - + " as " + lirState.toString() - + " and I haven't recovered yet, so I shouldn't be the leader."); - return false; - } - log.info("My last published State was Active, it's okay to be the leader."); return true; } diff --git a/solr/core/src/test/org/apache/solr/cloud/LeaderInitiatedRecoveryOnShardRestartTest.java b/solr/core/src/test/org/apache/solr/cloud/LeaderInitiatedRecoveryOnShardRestartTest.java index 553f58637b7..4e86970e114 100644 --- a/solr/core/src/test/org/apache/solr/cloud/LeaderInitiatedRecoveryOnShardRestartTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/LeaderInitiatedRecoveryOnShardRestartTest.java @@ -26,6 +26,7 @@ import org.apache.solr.client.solrj.embedded.JettySolrRunner; import org.apache.solr.common.cloud.SolrZkClient; import org.apache.solr.common.cloud.ZkStateReader; import org.apache.solr.common.util.Utils; +import org.apache.zookeeper.KeeperException.NodeExistsException; import org.junit.Test; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -41,7 +42,7 @@ public class LeaderInitiatedRecoveryOnShardRestartTest extends AbstractFullDistr public LeaderInitiatedRecoveryOnShardRestartTest() { super(); sliceCount = 1; - fixShardCount(2); + fixShardCount(3); } @Test @@ -50,7 +51,7 @@ public class LeaderInitiatedRecoveryOnShardRestartTest extends AbstractFullDistr String testCollectionName = "all_in_lir"; String shardId = "shard1"; - createCollection(testCollectionName, 1, 2, 1); + createCollection(testCollectionName, 1, 3, 1); cloudClient.setDefaultCollection(testCollectionName); @@ -64,8 +65,9 @@ public class LeaderInitiatedRecoveryOnShardRestartTest extends AbstractFullDistr SolrZkClient zkClient = cloudClient.getZkStateReader().getZkClient(); zkClient.makePath("/collections/" + testCollectionName + "/leader_initiated_recovery/" + shardId + "/core_node1", znodeData, true); zkClient.makePath("/collections/" + testCollectionName + "/leader_initiated_recovery/" + shardId + "/core_node2", znodeData, true); + zkClient.makePath("/collections/" + testCollectionName + "/leader_initiated_recovery/" + shardId + "/core_node3", znodeData, true); - printLayout(); + // printLayout(); for (JettySolrRunner jetty : jettys) { ChaosMonkey.stop(jetty); @@ -79,5 +81,31 @@ public class LeaderInitiatedRecoveryOnShardRestartTest extends AbstractFullDistr // recoveries will not finish without SOLR-8075 waitForRecoveriesToFinish(testCollectionName, true); + + // now expire each node + try { + zkClient.makePath("/collections/" + testCollectionName + "/leader_initiated_recovery/" + shardId + "/core_node1", znodeData, true); + } catch (NodeExistsException e) { + + } + try { + zkClient.makePath("/collections/" + testCollectionName + "/leader_initiated_recovery/" + shardId + "/core_node2", znodeData, true); + } catch (NodeExistsException e) { + + } + try { + zkClient.makePath("/collections/" + testCollectionName + "/leader_initiated_recovery/" + shardId + "/core_node3", znodeData, true); + } catch (NodeExistsException e) { + + } + + for (JettySolrRunner jetty : jettys) { + chaosMonkey.expireSession(jetty); + } + + Thread.sleep(2000); + + // recoveries will not finish without SOLR-8075 + waitForRecoveriesToFinish(testCollectionName, true); } }