madrob commented on a change in pull request #644:
URL: https://github.com/apache/solr/pull/644#discussion_r807518186
##########
File path: solr/core/src/java/org/apache/solr/cloud/Overseer.java
##########
@@ -19,14 +19,7 @@
import java.io.Closeable;
import java.io.IOException;
import java.lang.invoke.MethodHandles;
-import java.util.Collection;
-import java.util.Collections;
-import java.util.HashSet;
-import java.util.LinkedList;
-import java.util.List;
-import java.util.Map;
-import java.util.Optional;
-import java.util.Set;
+import java.util.*;
Review comment:
please configure your editor to not do this
##########
File path:
solr/core/src/java/org/apache/solr/cloud/overseer/CollectionMutator.java
##########
@@ -160,6 +164,38 @@ public ZkWriteCommand modifyCollection(final ClusterState
clusterState, ZkNodePr
}
}
+ /**
+ * Update the state/leader of replicas in state.json from PRS data
+ */
+ public static DocCollection updateReplicasFromPrs(DocCollection coll,
PerReplicaStates prs) {
+ //we are disabling PRS. Update the replica states
+ Map<String, Slice> modifiedSlices = new LinkedHashMap<>();
+ coll.forEachReplica((s, replica) -> {
+ PerReplicaStates.State prsState = prs.states.get(replica.getName());
+ if (prsState != null) {
+ if (prsState.state != replica.getState()) {
Review comment:
This if statement and the next one look like they are almost doing the
same thing, is there a way to simplify this logic so that we are only
getting/setting from modifiedSlices once?
##########
File path:
solr/core/src/java/org/apache/solr/cloud/overseer/CollectionMutator.java
##########
@@ -55,7 +55,8 @@ public ZkWriteCommand createShard(final ClusterState
clusterState, ZkNodeProps m
DocCollection collection = clusterState.getCollection(collectionName);
Slice slice = collection.getSlice(shardId);
if (slice == null) {
- Map<String, Replica> replicas = Collections.emptyMap();
+ @SuppressWarnings({"unchecked"})
+ Map<String, Replica> replicas = Collections.EMPTY_MAP;
Review comment:
why?
##########
File path:
solr/core/src/java/org/apache/solr/cloud/overseer/CollectionMutator.java
##########
@@ -113,10 +114,13 @@ public ZkWriteCommand modifyCollection(final ClusterState
clusterState, ZkNodePr
log.error("trying to set perReplicaState to {} from {}", val,
coll.isPerReplicaState());
continue;
}
- replicaOps = PerReplicaStatesOps.modifyCollection(coll, enable,
PerReplicaStates.fetch(coll.getZNode(), zkClient, null));
+ PerReplicaStates prs = PerReplicaStates.fetch(coll.getZNode(),
zkClient, null);
+ replicaOps = PerReplicaStatesOps.modifyCollection(coll, enable, prs);
+ if(!enable) {
+ coll = updateReplicasFromPrs(coll, prs);
+ }
Review comment:
Is there still a race condition here? What happens if a replica goes
down (or changes leadership) between the fetch and the persist which happens
much later?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@solr.apache.org
For additional commands, e-mail: issues-h...@solr.apache.org
