You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ma...@apache.org on 2020/10/28 21:02:53 UTC

[lucene-solr] branch reference_impl updated (87b6d4f -> 3bcb6f7)

This is an automated email from the ASF dual-hosted git repository.

markrmiller pushed a change to branch reference_impl
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git.


    from 87b6d4f  @1056 Bring ZkShardTerms back closer to former impl, I think this version really does need to start at 1 and not 0 given the logs I'm seeing now.
     new 07b55e2  @1057 Some retry on connection loss for Overseer, remove broken clusterstate refresh thing, harden CollectionsAPISolrjTest a bit.
     new e33c447  @1058 Harden.
     new 3bcb6f7  @1059 Harden.

The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../src/java/org/apache/solr/cloud/Overseer.java   | 36 ++++++++++++++--------
 .../apache/solr/cloud/OverseerElectionContext.java | 22 +++++--------
 .../org/apache/solr/cloud/OverseerTaskQueue.java   | 14 ++++-----
 .../org/apache/solr/cloud/ZkDistributedQueue.java  | 12 ++++----
 .../solr/cloud/api/collections/SplitShardCmd.java  | 25 ++++++++++++++-
 .../solr/cloud/overseer/CollectionMutator.java     |  8 ++---
 .../apache/solr/cloud/CollectionsAPISolrJTest.java |  7 +++--
 .../cloud/api/collections/SplitByPrefixTest.java   |  5 +--
 .../src/java/org/apache/solr/common/ParWork.java   |  2 +-
 .../solr/common/cloud/ConnectionManager.java       | 20 ++++++++----
 10 files changed, 92 insertions(+), 59 deletions(-)


[lucene-solr] 02/03: @1058 Harden.

Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

markrmiller pushed a commit to branch reference_impl
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git

commit e33c44705e8cf9b459adee7620f48cfd515a1971
Author: markrmiller@gmail.com <ma...@gmail.com>
AuthorDate: Wed Oct 28 15:29:05 2020 -0500

    @1058 Harden.
---
 .../src/java/org/apache/solr/cloud/Overseer.java   | 24 +++++++++++++++++-----
 .../apache/solr/cloud/OverseerElectionContext.java | 22 +++++++-------------
 .../solr/cloud/overseer/CollectionMutator.java     |  8 ++++----
 .../apache/solr/cloud/CollectionsAPISolrJTest.java |  5 ++++-
 .../src/java/org/apache/solr/common/ParWork.java   |  2 +-
 .../solr/common/cloud/ConnectionManager.java       | 20 ++++++++++++------
 6 files changed, 49 insertions(+), 32 deletions(-)

