Switching a PRS collection from true -> false -> true results in INAC…

…TIVE replicas (#413)
apache · Nov 15, 2021 · f088857 · f088857
1 parent 9ee30f1
commit f088857
Show file tree

Hide file tree

Showing 4 changed files with 75 additions and 3 deletions.
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
@@ -417,6 +417,13 @@ Bug Fixes
 
 * SOLR-15783: Prevent Logging MDC values from leaking between request threads, and set 'trace_id' in MDC as soon as it's available (hossman)
 
+==================  8.11.1 ==================
+
+Bug Fixes
+---------------------
+* SOLR-15794: Switching a PRS collection from true -> false -> true results in INACTIVE replicas (noble)
+
+
 ==================  8.11.0 ==================
 
 Consult the LUCENE_CHANGES.txt file for additional, low level, changes in this release.

diff --git a/solr/solrj/src/java/org/apache/solr/common/cloud/PerReplicaStates.java b/solr/solrj/src/java/org/apache/solr/common/cloud/PerReplicaStates.java
@@ -320,6 +320,10 @@ public boolean equals(Object o) {
     public int hashCode() {
       return asString.hashCode();
     }
+
+    public State getDuplicate() {
+      return duplicate;
+    }
   }
 
 }
diff --git a/solr/solrj/src/java/org/apache/solr/common/cloud/PerReplicaStatesOps.java b/solr/solrj/src/java/org/apache/solr/common/cloud/PerReplicaStatesOps.java
@@ -134,13 +134,27 @@ public static PerReplicaStatesOps flipState(String replica, Replica.State newSta
      * Switch a collection from/to perReplicaState=true
      */
     public static PerReplicaStatesOps modifyCollection(DocCollection coll, boolean enable, PerReplicaStates rs) {
-        return new PerReplicaStatesOps(prs -> enable ? enable(coll) : disable(prs)).init(rs);
+        return new PerReplicaStatesOps(prs -> enable ?
+                enable(coll,prs) :
+                disable(prs))
+                .init(rs);
 
     }
 
-    private static List<PerReplicaStates.Operation> enable(DocCollection coll) {
+    private static List<PerReplicaStates.Operation> enable(DocCollection coll, PerReplicaStates prs) {
+        log.info("ENABLING_PRS ");
         List<PerReplicaStates.Operation> result = new ArrayList<>();
-        coll.forEachReplica((s, r) -> result.add(new PerReplicaStates.Operation(PerReplicaStates.Operation.Type.ADD, new PerReplicaStates.State(r.getName(), r.getState(), r.isLeader(), 0))));
+        coll.forEachReplica((s, r) -> {
+            PerReplicaStates.State st = prs.get(r.getName());
+            int newVer = 0;
+            if (st != null) {
+                result.add(new PerReplicaStates.Operation(PerReplicaStates.Operation.Type.DELETE, st));
+                newVer = st.version + 1;
+            }
+            result.add(new PerReplicaStates.Operation(PerReplicaStates.Operation.Type.ADD,
+                    new PerReplicaStates.State(r.getName(), r.getState(), r.isLeader(), newVer)));
+        });
+        log.info("ENABLING_PRS OPS {}", result);
         return result;
     }
 

diff --git a/solr/solrj/src/test/org/apache/solr/common/cloud/PerReplicaStatesIntegrationTest.java b/solr/solrj/src/test/org/apache/solr/common/cloud/PerReplicaStatesIntegrationTest.java
@@ -20,6 +20,9 @@
 
 import java.lang.invoke.MethodHandles;
 
+import java.util.Collections;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
 import org.apache.lucene.util.LuceneTestCase.Slow;
 import org.apache.solr.client.solrj.SolrClient;
 import org.apache.solr.client.solrj.embedded.JettySolrRunner;
@@ -33,6 +36,7 @@
 import org.slf4j.LoggerFactory;
 
 import static org.apache.solr.client.solrj.SolrRequest.METHOD.POST;
+import static org.apache.solr.common.cloud.DocCollection.PER_REPLICA_STATE;
 
 
 /**
@@ -155,4 +159,47 @@ public void testRestart() throws Exception {
     }
 
   }
+
+  public void testMultipleTransitions() throws Exception {
+    String COLL = "prs_modify_op_coll";
+    MiniSolrCloudCluster cluster =
+            configureCluster(2)
+                    .withJettyConfig(jetty -> jetty.enableV2(true))
+                    .addConfig("conf", getFile("solrj").toPath().resolve("solr").resolve("configsets").resolve("streaming").resolve("conf"))
+                    .configure();
+    PerReplicaStates original = null;
+    try {
+      CollectionAdminRequest.createCollection(COLL, "conf", 3, 1)
+              .setPerReplicaState(Boolean.TRUE)
+              .process(cluster.getSolrClient());
+      cluster.waitForActiveCollection(COLL, 3, 3);
+
+      PerReplicaStates prs1 = original =  PerReplicaStates.fetch(ZkStateReader.getCollectionPath(COLL), cluster.getZkClient(), null);
+      log.info("prs1 : {}", prs1);
+
+      CollectionAdminRequest.modifyCollection(COLL,
+              Collections.singletonMap(PER_REPLICA_STATE, "false"))
+              .process(cluster.getSolrClient());
+      cluster.getSolrClient().getZkStateReader().waitForState(COLL, 5, TimeUnit.SECONDS,
+              (liveNodes, collectionState) -> "false".equals(collectionState.getProperties().get(PER_REPLICA_STATE)));
+      CollectionAdminRequest.modifyCollection(COLL,
+              Collections.singletonMap(PER_REPLICA_STATE, "true"))
+              .process(cluster.getSolrClient());
+      cluster.getSolrClient().getZkStateReader().waitForState(COLL, 5, TimeUnit.SECONDS, (liveNodes, collectionState) -> {
+        AtomicBoolean anyFail = new AtomicBoolean(false);
+        PerReplicaStates prs2 = PerReplicaStates.fetch(ZkStateReader.getCollectionPath(COLL), cluster.getZkClient(), null);
+        prs2.states.forEachEntry((r, newState) -> {
+          if(newState.getDuplicate() !=null) anyFail.set(true);
+        });
+        return !anyFail.get();
+      });
+
+    } finally {
+      System.out.println("prs1 : "+ original);
+      System.out.println("prs2 : "+ PerReplicaStates.fetch(ZkStateReader.getCollectionPath(COLL), cluster.getZkClient(), null));
+      cluster.shutdown();
+
+    }
+
+  }
 }