You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by da...@apache.org on 2018/01/15 06:02:37 UTC

lucene-solr:jira/solr-11702: SOLR-11702: Update based on shalin review result

Repository: lucene-solr
Updated Branches:
  refs/heads/jira/solr-11702 a1ec63d8a -> ad756efb0


SOLR-11702: Update based on shalin review result


Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/ad756efb
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/ad756efb
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/ad756efb

Branch: refs/heads/jira/solr-11702
Commit: ad756efb01e1b65b8e7ffa0d9c90938e4d9a9185
Parents: a1ec63d
Author: Cao Manh Dat <da...@apache.org>
Authored: Mon Jan 15 13:02:18 2018 +0700
Committer: Cao Manh Dat <da...@apache.org>
Committed: Mon Jan 15 13:02:18 2018 +0700

----------------------------------------------------------------------
 .../org/apache/solr/cloud/ElectionContext.java  |  7 +++---
 .../cloud/LeaderInitiatedRecoveryThread.java    |  2 +-
 .../solr/cloud/RecoveringCoreTermWatcher.java   | 20 +++++++--------
 .../apache/solr/cloud/ZkCollectionTerms.java    | 26 ++++++++++++--------
 .../org/apache/solr/cloud/ZkController.java     | 25 +++++++++++++------
 .../solr/handler/admin/CollectionsHandler.java  |  1 -
 .../processor/DistributedUpdateProcessor.java   |  2 ++
 ...aderInitiatedRecoveryOnShardRestartTest.java |  2 +-
 .../TestLeaderInitiatedRecoveryThread.java      |  1 +
 9 files changed, 51 insertions(+), 35 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/ad756efb/solr/core/src/java/org/apache/solr/cloud/ElectionContext.java
