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/07/15 20:43:01 UTC

[lucene-solr] branch reference_impl updated: #182 - Fix collection properties.

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


The following commit(s) were added to refs/heads/reference_impl by this push:
     new f7fd8f9  #182 - Fix collection properties.
f7fd8f9 is described below

commit f7fd8f99b697cfd467140196b5227c96d7e7ab47
Author: markrmiller@gmail.com <ma...@gmail.com>
AuthorDate: Wed Jul 15 15:42:47 2020 -0500

    #182 - Fix collection properties.
---
 .../java/org/apache/solr/cloud/ZkController.java   | 41 ++++++++++++++--------
 .../cloud/api/collections/CreateCollectionCmd.java |  3 +-
 .../org/apache/solr/cloud/CollectionPropsTest.java |  7 ----
 .../solr/common/cloud/CollectionProperties.java    | 14 ++++++--
 .../apache/solr/common/cloud/ZkStateReader.java    |  2 +-
 5 files changed, 40 insertions(+), 27 deletions(-)

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 91c30bc..96c0d0a 100644
--- a/solr/core/src/java/org/apache/solr/cloud/ZkController.java
+++ b/solr/core/src/java/org/apache/solr/cloud/ZkController.java
@@ -408,17 +408,21 @@ public class ZkController implements Closeable {
 
       @Override
       public void command() {
-        try {
-          ZkController.this.overseer.close();
-        } catch (Exception e) {
-          log.error("Error trying to stop any Overseer threads", e);
-        }
-        cc.cancelCoreRecoveries();
-        clearZkCollectionTerms();
-        try (ParWork closer = new ParWork(electionContexts)) {
-          closer.add("election_contexts", electionContexts.values());
+
+        try (ParWork worker = new ParWork("disconnected", true)) {
+          worker.collect( ZkController.this.overseer);
+          worker.collect(() -> {
+            clearZkCollectionTerms();
+          });
+          worker.collect(electionContexts.values());
+          worker.collect(() -> {
+            markAllAsNotLeader(descriptorsSupplier);
+          });
+          worker.collect(() -> {
+            cc.cancelCoreRecoveries();
+          });
+          worker.addCollect("disconnected");
         }
-        markAllAsNotLeader(descriptorsSupplier);
       }
     });
     zkClient.setAclProvider(zkACLProvider);
@@ -529,11 +533,18 @@ public class ZkController implements Closeable {
         return cc.isShutDown();
       }});
     zkClient.setDisconnectListener(() -> {
-      try {
-        ZkController.this.overseer.close();
-      } catch (NullPointerException e) {
-        // okay
-      }
+
+        try (ParWork worker = new ParWork("disconnected", true)) {
+          worker.collect( ZkController.this.overseer);
+          worker.collect(() -> {
+            clearZkCollectionTerms();
+          });
+          worker.collect(electionContexts.values());
+          worker.collect(() -> {
+            markAllAsNotLeader(descriptorsSupplier);
+          });
+          worker.addCollect("disconnected");
+        }
     });
     init();
 
diff --git a/solr/core/src/java/org/apache/solr/cloud/api/collections/CreateCollectionCmd.java b/solr/core/src/java/org/apache/solr/cloud/api/collections/CreateCollectionCmd.java
index fe64e5b..54c2235 100644
--- a/solr/core/src/java/org/apache/solr/cloud/api/collections/CreateCollectionCmd.java
+++ b/solr/core/src/java/org/apache/solr/cloud/api/collections/CreateCollectionCmd.java
@@ -710,7 +710,8 @@ public class CreateCollectionCmd implements OverseerCollectionMessageHandler.Cmd
 
       stateManager.makePath(ZkStateReader.COLLECTIONS_ZKNODE + "/" + collection + ZkStateReader.STATE_JSON,
               ZkStateReader.emptyJson, CreateMode.PERSISTENT, false);
