[solr] branch main updated: SOLR-16860: Remove watch from ZkStateReader of Coordinator Node when collection is deleted (#1752)

[no...@apache.org]

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

noble pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/solr.git


The following commit(s) were added to refs/heads/main by this push:
     new 1bdaf44676c SOLR-16860: Remove watch from ZkStateReader of Coordinator Node when collection is deleted (#1752)
1bdaf44676c is described below

commit 1bdaf44676c29931b1749e8275732c6987fccd4f
Author: patsonluk <pa...@users.noreply.github.com>
AuthorDate: Wed Jul 5 01:38:25 2023 -0700

    SOLR-16860: Remove watch from ZkStateReader of Coordinator Node when collection is deleted (#1752)
---
 .../solr/servlet/CoordinatorHttpSolrCall.java      |  8 +++-
 .../solr/common/cloud/ZkStateReaderAccessor.java   |  5 +++
 .../apache/solr/search/TestCoordinatorRole.java    | 52 ++++++++++++++++++++++
 .../apache/solr/common/cloud/ZkStateReader.java    |  5 +++
 4 files changed, 69 insertions(+), 1 deletion(-)

diff --git a/solr/core/src/java/org/apache/solr/servlet/CoordinatorHttpSolrCall.java b/solr/core/src/java/org/apache/solr/servlet/CoordinatorHttpSolrCall.java
index 44f1fd5f837..a9448d1310c 100644
--- a/solr/core/src/java/org/apache/solr/servlet/CoordinatorHttpSolrCall.java
+++ b/solr/core/src/java/org/apache/solr/servlet/CoordinatorHttpSolrCall.java
@@ -96,7 +96,13 @@ public class CoordinatorHttpSolrCall extends HttpSolrCall {
         SolrCore core = solrCall.getCoreByCollection(syntheticCollectionName, isPreferLeader);
         if (core != null) {
           factory.collectionVsCoreNameMapping.put(collectionName, core.getName());
-          solrCall.cores.getZkController().getZkStateReader().registerCore(collectionName);
+          // for the watcher, only remove on collection deletion (ie collection == null), since
+          // watch from coordinator is collection specific
+          solrCall
+              .cores
+              .getZkController()
+              .getZkStateReader()
+              .registerDocCollectionWatcher(collectionName, collection -> collection == null);
           if (log.isDebugEnabled()) {
             log.debug("coordinator node, returns synthetic core: {}", core.getName());
           }
diff --git a/solr/core/src/test/org/apache/solr/common/cloud/ZkStateReaderAccessor.java b/solr/core/src/test/org/apache/solr/common/cloud/ZkStateReaderAccessor.java
index 0b51b1d9c2d..c5b8d02ef2c 100644
--- a/solr/core/src/test/org/apache/solr/common/cloud/ZkStateReaderAccessor.java
+++ b/solr/core/src/test/org/apache/solr/common/cloud/ZkStateReaderAccessor.java
@@ -16,6 +16,7 @@
  */
 package org.apache.solr.common.cloud;
 
+import java.util.Collections;
 import java.util.Set;
 
 /** Helper class to access package-private methods from ZkStateReader. */
@@ -29,4 +30,8 @@ public class ZkStateReaderAccessor {
   public Set<DocCollectionWatcher> getStateWatchers(String collection) {
     return zkStateReader.getStateWatchers(collection);
   }
+
+  public Set<String> getWatchedCollections() {
+    return Collections.unmodifiableSet(zkStateReader.getCollectionWatches().keySet());
+  }
 }
diff --git a/solr/core/src/test/org/apache/solr/search/TestCoordinatorRole.java b/solr/core/src/test/org/apache/solr/search/TestCoordinatorRole.java
index d164a7b1bbc..705429de5e5 100644
--- a/solr/core/src/test/org/apache/solr/search/TestCoordinatorRole.java
+++ b/solr/core/src/test/org/apache/solr/search/TestCoordinatorRole.java
@@ -25,6 +25,7 @@ import java.util.Date;
 import java.util.EnumSet;
 import java.util.HashSet;
 import java.util.List;
+import java.util.Objects;
 import java.util.Random;
 import java.util.Set;
 import java.util.concurrent.ExecutorService;
@@ -47,6 +48,8 @@ import org.apache.solr.common.SolrException;
 import org.apache.solr.common.SolrInputDocument;
 import org.apache.solr.common.cloud.DocCollection;
 import org.apache.solr.common.cloud.Replica;
+import org.apache.solr.common.cloud.ZkStateReader;
+import org.apache.solr.common.cloud.ZkStateReaderAccessor;
 import org.apache.solr.common.params.CommonParams;
 import org.apache.solr.common.params.ShardParams;
 import org.apache.solr.common.util.ExecutorUtil;
@@ -411,4 +414,53 @@ public class TestCoordinatorRole extends SolrCloudTestCase {
     assertTrue(found);
     return (String) docs.get(0).getFieldValue("_core_");
   }
+
+  public void testWatch() throws Exception {
+    final int DATA_NODE_COUNT = 2;
+    MiniSolrCloudCluster cluster =
+        configureCluster(DATA_NODE_COUNT)
+            .addConfig("conf1", configset("cloud-minimal"))
+            .configure();
+    final String TEST_COLLECTION = "c1";
+
+    try {
+      CloudSolrClient client = cluster.getSolrClient();
+      CollectionAdminRequest.createCollection(TEST_COLLECTION, "conf1", 1, 2).process(client);
+      cluster.waitForActiveCollection(TEST_COLLECTION, 1, 2);
+      System.setProperty(NodeRoles.NODE_ROLES_PROP, "coordinator:on");
+      JettySolrRunner coordinatorJetty;
+      try {
+        coordinatorJetty = cluster.startJettySolrRunner();
+      } finally {
+        System.clearProperty(NodeRoles.NODE_ROLES_PROP);
+      }
+
+      ZkStateReader zkStateReader =
+          coordinatorJetty.getCoreContainer().getZkController().getZkStateReader();
+      ZkStateReaderAccessor zkWatchAccessor = new ZkStateReaderAccessor(zkStateReader);
+
+      // no watch at first
+      assertTrue(!zkWatchAccessor.getWatchedCollections().contains(TEST_COLLECTION));
+      new QueryRequest(new SolrQuery("*:*"))
+          .setPreferredNodes(List.of(coordinatorJetty.getNodeName()))
+          .process(client, TEST_COLLECTION);
+
+      // now it should be watching it after the query
+      assertTrue(zkWatchAccessor.getWatchedCollections().contains(TEST_COLLECTION));
+
+      CollectionAdminRequest.deleteReplica(TEST_COLLECTION, "shard1", 1).process(client);
+      cluster.waitForActiveCollection(TEST_COLLECTION, 1, 1);
+
+      // still one replica left, should not remove the watch
+      assertTrue(zkWatchAccessor.getWatchedCollections().contains(TEST_COLLECTION));
+
+      CollectionAdminRequest.deleteCollection(TEST_COLLECTION).process(client);
+      zkStateReader.waitForState(TEST_COLLECTION, 30, TimeUnit.SECONDS, Objects::isNull);
+
+      // watch should be removed after collection deletion
+      assertTrue(!zkWatchAccessor.getWatchedCollections().contains(TEST_COLLECTION));
+    } finally {
+      cluster.shutdown();
+    }
+  }
 }
diff --git a/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/ZkStateReader.java b/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/ZkStateReader.java
index 0074ab74762..d8115fdf1b8 100644
--- a/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/ZkStateReader.java
+++ b/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/ZkStateReader.java
@@ -1977,6 +1977,11 @@ public class ZkStateReader implements SolrCloseable {
     return watchers;
   }
 
+  /* package-private for testing*/
+  Map<String, StatefulCollectionWatch> getCollectionWatches() {
+    return Collections.unmodifiableMap(collectionWatches.statefulWatchesByCollectionName);
+  }
+
   public void registerCollectionPropsWatcher(
       final String collection, CollectionPropsWatcher propsWatcher) {
     AtomicBoolean watchSet = new AtomicBoolean(false);