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 02:54:22 UTC

[solr] branch jira/no_touchchildren created (now bc3866085d7)

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

noble pushed a change to branch jira/no_touchchildren
in repository https://gitbox.apache.org/repos/asf/solr.git


      at bc3866085d7 touchChildren is redundant

This branch includes the following new commits:

     new bc3866085d7 touchChildren is redundant

The 1 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.



[solr] 01/01: touchChildren is redundant

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

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

commit bc3866085d7068aef33be7a8242ca368508e1cf9
Author: Noble Paul <no...@gmail.com>
AuthorDate: Mon Mar 6 13:54:12 2023 +1100

    touchChildren is redundant
---
 .../apache/solr/cloud/overseer/ZkStateWriter.java  |  7 ------
 .../solr/cloud/overseer/ZkStateReaderTest.java     | 16 ++++++-------
 .../solr/common/cloud/PerReplicaStatesOps.java     | 27 +---------------------
 3 files changed, 9 insertions(+), 41 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..0163dbbef8e 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
@@ -318,14 +318,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..3e4c6c72c41 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
@@ -72,7 +72,7 @@ public class ZkStateReaderTest extends SolrTestCaseJ4 {
     private final ZkStateWriter writer;
 
     private TestFixture(
-        ZkTestServer server, SolrZkClient zkClient, ZkStateReader reader, ZkStateWriter writer) {
+            ZkTestServer server, SolrZkClient zkClient, ZkStateReader reader, ZkStateWriter writer) {
       this.server = server;
       this.zkClient = zkClient;
       this.reader = reader;
@@ -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 {
@@ -665,4 +665,4 @@ public class ZkStateReaderTest extends SolrTestCaseJ4 {
       ExecutorUtil.awaitTermination(executorService);
     }
   }
-}
+}
\ No newline at end of file
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..067e0d79b21 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
@@ -299,32 +299,7 @@ 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) {
+    PerReplicaStatesOps init(PerReplicaStates rs) {
     if (rs == null) return null;
     get(rs);
     return this;