diff --git a/solr/core/src/java/org/apache/solr/cloud/Overseer.java b/solr/core/src/java/org/apache/solr/cloud/Overseer.java
index d3af42f..7dd0116 100644
--- a/solr/core/src/java/org/apache/solr/cloud/Overseer.java
+++ b/solr/core/src/java/org/apache/solr/cloud/Overseer.java
@@ -237,7 +237,7 @@ public class Overseer implements SolrCloseable {
         // we do not sure which message is bad message, therefore we will re-process node one by one
         int fallbackQueueSize = Integer.MAX_VALUE;
         ZkDistributedQueue fallbackQueue = workQueue;
-        while (!isClosed()) {
+        while (!checkClosed()) {
           if (zkStateWriter == null) {
             try {
               zkStateWriter = new ZkStateWriter(reader, stats);
@@ -282,7 +282,6 @@ public class Overseer implements SolrCloseable {
               }
               // force flush at the end of the loop, if there are no pending updates, this is a no op call
               clusterState = zkStateWriter.writePendingUpdates(clusterState);
-              assert clusterState != null;
               // the workQueue is empty now, use stateUpdateQueue as fallback queue
               fallbackQueue = stateUpdateQueue;
               fallbackQueueSize = 0;
@@ -315,6 +314,10 @@ public class Overseer implements SolrCloseable {
 //            }
             queue = new LinkedList<>(stateUpdateQueue.peekElements(1000, wait, (x) -> true));
           } catch (AlreadyClosedException e) {
+            if (isClosed()) {
+              log.info("Overseer closed (AlreadyClosedException), exiting loop");
+              return;
+            }
             return;
           } catch (KeeperException.SessionExpiredException e) {
             log.warn("Solr cannot talk to ZK, exiting Overseer work queue loop", e);
@@ -350,7 +353,10 @@ public class Overseer implements SolrCloseable {
                   processedNodes.clear();
                 });
               }
-              if (isClosed()) return;
+              if (isClosed()) {
+                log.info("Overseer closed, exiting loop");
+                return;
+              }
               // if an event comes in the next *ms batch it together
               int wait = 0;
 //              if (zkStateWriter.getUpdatesToWrite().isEmpty()) {
@@ -359,7 +365,7 @@ public class Overseer implements SolrCloseable {
 //                wait = 0;
 //              }
               queue = new LinkedList<>(stateUpdateQueue.peekElements(100, wait, node -> !processedNodes.contains(node)));
-              if (loopCnt >= 1) {
+              if (loopCnt >= 3) {
                 break;
               }
               loopCnt++;
@@ -399,6 +405,14 @@ public class Overseer implements SolrCloseable {
       }
     }
 
+    private boolean checkClosed() {
+      boolean closed = isClosed();
+      if (closed) {
+        log.info("Overseer is closed, will not continue loop ...");
+      }
+      return closed;
+    }
+
     // Return true whenever the exception thrown by ZkStateWriter is correspond
     // to a invalid state or 'bad' message (in this case, we should remove that message from queue)
     private boolean isBadMessage(Exception e) {
@@ -673,11 +687,11 @@ public class Overseer implements SolrCloseable {
   }
 
   public synchronized void start(String id, ElectionContext context) throws KeeperException {
+    doClose();
     if (getCoreContainer().isShutDown() || closeAndDone) {
       if (log.isDebugEnabled()) log.debug("Already closed, exiting");
       return;
     }
-    doClose();
     closed = false;
 
     MDCLoggingContext.setNode(zkController == null ?
diff --git a/solr/core/src/java/org/apache/solr/cloud/OverseerElectionContext.java b/solr/core/src/java/org/apache/solr/cloud/OverseerElectionContext.java
index 2c811c6..6913b48 100644
--- a/solr/core/src/java/org/apache/solr/cloud/OverseerElectionContext.java
+++ b/solr/core/src/java/org/apache/solr/cloud/OverseerElectionContext.java
@@ -76,22 +76,14 @@ final class OverseerElectionContext extends ShardLeaderElectionContextBase {
 
     log.info("Registered as Overseer leader, starting Overseer ...");
 
-    synchronized (this) {
-      if (isClosed()) {
-        log.info("Bailing on becoming leader, we are closed");
-        return;
-      }
-      if (!isClosed() && !overseer.getZkController().getCoreContainer().isShutDown() && !overseer.isDone() && (overseer.getUpdaterThread() == null || !overseer.getUpdaterThread().isAlive())) {
-
-        try {
-          overseer.start(id, context);
-        } finally {
-          if (isClosed()) {
-            overseer.close();
-          }
-        }
-      }
+    if (isClosed()) {
+      log.info("Bailing on becoming leader, we are closed");
+      return;
     }
+    if (!isClosed() && !overseer.getZkController().getCoreContainer().isShutDown() && !overseer.isDone() && (overseer.getUpdaterThread() == null || !overseer.getUpdaterThread().isAlive())) {
+      overseer.start(id, context);
+    }
+
   }
 
   private void clearQueue(ZkDistributedQueue queue)
diff --git a/solr/core/src/java/org/apache/solr/cloud/overseer/CollectionMutator.java b/solr/core/src/java/org/apache/solr/cloud/overseer/CollectionMutator.java
index a64cee0..83c2331 100644
--- a/solr/core/src/java/org/apache/solr/cloud/overseer/CollectionMutator.java
+++ b/solr/core/src/java/org/apache/solr/cloud/overseer/CollectionMutator.java
@@ -91,11 +91,11 @@ public class CollectionMutator {
       // TODO - fix, no makePath (ensure every path part exists), async, single node
       try {
         stateManager.makePath(ZkStateReader.COLLECTIONS_ZKNODE + "/" + collectionName
-            + "/leader_elect/" + shardId);
+            + "/leader_elect/" + shardId, null, CreateMode.PERSISTENT, false);
         stateManager.makePath(ZkStateReader.COLLECTIONS_ZKNODE + "/" + collectionName
-            + "/leader_elect/" + shardId + LeaderElector.ELECTION_NODE);
+            + "/leader_elect/" + shardId + LeaderElector.ELECTION_NODE, null, CreateMode.PERSISTENT, false);
         stateManager.makePath(ZkStateReader.COLLECTIONS_ZKNODE+ "/" + collectionName + "/" + shardId
-            + ZkStateReader.SHARD_LEADERS_ZKNODE);
+            + ZkStateReader.SHARD_LEADERS_ZKNODE, null, CreateMode.PERSISTENT, false);
 
         stateManager.makePath(ZkStateReader.COLLECTIONS_ZKNODE + "/" + collectionName + "/" +  shardId, null, CreateMode.PERSISTENT, false);
         stateManager.makePath(ZkStateReader.COLLECTIONS_ZKNODE + "/" + collectionName + "/leader_elect/" +  shardId, null, CreateMode.PERSISTENT, false);
@@ -104,7 +104,7 @@ public class CollectionMutator {
         stateManager.makePath(ZkStateReader.COLLECTIONS_ZKNODE + "/" + collectionName + "/terms/" +  shardId, ZkStateReader.emptyJson, CreateMode.PERSISTENT, false);
         stateManager.makePath(ZkStateReader.COLLECTIONS_ZKNODE + "/" + collectionName + "/schema_lock", null, CreateMode.PERSISTENT, false);
       } catch (AlreadyExistsException e) {
-        throw new AlreadyClosedException();
+        throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, e);
       } catch (IOException e) {
         throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, e);
       } catch (KeeperException e) {
diff --git a/solr/core/src/test/org/apache/solr/cloud/CollectionsAPISolrJTest.java b/solr/core/src/test/org/apache/solr/cloud/CollectionsAPISolrJTest.java
index 5c09ae8..f4c851b 100644
--- a/solr/core/src/test/org/apache/solr/cloud/CollectionsAPISolrJTest.java
+++ b/solr/core/src/test/org/apache/solr/cloud/CollectionsAPISolrJTest.java
@@ -351,6 +351,8 @@ public class CollectionsAPISolrJTest extends SolrCloudTestCase {
       return true;
     });
 
+    cluster.waitForActiveCollection(collectionName, 3, 3);
+
     // Test splitting using split.key
     response = CollectionAdminRequest.splitShard(collectionName)
         .setSplitKey("b!")
@@ -359,7 +361,8 @@ public class CollectionsAPISolrJTest extends SolrCloudTestCase {
     assertEquals(0, response.getStatus());
     assertTrue(response.isSuccess());
 
-    waitForState("Expected 5 slices to be active", collectionName, (n, c) -> c.getActiveSlices().size() == 5);
+    // wait for 5 active shards
+    cluster.waitForActiveCollection(collectionName, 5, 5);
 
   }
 
diff --git a/solr/solrj/src/java/org/apache/solr/common/ParWork.java b/solr/solrj/src/java/org/apache/solr/common/ParWork.java
index 07e35a2..69125c8 100644
--- a/solr/solrj/src/java/org/apache/solr/common/ParWork.java
+++ b/solr/solrj/src/java/org/apache/solr/common/ParWork.java
@@ -640,7 +640,7 @@ public class ParWork implements Closeable {
       Thread.currentThread().interrupt();
     } else {
       if (infoLogMsg) {
-        log.info(t.getMessage());
+        log.info(t.getClass().getName() + " " + t.getMessage());
       } else {
         log.warn("Solr ran into an unexpected exception", t);
       }
diff --git a/solr/solrj/src/java/org/apache/solr/common/cloud/ConnectionManager.java b/solr/solrj/src/java/org/apache/solr/common/cloud/ConnectionManager.java
index b2bd431..55254e2 100644
--- a/solr/solrj/src/java/org/apache/solr/common/cloud/ConnectionManager.java
+++ b/solr/solrj/src/java/org/apache/solr/common/cloud/ConnectionManager.java
@@ -22,6 +22,7 @@ import java.lang.invoke.MethodHandles;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
+import java.util.concurrent.locks.ReentrantLock;
 
 import org.apache.solr.common.AlreadyClosedException;
 import org.apache.solr.common.ParWork;
@@ -60,7 +61,7 @@ public class ConnectionManager implements Watcher, Closeable {
 
   private volatile boolean isClosed = false;
 
-  private final Object keeperLock = new Object();
+  private final ReentrantLock keeperLock = new ReentrantLock(true);
 
   private volatile CountDownLatch connectedLatch = new CountDownLatch(1);
   private volatile CountDownLatch disconnectedLatch = new CountDownLatch(1);
@@ -82,12 +83,15 @@ public class ConnectionManager implements Watcher, Closeable {
   }
 
   public ZooKeeper getKeeper() {
-    synchronized (keeperLock) {
+    keeperLock.lock();
+    try {
       SolrZooKeeper rKeeper = keeper;
       if (rKeeper == null) {
         throw new AlreadyClosedException();
       }
       return rKeeper;
+    } finally {
+      keeperLock.unlock();
     }
   }
 
@@ -189,14 +193,16 @@ public class ConnectionManager implements Watcher, Closeable {
   }
 
   private void updatezk() throws IOException {
-    synchronized (keeperLock) {
+    keeperLock.lock();
+    try {
       if (isClosed()) return;
       if (keeper != null) {
         ParWork.close(keeper);
-        keeper = null;
       }
       SolrZooKeeper zk = createSolrZooKeeper(zkServerAddress, zkTimeout, this);
       keeper = zk;
+    } finally {
+      keeperLock.unlock();
     }
   }
 
@@ -265,7 +271,8 @@ public class ConnectionManager implements Watcher, Closeable {
       }
     }
 
-    synchronized (keeperLock) {
+    keeperLock.lock();
+    try {
       if (isClosed()) return;
       if (keeper != null) {
         // if there was a problem creating the new SolrZooKeeper
@@ -281,6 +288,8 @@ public class ConnectionManager implements Watcher, Closeable {
         }
       }
 
+    } finally {
+      keeperLock.unlock();
     }
 
     do {
@@ -351,7 +360,6 @@ public class ConnectionManager implements Watcher, Closeable {
     if (keeper != null) {
       keeper.close();
     }
-    keeper = null;
 
     ExecutorUtil.awaitTermination(client.zkCallbackSerialExecutor);
     ExecutorUtil.awaitTermination(client.zkCallbackExecutor);


[lucene-solr] 01/03: @1057 Some retry on connection loss for Overseer, remove broken clusterstate refresh thing, harden CollectionsAPISolrjTest a bit.

Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

markrmiller pushed a commit to branch reference_impl
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git

commit 07b55e25a42bb008d64dbe824bbaedb5ebc1549b
Author: markrmiller@gmail.com <ma...@gmail.com>
AuthorDate: Wed Oct 28 14:43:29 2020 -0500

    @1057 Some retry on connection loss for Overseer, remove broken clusterstate refresh thing, harden CollectionsAPISolrjTest a bit.
---
 solr/core/src/java/org/apache/solr/cloud/Overseer.java     | 14 +++++---------
 .../src/java/org/apache/solr/cloud/OverseerTaskQueue.java  | 14 +++++++-------
 .../src/java/org/apache/solr/cloud/ZkDistributedQueue.java | 12 ++++++------
 .../org/apache/solr/cloud/CollectionsAPISolrJTest.java     |  2 +-
 4 files changed, 19 insertions(+), 23 deletions(-)

diff --git a/solr/core/src/java/org/apache/solr/cloud/Overseer.java b/solr/core/src/java/org/apache/solr/cloud/Overseer.java
index 767b2ce..d3af42f 100644
--- a/solr/core/src/java/org/apache/solr/cloud/Overseer.java
+++ b/solr/core/src/java/org/apache/solr/cloud/Overseer.java
@@ -237,7 +237,7 @@ public class Overseer implements SolrCloseable {
         // we do not sure which message is bad message, therefore we will re-process node one by one
         int fallbackQueueSize = Integer.MAX_VALUE;
         ZkDistributedQueue fallbackQueue = workQueue;
-        while (!isClosed() && !Thread.currentThread().isInterrupted()) {
+        while (!isClosed()) {
           if (zkStateWriter == null) {
             try {
               zkStateWriter = new ZkStateWriter(reader, stats);
@@ -439,13 +439,9 @@ public class Overseer implements SolrCloseable {
       ClusterState state;
       LinkedHashMap<String,ClusterState.CollectionRef> collStates;
       ClusterState prevState = null;
-      if (itemsQueued.sum() == 1) {
-        log.info("First queue item for Overseer ...");
-        // we have ensured a full cluster state update on start over cluster state updater loop
-        state = reader.getClusterState();
-      } else {
-        state = clusterState;
-      }
+
+      state = clusterState;
+
       collStates = new LinkedHashMap<>(state.getCollectionStates());
       for (DocCollection docCollection : updatesToWrite.values()) {
         Map<String,Slice> slicesMap = docCollection.getSlicesMap();
@@ -713,7 +709,7 @@ public class Overseer implements SolrCloseable {
             log.info("Overseer leader has changed, closing ...");
             Overseer.this.close();
           }
-        }});
+        }}, true);
     } catch (KeeperException.SessionExpiredException e) {
       log.warn("ZooKeeper session expired");
       return;
diff --git a/solr/core/src/java/org/apache/solr/cloud/OverseerTaskQueue.java b/solr/core/src/java/org/apache/solr/cloud/OverseerTaskQueue.java
index a9c2e45..d9551d8 100644
--- a/solr/core/src/java/org/apache/solr/cloud/OverseerTaskQueue.java
+++ b/solr/core/src/java/org/apache/solr/cloud/OverseerTaskQueue.java
@@ -83,7 +83,7 @@ public class OverseerTaskQueue extends ZkDistributedQueue {
     for (String childName : childNames) {
       if (childName != null && childName.startsWith(PREFIX)) {
         try {
-          byte[] data = zookeeper.getData(dir + "/" + childName, null, null);
+          byte[] data = zookeeper.getData(dir + "/" + childName, null, null, true);
           if (data != null) {
             ZkNodeProps message = ZkNodeProps.load(data);
             if (message.containsKey(requestIdKey)) {
@@ -114,13 +114,13 @@ public class OverseerTaskQueue extends ZkDistributedQueue {
           + path.substring(path.lastIndexOf("-") + 1);
 
       try {
-        zookeeper.setData(responsePath, event.getBytes(), false);
+        zookeeper.setData(responsePath, event.getBytes(), true);
       } catch (KeeperException.NoNodeException ignored) {
         // this will often not exist or have been removed
         if (log.isDebugEnabled()) log.debug("Response ZK path: {} doesn't exist.", responsePath);
       }
       try {
-        zookeeper.delete(path, -1);
+        zookeeper.delete(path, -1, true);
       } catch (KeeperException.NoNodeException ignored) {
       }
     } finally {
@@ -232,7 +232,7 @@ public class OverseerTaskQueue extends ZkDistributedQueue {
       String watchID = createResponseNode();
 
       LatchWatcher watcher = new LatchWatcher(zookeeper);
-      byte[] bytes = zookeeper.getData(watchID, watcher, null);
+      byte[] bytes = zookeeper.getData(watchID, watcher, null, true);
 
       // create the request node
       createRequestNode(data, watchID);
@@ -240,13 +240,13 @@ public class OverseerTaskQueue extends ZkDistributedQueue {
       pendingResponses.increment();
       if (bytes == null) {
         watcher.await(timeout);
-        bytes = zookeeper.getData(watchID, null, null);
+        bytes = zookeeper.getData(watchID, null, null, true);
       }
 
       // create the event before deleting the node, otherwise we can get the deleted
       // event from the watcher.
       QueueEvent event =  new QueueEvent(watchID, bytes, watcher.getWatchedEvent());
-      zookeeper.delete(watchID, -1);
+      zookeeper.delete(watchID, -1, true);
       return event;
     } finally {
       time.stop();
@@ -311,7 +311,7 @@ public class OverseerTaskQueue extends ZkDistributedQueue {
       if (headNode != null) {
         try {
           QueueEvent queueEvent = new QueueEvent(dir + "/" + headNode, zookeeper.getData(dir + "/" + headNode,
-              null, null), null);
+              null, null, true), null);
           return queueEvent.getId();
         } catch (KeeperException.NoNodeException e) {
           // Another client removed the node first, try next
diff --git a/solr/core/src/java/org/apache/solr/cloud/ZkDistributedQueue.java b/solr/core/src/java/org/apache/solr/cloud/ZkDistributedQueue.java
index 33fc44a..33cf8bf 100644
--- a/solr/core/src/java/org/apache/solr/cloud/ZkDistributedQueue.java
+++ b/solr/core/src/java/org/apache/solr/cloud/ZkDistributedQueue.java
@@ -279,7 +279,7 @@ public class ZkDistributedQueue implements DistributedQueue {
           // don't know which nodes are not exist, so try to delete one by one node
           for (int j = from; j < to; j++) {
             try {
-              zookeeper.delete(ops.get(j).getPath(), -1);
+              zookeeper.delete(ops.get(j).getPath(), -1, true);
             } catch (KeeperException.NoNodeException e2) {
               if (log.isDebugEnabled()) {
                 log.debug("Can not remove node which is not exist : {}", ops.get(j).getPath());
@@ -333,7 +333,7 @@ public class ZkDistributedQueue implements DistributedQueue {
         if (maxQueueSize > 0) {
           if (offerPermits.get() <= 0 || offerPermits.getAndDecrement() <= 0) {
             // If a max queue size is set, check it before creating a new queue item.
-            Stat stat = zookeeper.exists(dir, null);
+            Stat stat = zookeeper.exists(dir, null, true);
             if (stat == null) {
               // jump to the code below, which tries to create dir if it doesn't exist
               throw new KeeperException.NoNodeException();
@@ -511,7 +511,7 @@ public class ZkDistributedQueue implements DistributedQueue {
           break;
         }
         try {
-          byte[] data = zookeeper.getData(dir + "/" + child, null, null);
+          byte[] data = zookeeper.getData(dir + "/" + child, null, null, true);
           result.add(new Pair<>(child, data));
         } catch (KeeperException.NoNodeException e) {
           continue;
@@ -538,7 +538,7 @@ public class ZkDistributedQueue implements DistributedQueue {
           return null;
         }
         try {
-          return zookeeper.getData(dir + "/" + firstChild, null, null);
+          return zookeeper.getData(dir + "/" + firstChild, null, null, true);
         } catch (KeeperException.NoNodeException e) {
           return null;
         }
@@ -558,8 +558,8 @@ public class ZkDistributedQueue implements DistributedQueue {
         }
         try {
           String path = dir + "/" + firstChild;
-          byte[] result = zookeeper.getData(path, null, null);
-          zookeeper.delete(path, -1);
+          byte[] result = zookeeper.getData(path, null, null, true);
+          zookeeper.delete(path, -1, true);
           // stats.setQueueLength(knownChildren.size());
           return result;
         } catch (KeeperException.NoNodeException e) {
diff --git a/solr/core/src/test/org/apache/solr/cloud/CollectionsAPISolrJTest.java b/solr/core/src/test/org/apache/solr/cloud/CollectionsAPISolrJTest.java
index e4d454b..5c09ae8 100644
--- a/solr/core/src/test/org/apache/solr/cloud/CollectionsAPISolrJTest.java
+++ b/solr/core/src/test/org/apache/solr/cloud/CollectionsAPISolrJTest.java
@@ -92,7 +92,7 @@ public class CollectionsAPISolrJTest extends SolrCloudTestCase {
 
   @After
   public void afterTest() throws Exception {
-    cluster.deleteAllCollections();
+
   }
 
   /**


[lucene-solr] 03/03: @1059 Harden.

Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

markrmiller pushed a commit to branch reference_impl
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git

commit 3bcb6f7d270b8fbd6695ea1d7161d153b57f17e6
Author: markrmiller@gmail.com <ma...@gmail.com>
AuthorDate: Wed Oct 28 16:02:24 2020 -0500

    @1059 Harden.
---
 .../solr/cloud/api/collections/SplitShardCmd.java  | 25 +++++++++++++++++++++-
 .../apache/solr/cloud/CollectionsAPISolrJTest.java |  2 +-
 .../cloud/api/collections/SplitByPrefixTest.java   |  5 +----
 3 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/SplitShardCmd.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/SplitShardCmd.java
index edb51df..4b5d65b 100644
--- a/solr/core/src/java/org/apache/solr/cloud/api/collections/SplitShardCmd.java
+++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/SplitShardCmd.java
@@ -24,6 +24,7 @@ import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
+import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.NoSuchElementException;
@@ -773,7 +774,7 @@ public class SplitShardCmd implements OverseerCollectionMessageHandler.Cmd {
         Collection<Slice> searchSlices = router.getSearchSlicesSingle(splitKey, new ModifiableSolrParams(), collection);
 
         int tryCnt = 0;
-        while (searchSlices.isEmpty() && tryCnt < 50) {
+        while (searchSlices.isEmpty() && tryCnt < 500) {
           tryCnt++;
           clusterState = zkStateReader.getClusterState();
           collection = clusterState.getCollection(collectionName);
@@ -784,6 +785,13 @@ public class SplitShardCmd implements OverseerCollectionMessageHandler.Cmd {
             ParWork.propagateInterrupt(e);
           }
           searchSlices = router.getSearchSlicesSingle(splitKey, new ModifiableSolrParams(), collection);
+          Iterator<Slice> it = searchSlices.iterator();
+          while (it.hasNext()) {
+            if (it.next().getState() != Slice.State.ACTIVE) {
+              searchSlices.clear();
+              break;
+            }
+          }
         }
 
         if (searchSlices.isEmpty()) {
@@ -803,6 +811,21 @@ public class SplitShardCmd implements OverseerCollectionMessageHandler.Cmd {
       }
     } else {
       parentSlice = collection.getSlice(slice.get());
+      int tryCnt = 0;
+      // TODO use zkStateReader#waitFor
+      while (parentSlice.getState() != Slice.State.ACTIVE && tryCnt < 500) {
+        tryCnt++;
+        try {
+          Thread.sleep(100);
+        } catch (InterruptedException e) {
+          ParWork.propagateInterrupt(e);
+        }
+        clusterState = zkStateReader.getClusterState();
+        collection = clusterState.getCollection(collectionName);
+
+        parentSlice = collection.getSlice(slice.get());
+      }
+
     }
 
     if (parentSlice == null) {
diff --git a/solr/core/src/test/org/apache/solr/cloud/CollectionsAPISolrJTest.java b/solr/core/src/test/org/apache/solr/cloud/CollectionsAPISolrJTest.java
index f4c851b..67f89a1 100644
--- a/solr/core/src/test/org/apache/solr/cloud/CollectionsAPISolrJTest.java
+++ b/solr/core/src/test/org/apache/solr/cloud/CollectionsAPISolrJTest.java
@@ -362,7 +362,7 @@ public class CollectionsAPISolrJTest extends SolrCloudTestCase {
     assertTrue(response.isSuccess());
 
     // wait for 5 active shards
-    cluster.waitForActiveCollection(collectionName, 5, 5);
+    cluster.waitForActiveCollection(collectionName, 4, 4);
 
   }
 
diff --git a/solr/core/src/test/org/apache/solr/cloud/api/collections/SplitByPrefixTest.java b/solr/core/src/test/org/apache/solr/cloud/api/collections/SplitByPrefixTest.java
index f16336a..ad7352b 100644
--- a/solr/core/src/test/org/apache/solr/cloud/api/collections/SplitByPrefixTest.java
+++ b/solr/core/src/test/org/apache/solr/cloud/api/collections/SplitByPrefixTest.java
@@ -75,7 +75,6 @@ public class SplitByPrefixTest extends SolrCloudTestCase {
   @Override
   public void tearDown() throws Exception {
     super.tearDown();
-    cluster.deleteAllCollections();
   }
 
   public static class Prefix implements Comparable<Prefix> {
@@ -174,7 +173,7 @@ public class SplitByPrefixTest extends SolrCloudTestCase {
     }
     splitShard.process(client);
     cluster.waitForActiveCollection(COLLECTION_NAME, 2, 2);
-    cluster.getZkClient().printLayout();
+
     List<Prefix> prefixes = findPrefixes(20, 0, 0x00ffffff);
     List<Prefix> uniquePrefixes = removeDups(prefixes);
     if (uniquePrefixes.size() % 2 == 1) {  // make it an even sized list so we can split it exactly in two
@@ -198,8 +197,6 @@ public class SplitByPrefixTest extends SolrCloudTestCase {
     splitShard.process(client);
     cluster.waitForActiveCollection(COLLECTION_NAME, 3, 3);
 
-    cluster.getZkClient().printLayout();
-
     // OK, now let's check that the correct split point was chosen
     // We can use the router to find the shards for the middle prefixes and they should be different.