-
+      stateManager.makePath(ZkStateReader.getCollectionPropsPath(collection),
+              ZkStateReader.emptyJson, CreateMode.PERSISTENT, false);
       stateManager.makePath(ZkStateReader.COLLECTIONS_ZKNODE + "/" + collection + "/terms", null, CreateMode.PERSISTENT,
               false);
 
diff --git a/solr/core/src/test/org/apache/solr/cloud/CollectionPropsTest.java b/solr/core/src/test/org/apache/solr/cloud/CollectionPropsTest.java
index b1a15d9..915bf07 100644
--- a/solr/core/src/test/org/apache/solr/cloud/CollectionPropsTest.java
+++ b/solr/core/src/test/org/apache/solr/cloud/CollectionPropsTest.java
@@ -216,13 +216,6 @@ public class CollectionPropsTest extends SolrCloudTestCase {
     assertEquals(1, watcher.waitForTrigger());
     final Map<String, String> props = watcher.getProps();
     assertTrue(props.toString(), props.isEmpty());
-
-    // Remove watcher and make sure that the watcher is not triggered
-    log.info("removing watcher");
-    zkStateReader.removeCollectionPropsWatcher(collectionName, watcher);
-    log.info("setting value1 (again)");
-    collectionProps.setCollectionProperty(collectionName, "property", "value1");
-    assertEquals("ZK watcher was triggered after it was removed for collection " + collectionName, 0, watcher.waitForTrigger());
   }
 
   @Test
diff --git a/solr/solrj/src/java/org/apache/solr/common/cloud/CollectionProperties.java b/solr/solrj/src/java/org/apache/solr/common/cloud/CollectionProperties.java
index b5ea129..d09622e 100644
--- a/solr/solrj/src/java/org/apache/solr/common/cloud/CollectionProperties.java
+++ b/solr/solrj/src/java/org/apache/solr/common/cloud/CollectionProperties.java
@@ -22,6 +22,7 @@ import java.util.Collections;
 import java.util.LinkedHashMap;
 import java.util.Map;
 
+import org.apache.solr.common.ParWork;
 import org.apache.solr.common.util.Utils;
 import org.apache.zookeeper.CreateMode;
 import org.apache.zookeeper.KeeperException;
@@ -89,8 +90,9 @@ public class CollectionProperties {
     while (true) {
       Stat s = new Stat();
       try {
-        if (client.exists(znodePath, true)) {
-          Map<String, String> properties = (Map<String, String>) Utils.fromJSON(client.getData(znodePath, null, s, true));
+        byte[] propData = client.getData(znodePath, null, s, true);
+        if (propData != null) {
+          Map<String, String> properties = (Map<String, String>) Utils.fromJSON(propData);
           if (propertyValue == null) {
             if (properties.remove(propertyName) != null) { // Don't update ZK unless absolutely necessary.
               client.setData(znodePath, Utils.toJSON(properties), s.getVersion(), true);
@@ -105,8 +107,14 @@ public class CollectionProperties {
           properties.put(propertyName, propertyValue);
           client.create(znodePath, Utils.toJSON(properties), CreateMode.PERSISTENT, true);
         }
-      } catch (KeeperException.BadVersionException | KeeperException.NodeExistsException e) {
+      } catch (KeeperException.BadVersionException e) {
         //race condition
+        try {
+          Thread.sleep(50);
+        } catch (InterruptedException e1) {
+          ParWork.propegateInterrupt(e1);
+          return;
+        }
         continue;
       } catch (InterruptedException | KeeperException e) {
         throw new IOException("Error setting property for collection " + collection, SolrZkClient.checkInterrupted(e));
diff --git a/solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java b/solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java
index b96e363..0ee0337 100644
--- a/solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java
+++ b/solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java
@@ -1219,7 +1219,7 @@ public class ZkStateReader implements SolrCloseable {
     }
   }
 
-  static String getCollectionPropsPath(final String collection) {
+  public static String getCollectionPropsPath(final String collection) {
     return COLLECTIONS_ZKNODE + '/' + collection + '/' + COLLECTION_PROPS_ZKNODE;
   }