dsmiley commented on code in PR #3834:
URL: https://github.com/apache/solr/pull/3834#discussion_r2492541935
##########
solr/core/src/java/org/apache/solr/update/processor/RoutedAliasUpdateProcessor.java:
##########
@@ -266,13 +267,13 @@ private List<SolrCmdDistributor.Node>
lookupShardLeadersOfCollections() {
}
private SolrCmdDistributor.Node lookupShardLeaderOfCollection(String
collection) {
- final Slice[] activeSlices =
-
zkController.getClusterState().getCollection(collection).getActiveSlicesArr();
- if (activeSlices.length == 0) {
+ final List<Slice> activeSlices =
+ new
ArrayList<>(zkController.getClusterState().getCollection(collection).getActiveSlices());
Review Comment:
why the ArrayList wrapper?
##########
solr/core/src/java/org/apache/solr/schema/ManagedIndexSchema.java:
##########
@@ -327,8 +327,8 @@ private static List<Replica> getActiveReplicas(
ClusterState clusterState = zkStateReader.getClusterState();
Set<String> liveNodes = clusterState.getLiveNodes();
final DocCollection docCollection =
clusterState.getCollectionOrNull(collection);
- if (docCollection != null && docCollection.getActiveSlicesArr().length >
0) {
- final Slice[] activeSlices = docCollection.getActiveSlicesArr();
+ if (docCollection != null && !docCollection.getActiveSlices().isEmpty()) {
+ final Collection<Slice> activeSlices = docCollection.getActiveSlices();
for (Slice next : activeSlices) {
Review Comment:
This can be much simpler; one line not 3 and avoid the whole conditional
check indentation.
##########
solr/core/src/java/org/apache/solr/update/processor/DocExpirationUpdateProcessorFactory.java:
##########
@@ -464,13 +463,13 @@ private boolean iAmInChargeOfPeriodicDeletes() {
String col = desc.getCollectionName();
DocCollection docCollection = zk.getClusterState().getCollection(col);
- if (docCollection.getActiveSlicesArr().length == 0) {
+ if (docCollection.getActiveSlices().isEmpty()) {
log.error("Collection {} has no active Slices?", col);
return false;
}
- List<Slice> slices = new
ArrayList<>(Arrays.asList(docCollection.getActiveSlicesArr()));
+ List<Slice> slices = new ArrayList<>(docCollection.getActiveSlices());
Review Comment:
we don't need to create a new List and sort. If you look at the code, the
task is to find the first, sorted by name. Streams can do that elegantly.
##########
solr/core/src/java/org/apache/solr/cloud/ZkController.java:
##########
@@ -2069,15 +2070,26 @@ private void waitForShardId(final CoreDescriptor cd) {
if (log.isDebugEnabled()) {
log.debug("waiting to find shard id in clusterstate for {}",
cd.getName());
}
+
+ Predicate<Replica> replicaPredicate = (Replica r) ->
cd.getName().equals(r.getCoreName());
try {
DocCollection collection =
zkStateReader.waitForState(
cd.getCollectionName(),
320,
TimeUnit.SECONDS,
- c -> c != null && c.getShardId(getNodeName(), cd.getName()) !=
null);
+ c ->
+ c != null
+ && c.getReplicasOnNode(getNodeName()) != null
Review Comment:
hmm; maybe 'c.getReplicasOnNode' should return an empty collection to avoid
awkward code like this, which raises potential concerns about double-calling a
method that perhaps might not trivially return something. WDYT?
##########
solr/core/src/java/org/apache/solr/handler/designer/SchemaDesignerAPI.java:
##########
@@ -1119,8 +1120,9 @@ protected Map<String, Object> buildResponse(
Map<String, Object> response = new HashMap<>();
DocCollection coll = zkStateReader().getCollection(mutableId);
- if (coll.getActiveSlicesArr().length > 0) {
- String coreName = coll.getActiveSlicesArr()[0].getLeader().getCoreName();
+ if (!coll.getActiveSlices().isEmpty()) {
+ List<Slice> activeSlices = new ArrayList<>(coll.getActiveSlices());
+ String coreName = activeSlices.getFirst().getLeader().getCoreName();
Review Comment:
why create an ArrayList wrapper?
And can we call getActiveSlices once to avoid potential concerns of what
work it may do?
--
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]
