dsmiley commented on code in PR #2846:
URL: https://github.com/apache/solr/pull/2846#discussion_r1830327967
##########
solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java:
##########
@@ -1115,20 +1110,20 @@ protected String getRemoteCoreUrl(String
collectionName, String origCorename)
ClusterState clusterState = cores.getZkController().getClusterState();
final DocCollection docCollection =
clusterState.getCollectionOrNull(collectionName);
Slice[] slices = (docCollection != null) ?
docCollection.getActiveSlicesArr() : null;
- List<Slice> activeSlices = new ArrayList<>();
+ List<Slice> activeSlices;
Review Comment:
I really liked this little improvement to this method clarify that we
produce the activeSlices once; it's not built up / amended to a growing list
(which wasn't super obvious)
##########
solr/test-framework/src/java/org/apache/solr/common/cloud/ClusterStateUtil.java:
##########
@@ -101,23 +103,32 @@ public static boolean waitForLiveAndActiveReplicaCount(
ZkStateReader zkStateReader, String collection, int replicaCount, int
timeoutInMs) {
return waitFor(
zkStateReader,
- timeoutInMs,
collection,
+ timeoutInMs,
+ TimeUnit.MILLISECONDS,
(liveNodes, state) ->
replicasOfActiveSlicesStream(state)
.filter(replica -> liveAndActivePredicate(replica,
liveNodes))
.count()
== replicaCount);
}
+ /**
+ * Calls {@link ZkStateReader#waitForState(String, long, TimeUnit,
CollectionStatePredicate)} but
+ * has an alternative implementation if {@code collection} is null, in which
the predicate must
+ * match *all* collections. Returns whether the predicate matches or not in
the allotted time;
+ * does *NOT* throw {@link TimeoutException}.
+ */
public static boolean waitFor(
Review Comment:
I aligned the parameters to be closer to that of ZkStateReader.waitForState
##########
solr/core/src/java/org/apache/solr/cloud/api/collections/DeleteCollectionCmd.java:
##########
@@ -168,7 +168,7 @@ public void call(ClusterState state, ZkNodeProps message,
NamedList<Object> resu
}
// delete related config set iff: it is auto generated AND not related
to any other collection
- String configSetName = coll.getConfigName();
+ String configSetName = coll == null ? null : coll.getConfigName();
Review Comment:
IntelliJ helpfully pointed out an issue nearby this PR with a trivial fix.
Basically, it might not have been possible to delete a non-existent collection
but now it might be. (couldn't find a test for this)
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