----------------------------------------------------------------------
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 ea196d2..2d00151 100644
--- a/solr/core/src/java/org/apache/solr/cloud/ElectionContext.java
+++ b/solr/core/src/java/org/apache/solr/cloud/ElectionContext.java
@@ -443,7 +443,6 @@ final class ShardLeaderElectionContext extends ShardLeaderElectionContextBase {
       boolean isLeader = true;
       if (!isClosed) {
         try {
-          //TODO remove this on 8.0, SOLR-11812
           // we must check LIR before registering as leader
           checkLIR(coreName, allReplicasInLine);
           if (replicaType == Replica.Type.TLOG) {
@@ -532,7 +531,7 @@ final class ShardLeaderElectionContext extends ShardLeaderElectionContextBase {
     return docCollection.getReplica(replicaName);
   }
 
-  //TODO remove this method on 8.0, SOLR-11812
+  @Deprecated
   public void checkLIR(String coreName, boolean allReplicasInLine)
       throws InterruptedException, KeeperException, IOException {
     if (allReplicasInLine) {
@@ -571,7 +570,7 @@ final class ShardLeaderElectionContext extends ShardLeaderElectionContextBase {
     }
   }
 
-  //TODO remove this method on 8.0, SOLR-11812
+  @Deprecated
   private void startLeaderInitiatedRecoveryOnReplicas(String coreName) throws Exception {
     try (SolrCore core = cc.getCore(coreName)) {
       CloudDescriptor cloudDesc = core.getCoreDescriptor().getCloudDescriptor();
@@ -600,7 +599,7 @@ final class ShardLeaderElectionContext extends ShardLeaderElectionContextBase {
             continue; // added safe-guard so we don't mark this core as down
 
           if (zkController.getShardTerms(collection, shardId).registered(replicaCoreNodeName)) {
-            // this replica is registered its term so it is running with the new LIR implementation
+            // the replica registered its term so it is running with the new LIR implementation
             // we can put this replica into recovery by increase our terms
             zkController.getShardTerms(collection, shardId).ensureTermsIsHigher(coreNodeName, Collections.singleton(replicaCoreNodeName));
             continue;

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/ad756efb/solr/core/src/java/org/apache/solr/cloud/LeaderInitiatedRecoveryThread.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/cloud/LeaderInitiatedRecoveryThread.java b/solr/core/src/java/org/apache/solr/cloud/LeaderInitiatedRecoveryThread.java
index 88baac5..9c46236 100644
--- a/solr/core/src/java/org/apache/solr/cloud/LeaderInitiatedRecoveryThread.java
+++ b/solr/core/src/java/org/apache/solr/cloud/LeaderInitiatedRecoveryThread.java
@@ -45,7 +45,7 @@ import java.util.List;
  * replica; used by a shard leader to nag a replica into recovering after the
  * leader experiences an error trying to send an update request to the replica.
  */
-//TODO remove this class on 8.0, SOLR-11812
+@Deprecated
 public class LeaderInitiatedRecoveryThread extends Thread {
 
   private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/ad756efb/solr/core/src/java/org/apache/solr/cloud/RecoveringCoreTermWatcher.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/cloud/RecoveringCoreTermWatcher.java b/solr/core/src/java/org/apache/solr/cloud/RecoveringCoreTermWatcher.java
index 13737d3..26fec97 100644
--- a/solr/core/src/java/org/apache/solr/cloud/RecoveringCoreTermWatcher.java
+++ b/solr/core/src/java/org/apache/solr/cloud/RecoveringCoreTermWatcher.java
@@ -44,17 +44,17 @@ public class RecoveringCoreTermWatcher implements ZkShardTerms.CoreTermWatcher {
     if (solrCore.isClosed()) {
       return false;
     }
-    try {
-      String coreNodeName = solrCore.getCoreDescriptor().getCloudDescriptor().getCoreNodeName();
-      if (terms.canBecomeLeader(coreNodeName)) return true;
-      if (lastTermDoRecovery.get() < terms.getTerm(coreNodeName)) {
-        log.info("Start recovery on {} because core's term is less than leader's term", coreNodeName);
-        lastTermDoRecovery.set(terms.getTerm(coreNodeName));
-        solrCore.getUpdateHandler().getSolrCoreState().doRecovery(solrCore.getCoreContainer(), solrCore.getCoreDescriptor());
-      }
-    } catch (NullPointerException e) {
-      log.info("NPE when getting coreNodeName, hence do not start recovery process");
+
+    if (solrCore.getCoreDescriptor() == null || solrCore.getCoreDescriptor().getCloudDescriptor() == null) return true;
+
+    String coreNodeName = solrCore.getCoreDescriptor().getCloudDescriptor().getCoreNodeName();
+    if (terms.canBecomeLeader(coreNodeName)) return true;
+    if (lastTermDoRecovery.get() < terms.getTerm(coreNodeName)) {
+      log.info("Start recovery on {} because core's term is less than leader's term", coreNodeName);
+      lastTermDoRecovery.set(terms.getTerm(coreNodeName));
+      solrCore.getUpdateHandler().getSolrCoreState().doRecovery(solrCore.getCoreContainer(), solrCore.getCoreDescriptor());
     }
+
     return true;
   }
 

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/ad756efb/solr/core/src/java/org/apache/solr/cloud/ZkCollectionTerms.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/cloud/ZkCollectionTerms.java b/solr/core/src/java/org/apache/solr/cloud/ZkCollectionTerms.java
index a393916..b232f9b 100644
--- a/solr/core/src/java/org/apache/solr/cloud/ZkCollectionTerms.java
+++ b/solr/core/src/java/org/apache/solr/cloud/ZkCollectionTerms.java
@@ -28,9 +28,9 @@ import org.apache.solr.core.CoreDescriptor;
  * Used to manage all ZkShardTerms of a collection
  */
 class ZkCollectionTerms implements AutoCloseable {
-  private String collection;
-  private Map<String, ZkShardTerms> terms;
-  private SolrZkClient zkClient;
+  private final String collection;
+  private final Map<String, ZkShardTerms> terms;
+  private final SolrZkClient zkClient;
 
   ZkCollectionTerms(String collection, SolrZkClient client) {
     this.collection = collection;
@@ -40,19 +40,25 @@ class ZkCollectionTerms implements AutoCloseable {
   }
 
 
-  public synchronized ZkShardTerms getShard(String shardId) {
-    if (!terms.containsKey(shardId)) terms.put(shardId, new ZkShardTerms(collection, shardId, zkClient));
-    return terms.get(shardId);
+  public ZkShardTerms getShard(String shardId) {
+    synchronized (terms) {
+      if (!terms.containsKey(shardId)) terms.put(shardId, new ZkShardTerms(collection, shardId, zkClient));
+      return terms.get(shardId);
+    }
   }
 
-  public synchronized void remove(String shardId, CoreDescriptor coreDescriptor) {
-    if (getShard(shardId).removeTerm(coreDescriptor)) {
-      terms.remove(shardId).close();
+  public void remove(String shardId, CoreDescriptor coreDescriptor) {
+    synchronized (terms) {
+      if (getShard(shardId).removeTerm(coreDescriptor)) {
+        terms.remove(shardId).close();
+      }
     }
   }
 
   public void close() {
-    terms.values().forEach(ZkShardTerms::close);
+    synchronized (terms) {
+      terms.values().forEach(ZkShardTerms::close);
+    }
     ObjectReleaseTracker.release(this);
   }
 

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/ad756efb/solr/core/src/java/org/apache/solr/cloud/ZkController.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/cloud/ZkController.java b/solr/core/src/java/org/apache/solr/cloud/ZkController.java
index b3d6bb1..ab9a224 100644
--- a/solr/core/src/java/org/apache/solr/cloud/ZkController.java
+++ b/solr/core/src/java/org/apache/solr/cloud/ZkController.java
@@ -226,7 +226,7 @@ public class ZkController {
 
   private volatile boolean isClosed;
 
-  //TODO remove in 8.0, SOLR-11812
+  @Deprecated
   // keeps track of replicas that have been asked to recover by leaders running on this node
   private final Map<String, String> replicasInLeaderInitiatedRecovery = new HashMap<String, String>();
 
@@ -1037,7 +1037,10 @@ public class ZkController {
       assert coreZkNodeName != null : "we should have a coreNodeName by now";
 
       ZkShardTerms shardTerms = getShardTerms(collection, cloudDesc.getShardId());
-      if ("new".equals(desc.getCoreProperty("lirVersion", "new"))) {
+
+      // This flag is used for testing rolling updates and should be removed in SOLR-11812
+      boolean isRunningInNewLIR = "new".equals(desc.getCoreProperty("lirVersion", "new"));
+      if (isRunningInNewLIR) {
         // this call is useful in case of reconnecting to ZK
         shardTerms.refreshTerms(true);
         shardTerms.registerTerm(coreZkNodeName);
@@ -1133,7 +1136,7 @@ public class ZkController {
           publish(desc, Replica.State.ACTIVE);
         }
 
-        if ("new".equals(desc.getCoreProperty("lirVersion", "new"))) {
+        if (isRunningInNewLIR) {
           shardTerms.addListener(new RecoveringCoreTermWatcher(core));
         }
         core.getCoreDescriptor().getCloudDescriptor().setHasRegistered(true);
@@ -1323,7 +1326,6 @@ public class ZkController {
         return true;
       }
 
-      //TODO remove in 8.0, SOLR-11812
       // see if the leader told us to recover
       final Replica.State lirState = getLeaderInitiatedRecoveryState(collection, shardId,
           core.getCoreDescriptor().getCloudDescriptor().getCoreNodeName());
@@ -1390,7 +1392,6 @@ public class ZkController {
       
       String coreNodeName = cd.getCloudDescriptor().getCoreNodeName();
 
-      //TODO remove in 8.0, SOLR-11812
       // If the leader initiated recovery, then verify that this replica has performed
       // recovery as requested before becoming active; don't even look at lirState if going down
       if (state != Replica.State.DOWN) {
@@ -1449,7 +1450,9 @@ public class ZkController {
         log.info("The core '{}' had failed to initialize before.", cd.getName());
       }
 
-      if (state == Replica.State.RECOVERING && "new".equals(cd.getCoreProperty("lirVersion", "new"))) {
+      // This flag is used for testing rolling updates and should be removed in SOLR-11812
+      boolean isRunningInNewLIR = "new".equals(cd.getCoreProperty("lirVersion", "new"));
+      if (state == Replica.State.RECOVERING && isRunningInNewLIR) {
         getShardTerms(collection, shardId).setEqualsToMax(coreNodeName);
       }
       ZkNodeProps m = new ZkNodeProps(props);
@@ -1755,7 +1758,6 @@ public class ZkController {
 
       // detect if this core is in leader-initiated recovery and if so,
       // then we don't need the leader to wait on seeing the down state
-      // TODO remove getting LIR state in 8.0, SOLR-11812
       Replica.State lirState = null;
       try {
         lirState = getLeaderInitiatedRecoveryState(collection, shard, myCoreNodeName);
@@ -2068,7 +2070,6 @@ public class ZkController {
     return cc;
   }
 
-  //TODO remove all old LIR implementation in 8.0, SOLR-11812
   /**
    * When a leader receives a communication error when trying to send a request to a replica,
    * it calls this method to ensure the replica enters recovery when connectivity is restored.
@@ -2077,6 +2078,7 @@ public class ZkController {
    * false means the node is not live either, so no point in trying to send recovery commands
    * to it.
    */
+  @Deprecated
   public boolean ensureReplicaInLeaderInitiatedRecovery(
       final CoreContainer container,
       final String collection, final String shardId, final ZkCoreNodeProps replicaCoreProps,
@@ -2146,6 +2148,7 @@ public class ZkController {
     return nodeIsLive;
   }
 
+  @Deprecated
   public boolean isReplicaInRecoveryHandling(String replicaUrl) {
     boolean exists = false;
     synchronized (replicasInLeaderInitiatedRecovery) {
@@ -2154,12 +2157,14 @@ public class ZkController {
     return exists;
   }
 
+  @Deprecated
   public void removeReplicaFromLeaderInitiatedRecoveryHandling(String replicaUrl) {
     synchronized (replicasInLeaderInitiatedRecovery) {
       replicasInLeaderInitiatedRecovery.remove(replicaUrl);
     }
   }
 
+  @Deprecated
   public Replica.State getLeaderInitiatedRecoveryState(String collection, String shardId, String coreNodeName) {
     final Map<String, Object> stateObj = getLeaderInitiatedRecoveryStateObject(collection, shardId, coreNodeName);
     if (stateObj == null) {
@@ -2169,6 +2174,7 @@ public class ZkController {
     return stateStr == null ? null : Replica.State.getState(stateStr);
   }
 
+  @Deprecated
   public Map<String, Object> getLeaderInitiatedRecoveryStateObject(String collection, String shardId, String coreNodeName) {
 
     if (collection == null || shardId == null || coreNodeName == null)
@@ -2213,6 +2219,7 @@ public class ZkController {
     return stateObj;
   }
 
+  @Deprecated
   public void updateLeaderInitiatedRecoveryState(String collection, String shardId, String coreNodeName,
       Replica.State state, CoreDescriptor leaderCd, boolean retryOnConnLoss) {
     if (collection == null || shardId == null || coreNodeName == null) {
@@ -2338,10 +2345,12 @@ public class ZkController {
     }
   }
 
+  @Deprecated
   public static String getLeaderInitiatedRecoveryZnodePath(String collection, String shardId) {
     return "/collections/" + collection + "/leader_initiated_recovery/" + shardId;
   }
 
+  @Deprecated
   public static String getLeaderInitiatedRecoveryZnodePath(String collection, String shardId, String coreNodeName) {
     return getLeaderInitiatedRecoveryZnodePath(collection, shardId) + "/" + coreNodeName;
   }

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/ad756efb/solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java b/solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java
index b54cbaf..11595af 100644
--- a/solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java
+++ b/solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java
@@ -987,7 +987,6 @@ public class CollectionsHandler extends RequestHandlerBase implements Permission
             "The shard already has an active leader. Force leader is not applicable. State: " + slice);
       }
 
-      //TODO remove this in 8.0, SOLR-11812
       // Clear out any LIR state
       String lirPath = handler.coreContainer.getZkController().getLeaderInitiatedRecoveryZnodePath(collectionName, sliceId);
       if (handler.coreContainer.getZkController().getZkClient().exists(lirPath, true)) {

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/ad756efb/solr/core/src/java/org/apache/solr/update/processor/DistributedUpdateProcessor.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/update/processor/DistributedUpdateProcessor.java b/solr/core/src/java/org/apache/solr/update/processor/DistributedUpdateProcessor.java
index 063a740..3cff171 100644
--- a/solr/core/src/java/org/apache/solr/update/processor/DistributedUpdateProcessor.java
+++ b/solr/core/src/java/org/apache/solr/update/processor/DistributedUpdateProcessor.java
@@ -185,6 +185,8 @@ public class DistributedUpdateProcessor extends UpdateRequestProcessor {
   // are custom and may modify the SolrInputDocument racing with its serialization for replication
   private final boolean cloneRequiredOnLeader;
   private final Replica.Type replicaType;
+
+  @Deprecated
   // this flag, used for testing rolling updates, should be removed by SOLR-11812
   private final boolean isOldLIRMode;
 

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/ad756efb/solr/core/src/test/org/apache/solr/cloud/LeaderInitiatedRecoveryOnShardRestartTest.java
----------------------------------------------------------------------
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 0c46689..12bde17 100644
--- a/solr/core/src/test/org/apache/solr/cloud/LeaderInitiatedRecoveryOnShardRestartTest.java
+++ b/solr/core/src/test/org/apache/solr/cloud/LeaderInitiatedRecoveryOnShardRestartTest.java
@@ -47,7 +47,7 @@ import org.slf4j.LoggerFactory;
 @Slow
 @Nightly
 @AwaitsFix(bugUrl = "https://issues.apache.org/jira/browse/SOLR-10071")
-//TODO remove this test on SOLR-11812
+@Deprecated
 public class LeaderInitiatedRecoveryOnShardRestartTest extends AbstractFullDistribZkTestBase {
   private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
   

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/ad756efb/solr/core/src/test/org/apache/solr/cloud/TestLeaderInitiatedRecoveryThread.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/cloud/TestLeaderInitiatedRecoveryThread.java b/solr/core/src/test/org/apache/solr/cloud/TestLeaderInitiatedRecoveryThread.java
index 0fbc0a1..f099fc6 100644
--- a/solr/core/src/test/org/apache/solr/cloud/TestLeaderInitiatedRecoveryThread.java
+++ b/solr/core/src/test/org/apache/solr/cloud/TestLeaderInitiatedRecoveryThread.java
@@ -34,6 +34,7 @@ import org.apache.zookeeper.data.Stat;
 /**
  * Test for {@link LeaderInitiatedRecoveryThread}
  */
+@Deprecated
 @SolrTestCaseJ4.SuppressSSL
 public class TestLeaderInitiatedRecoveryThread extends AbstractFullDistribZkTestBase {