murblanc commented on a change in pull request #2318:
URL: https://github.com/apache/lucene-solr/pull/2318#discussion_r572075066
##########
File path:
solr/solrj/src/java/org/apache/solr/common/cloud/PerReplicaStates.java
##########
@@ -92,6 +94,17 @@ public PerReplicaStates(String path, int cversion,
List<String> states) {
}
+ /** Check and return if all replicas are ACTIVE
+ */
+ public boolean allActive() {
+ if (this.allActive != null) return allActive;
+ boolean[] result = new boolean[]{true};
Review comment:
`AtomicBoolean` would likely make code easier to read.
##########
File path:
solr/core/src/java/org/apache/solr/cloud/api/collections/CreateCollectionCmd.java
##########
@@ -264,8 +264,11 @@ public void call(ClusterState clusterState, ZkNodeProps
message, @SuppressWarnin
log.info("Cleaned up artifacts for failed create collection for [{}]",
collectionName);
throw new SolrException(ErrorCode.BAD_REQUEST, "Underlying core
creation failed while creating collection: " + collectionName);
} else {
+ //we want to wait till all the replicas are ACTIVE for PRS collections
because
+ ocmh.zkStateReader.waitForState(collectionName, 30,
TimeUnit.SECONDS, (liveNodes, c) ->
+ c.getPerReplicaStates() == null || // this is not a PRS
collection
Review comment:
Shouldn't that test about collection not being PRS be made before the
call to `waitForState` to just skip that call?
##########
File path:
solr/solrj/src/java/org/apache/solr/common/cloud/PerReplicaStates.java
##########
@@ -92,6 +94,17 @@ public PerReplicaStates(String path, int cversion,
List<String> states) {
}
+ /** Check and return if all replicas are ACTIVE
+ */
+ public boolean allActive() {
+ if (this.allActive != null) return allActive;
Review comment:
This method caches the value computed the first time it's called, then
returns that value on each subsequent call.
When used as part of the predicate in `CollectionStateWatcher`, even if the
watch fires multiple times wouldn't it always return the first computed state
(which is likely `false` as all replicas are not active initially)?
##########
File path:
solr/core/src/java/org/apache/solr/cloud/api/collections/CreateCollectionCmd.java
##########
@@ -264,8 +264,11 @@ public void call(ClusterState clusterState, ZkNodeProps
message, @SuppressWarnin
log.info("Cleaned up artifacts for failed create collection for [{}]",
collectionName);
throw new SolrException(ErrorCode.BAD_REQUEST, "Underlying core
creation failed while creating collection: " + collectionName);
} else {
+ //we want to wait till all the replicas are ACTIVE for PRS collections
because
Review comment:
Truncated comment
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
