You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@solr.apache.org by no...@apache.org on 2023/03/06 05:00:22 UTC

[solr] branch branch_9x updated: Eliminate redundant operation in PRS (#1432)

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

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


The following commit(s) were added to refs/heads/branch_9x by this push:
     new 468b6cdc0a8 Eliminate redundant operation in PRS (#1432)
468b6cdc0a8 is described below

commit 468b6cdc0a88333919cf295479d8a9b3504d8e25
Author: Noble Paul <no...@users.noreply.github.com>
AuthorDate: Mon Mar 6 15:34:42 2023 +1100

    Eliminate redundant operation in PRS (#1432)
---
 .../apache/solr/cloud/overseer/ZkStateWriter.java  |  8 -------
 .../solr/cloud/overseer/ZkStateReaderTest.java     | 12 +++++-----
 .../solr/common/cloud/PerReplicaStatesOps.java     | 26 ----------------------
 3 files changed, 6 insertions(+), 40 deletions(-)

diff --git a/solr/core/src/java/org/apache/solr/cloud/overseer/ZkStateWriter.java b/solr/core/src/java/org/apache/solr/cloud/overseer/ZkStateWriter.java
index 9cd2089e881..b3ad7bf61fb 100644
--- a/solr/core/src/java/org/apache/solr/cloud/overseer/ZkStateWriter.java
+++ b/solr/core/src/java/org/apache/solr/cloud/overseer/ZkStateWriter.java
@@ -31,7 +31,6 @@ import org.apache.solr.cloud.Stats;
 import org.apache.solr.common.cloud.ClusterState;
 import org.apache.solr.common.cloud.DocCollection;
 import org.apache.solr.common.cloud.PerReplicaStatesFetcher;
-import org.apache.solr.common.cloud.PerReplicaStatesOps;
 import org.apache.solr.common.cloud.ZkStateReader;
 import org.apache.solr.common.util.Compressor;
 import org.apache.solr.common.util.Utils;
@@ -318,14 +317,7 @@ public class ZkStateWriter {
             }
           }
 
-          // When dealing with a per replica collection that did not do any update to the per
-          // replica states znodes but did update state.json, we add then remove a dummy node to
-          // change the cversion of the parent znode. This is not needed by Solr, there's no code
-          // watching the children and not watching the state.json node itself. It would be useful
-          // for external code watching the collection's Zookeeper state.json node children but not
-          // the node itself.
           if (cmd.ops == null && cmd.isPerReplicaStateCollection) {
-            PerReplicaStatesOps.touchChildren().persist(path, reader.getZkClient());
             DocCollection currentCollState = clusterState.getCollection(cmd.name);
             if (currentCollState != null) {
               clusterState =
diff --git a/solr/core/src/test/org/apache/solr/cloud/overseer/ZkStateReaderTest.java b/solr/core/src/test/org/apache/solr/cloud/overseer/ZkStateReaderTest.java
index 38886c5e35f..b2b98fa1ea4 100644
--- a/solr/core/src/test/org/apache/solr/cloud/overseer/ZkStateReaderTest.java
+++ b/solr/core/src/test/org/apache/solr/cloud/overseer/ZkStateReaderTest.java
@@ -287,8 +287,8 @@ public class ZkStateReaderTest extends SolrTestCaseJ4 {
     ClusterState.CollectionRef ref = reader.getClusterState().getCollectionRef("c1");
     assertFalse(ref.isLazilyLoaded());
     assertEquals(0, ref.get().getZNodeVersion());
-    // dummy node created +1 and deleted +1 so 2
-    assertEquals(2, ref.get().getChildNodesVersion());
+    // no more dummy node
+    assertEquals(0, ref.get().getChildNodesVersion());
 
     DocCollection collection = ref.get();
     PerReplicaStates prs =
@@ -307,7 +307,7 @@ public class ZkStateReaderTest extends SolrTestCaseJ4 {
 
     ref = reader.getClusterState().getCollectionRef("c1");
     assertEquals(0, ref.get().getZNodeVersion()); // no change in Znode version
-    assertEquals(3, ref.get().getChildNodesVersion()); // but child version should be 1 now
+    assertEquals(1, ref.get().getChildNodesVersion()); // but child version should be 1 now
 
     prs = ref.get().getPerReplicaStates();
     PerReplicaStatesOps.flipState("r1", Replica.State.ACTIVE, prs)
@@ -321,7 +321,7 @@ public class ZkStateReaderTest extends SolrTestCaseJ4 {
     ref = reader.getClusterState().getCollectionRef("c1");
     assertEquals(0, ref.get().getZNodeVersion()); // no change in Znode version
     // but child version should be 3 now (1 del + 1 add)
-    assertEquals(5, ref.get().getChildNodesVersion());
+    assertEquals(3, ref.get().getChildNodesVersion());
 
     // now delete the collection
     wc = new ZkWriteCommand("c1", null);
@@ -346,7 +346,7 @@ public class ZkStateReaderTest extends SolrTestCaseJ4 {
     ref = reader.getClusterState().getCollectionRef("c1");
     assertFalse(ref.isLazilyLoaded());
     assertEquals(0, ref.get().getZNodeVersion());
-    assertEquals(2, ref.get().getChildNodesVersion()); // child node version is reset
+    assertEquals(0, ref.get().getChildNodesVersion()); // child node version is reset
 
     // re-add PRS
     collection = ref.get();
@@ -367,7 +367,7 @@ public class ZkStateReaderTest extends SolrTestCaseJ4 {
     ref = reader.getClusterState().getCollectionRef("c1");
 
     // child version should be reset since the state.json node was deleted and re-created
-    assertEquals(3, ref.get().getChildNodesVersion());
+    assertEquals(1, ref.get().getChildNodesVersion());
   }
 
   public void testForciblyRefreshAllClusterState() throws Exception {
diff --git a/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/PerReplicaStatesOps.java b/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/PerReplicaStatesOps.java
index a21d3be9387..e85646d7207 100644
--- a/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/PerReplicaStatesOps.java
+++ b/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/PerReplicaStatesOps.java
@@ -29,7 +29,6 @@ import java.util.function.Function;
 import org.apache.zookeeper.CreateMode;
 import org.apache.zookeeper.KeeperException;
 import org.apache.zookeeper.Op;
-import org.apache.zookeeper.data.Stat;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -299,31 +298,6 @@ public class PerReplicaStatesOps {
         .init(rs);
   }
 
-  /**
-   * Just creates and deletes a dummy entry so that the {@link Stat#getCversion()} of state.json is
-   * updated
-   */
-  public static PerReplicaStatesOps touchChildren() {
-    PerReplicaStatesOps result =
-        new PerReplicaStatesOps(
-            prs -> {
-              List<PerReplicaStates.Operation> operations = new ArrayList<>(2);
-              PerReplicaStates.State st =
-                  new PerReplicaStates.State(
-                      ".dummy." + System.nanoTime(), Replica.State.DOWN, Boolean.FALSE, 0);
-              operations.add(
-                  new PerReplicaStates.Operation(PerReplicaStates.Operation.Type.ADD, st));
-              operations.add(
-                  new PerReplicaStates.Operation(PerReplicaStates.Operation.Type.DELETE, st));
-              if (log.isDebugEnabled()) {
-                log.debug("touchChildren {}", operations);
-              }
-              return operations;
-            });
-    result.ops = result.refresh(null);
-    return result;
-  }
-
   PerReplicaStatesOps init(PerReplicaStates rs) {
     if (rs == null) return null;
     get(rs);