From f64e3d35c162565adf91ba5491c63dd2c39b16f8 Mon Sep 17 00:00:00 2001 From: Matthew Biscocho Date: Tue, 17 Dec 2024 09:53:54 -0500 Subject: [PATCH 1/7] Move collections properties into an iterable to stream --- .../solr/handler/admin/ClusterStatus.java | 91 +++++++++++-------- 1 file changed, 51 insertions(+), 40 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/handler/admin/ClusterStatus.java b/solr/core/src/java/org/apache/solr/handler/admin/ClusterStatus.java index 18c8843f916..ef50d1b0e2f 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/ClusterStatus.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/ClusterStatus.java @@ -22,6 +22,7 @@ import java.util.Collections; import java.util.HashMap; import java.util.HashSet; +import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Objects; @@ -205,54 +206,64 @@ private void fetchClusterStatusForCollOrAlias( } } - // TODO use an Iterable to stream the data to the client instead of gathering it all in mem + Iterator> collectionsResponse = + new Iterator<>() { + final Iterator it = collectionStream.iterator(); - NamedList collectionProps = new SimpleOrderedMap<>(); - - collectionStream.forEach( - clusterStateCollection -> { - Map collectionStatus; - String name = clusterStateCollection.getName(); + @Override + public boolean hasNext() { + return it.hasNext(); + } - Set requestedShards = new HashSet<>(); - if (routeKey != null) { - DocRouter router = clusterStateCollection.getRouter(); - Collection slices = - router.getSearchSlices(routeKey, null, clusterStateCollection); - for (Slice slice : slices) { - requestedShards.add(slice.getName()); + @Override + public NamedList next() { + NamedList collectionProps = new SimpleOrderedMap<>(); + DocCollection clusterStateCollection = it.next(); + Map collectionStatus; + String name = clusterStateCollection.getName(); + + Set requestedShards = new HashSet<>(); + if (routeKey != null) { + DocRouter router = clusterStateCollection.getRouter(); + Collection slices = + router.getSearchSlices(routeKey, null, clusterStateCollection); + for (Slice slice : slices) { + requestedShards.add(slice.getName()); + } + } + if (shard != null) { + String[] paramShards = shard.split(","); + requestedShards.addAll(Arrays.asList(paramShards)); } - } - if (shard != null) { - String[] paramShards = shard.split(","); - requestedShards.addAll(Arrays.asList(paramShards)); - } - byte[] bytes = Utils.toJSON(clusterStateCollection); - @SuppressWarnings("unchecked") - Map docCollection = (Map) Utils.fromJSON(bytes); - collectionStatus = getCollectionStatus(docCollection, name, requestedShards); + byte[] bytes = Utils.toJSON(clusterStateCollection); + @SuppressWarnings("unchecked") + Map docCollection = (Map) Utils.fromJSON(bytes); + collectionStatus = getCollectionStatus(docCollection, name, requestedShards); - collectionStatus.put("znodeVersion", clusterStateCollection.getZNodeVersion()); - collectionStatus.put( - "creationTimeMillis", clusterStateCollection.getCreationTime().toEpochMilli()); + collectionStatus.put("znodeVersion", clusterStateCollection.getZNodeVersion()); + collectionStatus.put( + "creationTimeMillis", clusterStateCollection.getCreationTime().toEpochMilli()); - if (collectionVsAliases.containsKey(name) && !collectionVsAliases.get(name).isEmpty()) { - collectionStatus.put("aliases", collectionVsAliases.get(name)); - } - String configName = clusterStateCollection.getConfigName(); - collectionStatus.put("configName", configName); - if (solrParams.getBool("prs", false) && clusterStateCollection.isPerReplicaState()) { - PerReplicaStates prs = clusterStateCollection.getPerReplicaStates(); - collectionStatus.put("PRS", prs); - } - collectionProps.add(name, collectionStatus); - }); + if (collectionVsAliases.containsKey(name) && !collectionVsAliases.get(name).isEmpty()) { + collectionStatus.put("aliases", collectionVsAliases.get(name)); + } + String configName = clusterStateCollection.getConfigName(); + collectionStatus.put("configName", configName); + if (solrParams.getBool("prs", false) && clusterStateCollection.isPerReplicaState()) { + PerReplicaStates prs = clusterStateCollection.getPerReplicaStates(); + collectionStatus.put("PRS", prs); + } + collectionProps.add(name, collectionStatus); - // now we need to walk the collectionProps tree to cross-check replica state with live nodes - crossCheckReplicaStateWithLiveNodes(liveNodes, collectionProps); + // now we need to walk the collectionProps tree to cross-check replica state with live + // nodes + crossCheckReplicaStateWithLiveNodes(liveNodes, collectionProps); + return collectionProps; + } + }; - clusterStatus.add("collections", collectionProps); + clusterStatus.add("collections", collectionsResponse); } private void addAliasMap(Aliases aliases, NamedList clusterStatus) { From bbbc62d64b1c59cbc2a7973b4765c086955b9084 Mon Sep 17 00:00:00 2001 From: Matthew Biscocho Date: Tue, 17 Dec 2024 11:24:45 -0500 Subject: [PATCH 2/7] Change name to collectionPropsIt --- .../src/java/org/apache/solr/handler/admin/ClusterStatus.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/handler/admin/ClusterStatus.java b/solr/core/src/java/org/apache/solr/handler/admin/ClusterStatus.java index ef50d1b0e2f..3ae1de98f8a 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/ClusterStatus.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/ClusterStatus.java @@ -206,7 +206,7 @@ private void fetchClusterStatusForCollOrAlias( } } - Iterator> collectionsResponse = + Iterator> collectionPropsIt = new Iterator<>() { final Iterator it = collectionStream.iterator(); @@ -263,7 +263,7 @@ public NamedList next() { } }; - clusterStatus.add("collections", collectionsResponse); + clusterStatus.add("collections", collectionPropsIt); } private void addAliasMap(Aliases aliases, NamedList clusterStatus) { From 6ed4a95b6399f9f867c1c7cc1861c6fa84dfb1b5 Mon Sep 17 00:00:00 2001 From: Matthew Biscocho Date: Wed, 18 Dec 2024 09:41:26 -0500 Subject: [PATCH 3/7] Use MapWriter --- .../solr/handler/admin/ClusterStatus.java | 137 +++++++++++------- 1 file changed, 86 insertions(+), 51 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/handler/admin/ClusterStatus.java b/solr/core/src/java/org/apache/solr/handler/admin/ClusterStatus.java index 3ae1de98f8a..bf4a3bcfe34 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/ClusterStatus.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/ClusterStatus.java @@ -16,6 +16,7 @@ */ package org.apache.solr.handler.admin; +import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -28,6 +29,7 @@ import java.util.Objects; import java.util.Set; import java.util.stream.Stream; +import org.apache.solr.common.MapWriter; import org.apache.solr.common.SolrException; import org.apache.solr.common.cloud.Aliases; import org.apache.solr.common.cloud.ClusterState; @@ -206,64 +208,97 @@ private void fetchClusterStatusForCollOrAlias( } } - Iterator> collectionPropsIt = - new Iterator<>() { - final Iterator it = collectionStream.iterator(); - - @Override - public boolean hasNext() { - return it.hasNext(); + MapWriter mw = + ew -> { + SolrCollectionPropsStreamer scps = + new SolrCollectionPropsStreamer( + collectionStream.iterator(), collectionVsAliases, routeKey, liveNodes, shard); + while (scps.hasNext()) { + Map output = (scps.next().asMap()); + output.forEach( + (key, value) -> { + try { + ew.put(key, value); + } catch (IOException e) { + throw new RuntimeException(e); + } + }); } + }; + clusterStatus.add("collections", mw); + } - @Override - public NamedList next() { - NamedList collectionProps = new SimpleOrderedMap<>(); - DocCollection clusterStateCollection = it.next(); - Map collectionStatus; - String name = clusterStateCollection.getName(); - - Set requestedShards = new HashSet<>(); - if (routeKey != null) { - DocRouter router = clusterStateCollection.getRouter(); - Collection slices = - router.getSearchSlices(routeKey, null, clusterStateCollection); - for (Slice slice : slices) { - requestedShards.add(slice.getName()); - } - } - if (shard != null) { - String[] paramShards = shard.split(","); - requestedShards.addAll(Arrays.asList(paramShards)); - } + public class SolrCollectionPropsStreamer implements Iterator> { + + final Iterator it; + Map> collectionVsAliases; + String routeKey; + List liveNodes; + String shard; + + public SolrCollectionPropsStreamer( + Iterator it, + Map> collectionVsAliases, + String routeKey, + List liveNodes, + String shard) { + this.it = it; + this.collectionVsAliases = collectionVsAliases; + this.routeKey = routeKey; + this.liveNodes = liveNodes; + this.shard = shard; + } + + @Override + public boolean hasNext() { + return it.hasNext(); + } - byte[] bytes = Utils.toJSON(clusterStateCollection); - @SuppressWarnings("unchecked") - Map docCollection = (Map) Utils.fromJSON(bytes); - collectionStatus = getCollectionStatus(docCollection, name, requestedShards); + @Override + public NamedList next() { + NamedList collectionProps = new SimpleOrderedMap<>(); + DocCollection clusterStateCollection = it.next(); + Map collectionStatus; + String name = clusterStateCollection.getName(); + + Set requestedShards = new HashSet<>(); + if (routeKey != null) { + DocRouter router = clusterStateCollection.getRouter(); + Collection slices = router.getSearchSlices(routeKey, null, clusterStateCollection); + for (Slice slice : slices) { + requestedShards.add(slice.getName()); + } + } + if (shard != null) { + String[] paramShards = shard.split(","); + requestedShards.addAll(Arrays.asList(paramShards)); + } - collectionStatus.put("znodeVersion", clusterStateCollection.getZNodeVersion()); - collectionStatus.put( - "creationTimeMillis", clusterStateCollection.getCreationTime().toEpochMilli()); + byte[] bytes = Utils.toJSON(clusterStateCollection); + @SuppressWarnings("unchecked") + Map docCollection = (Map) Utils.fromJSON(bytes); + collectionStatus = getCollectionStatus(docCollection, name, requestedShards); - if (collectionVsAliases.containsKey(name) && !collectionVsAliases.get(name).isEmpty()) { - collectionStatus.put("aliases", collectionVsAliases.get(name)); - } - String configName = clusterStateCollection.getConfigName(); - collectionStatus.put("configName", configName); - if (solrParams.getBool("prs", false) && clusterStateCollection.isPerReplicaState()) { - PerReplicaStates prs = clusterStateCollection.getPerReplicaStates(); - collectionStatus.put("PRS", prs); - } - collectionProps.add(name, collectionStatus); + collectionStatus.put("znodeVersion", clusterStateCollection.getZNodeVersion()); + collectionStatus.put( + "creationTimeMillis", clusterStateCollection.getCreationTime().toEpochMilli()); - // now we need to walk the collectionProps tree to cross-check replica state with live - // nodes - crossCheckReplicaStateWithLiveNodes(liveNodes, collectionProps); - return collectionProps; - } - }; + if (collectionVsAliases.containsKey(name) && !collectionVsAliases.get(name).isEmpty()) { + collectionStatus.put("aliases", collectionVsAliases.get(name)); + } + String configName = clusterStateCollection.getConfigName(); + collectionStatus.put("configName", configName); + if (solrParams.getBool("prs", false) && clusterStateCollection.isPerReplicaState()) { + PerReplicaStates prs = clusterStateCollection.getPerReplicaStates(); + collectionStatus.put("PRS", prs); + } + collectionProps.add(name, collectionStatus); - clusterStatus.add("collections", collectionPropsIt); + // now we need to walk the collectionProps tree to cross-check replica state with live + // nodes + crossCheckReplicaStateWithLiveNodes(liveNodes, collectionProps); + return collectionProps; + } } private void addAliasMap(Aliases aliases, NamedList clusterStatus) { From 3627bda189f21aa18ef0dec3a5146b653e8fe5e4 Mon Sep 17 00:00:00 2001 From: Matthew Biscocho Date: Wed, 18 Dec 2024 11:15:04 -0500 Subject: [PATCH 4/7] Fix tests where it used namedList --- .../java/org/apache/solr/cli/StatusTool.java | 2 +- .../solr/handler/admin/ClusterStatus.java | 160 +++++++++--------- .../api/collections/TestCollectionAPI.java | 34 ++-- .../impl/BaseHttpClusterStateProvider.java | 9 +- .../solrj/impl/ClusterStateProviderTest.java | 2 +- 5 files changed, 106 insertions(+), 101 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/cli/StatusTool.java b/solr/core/src/java/org/apache/solr/cli/StatusTool.java index 45713fbc085..acf1f3bc7db 100644 --- a/solr/core/src/java/org/apache/solr/cli/StatusTool.java +++ b/solr/core/src/java/org/apache/solr/cli/StatusTool.java @@ -352,7 +352,7 @@ protected Map getCloudStatus(SolrClient solrClient, String zkHos cloudStatus.put("liveNodes", String.valueOf(liveNodes.size())); Map collections = - ((NamedList) json.findRecursive("cluster", "collections")).asMap(); + (Map) json.findRecursive("cluster", "collections"); cloudStatus.put("collections", String.valueOf(collections.size())); return cloudStatus; diff --git a/solr/core/src/java/org/apache/solr/handler/admin/ClusterStatus.java b/solr/core/src/java/org/apache/solr/handler/admin/ClusterStatus.java index bf4a3bcfe34..db7910bb210 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/ClusterStatus.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/ClusterStatus.java @@ -208,14 +208,14 @@ private void fetchClusterStatusForCollOrAlias( } } - MapWriter mw = + MapWriter collectionPropsWriter = ew -> { - SolrCollectionPropsStreamer scps = - new SolrCollectionPropsStreamer( + SolrCollectionProperiesIterator collectionProps = + new SolrCollectionProperiesIterator( collectionStream.iterator(), collectionVsAliases, routeKey, liveNodes, shard); - while (scps.hasNext()) { - Map output = (scps.next().asMap()); - output.forEach( + while (collectionProps.hasNext()) { + Map collectionPropsMap = (collectionProps.next().asMap()); + collectionPropsMap.forEach( (key, value) -> { try { ew.put(key, value); @@ -225,80 +225,7 @@ private void fetchClusterStatusForCollOrAlias( }); } }; - clusterStatus.add("collections", mw); - } - - public class SolrCollectionPropsStreamer implements Iterator> { - - final Iterator it; - Map> collectionVsAliases; - String routeKey; - List liveNodes; - String shard; - - public SolrCollectionPropsStreamer( - Iterator it, - Map> collectionVsAliases, - String routeKey, - List liveNodes, - String shard) { - this.it = it; - this.collectionVsAliases = collectionVsAliases; - this.routeKey = routeKey; - this.liveNodes = liveNodes; - this.shard = shard; - } - - @Override - public boolean hasNext() { - return it.hasNext(); - } - - @Override - public NamedList next() { - NamedList collectionProps = new SimpleOrderedMap<>(); - DocCollection clusterStateCollection = it.next(); - Map collectionStatus; - String name = clusterStateCollection.getName(); - - Set requestedShards = new HashSet<>(); - if (routeKey != null) { - DocRouter router = clusterStateCollection.getRouter(); - Collection slices = router.getSearchSlices(routeKey, null, clusterStateCollection); - for (Slice slice : slices) { - requestedShards.add(slice.getName()); - } - } - if (shard != null) { - String[] paramShards = shard.split(","); - requestedShards.addAll(Arrays.asList(paramShards)); - } - - byte[] bytes = Utils.toJSON(clusterStateCollection); - @SuppressWarnings("unchecked") - Map docCollection = (Map) Utils.fromJSON(bytes); - collectionStatus = getCollectionStatus(docCollection, name, requestedShards); - - collectionStatus.put("znodeVersion", clusterStateCollection.getZNodeVersion()); - collectionStatus.put( - "creationTimeMillis", clusterStateCollection.getCreationTime().toEpochMilli()); - - if (collectionVsAliases.containsKey(name) && !collectionVsAliases.get(name).isEmpty()) { - collectionStatus.put("aliases", collectionVsAliases.get(name)); - } - String configName = clusterStateCollection.getConfigName(); - collectionStatus.put("configName", configName); - if (solrParams.getBool("prs", false) && clusterStateCollection.isPerReplicaState()) { - PerReplicaStates prs = clusterStateCollection.getPerReplicaStates(); - collectionStatus.put("PRS", prs); - } - collectionProps.add(name, collectionStatus); - - // now we need to walk the collectionProps tree to cross-check replica state with live - // nodes - crossCheckReplicaStateWithLiveNodes(liveNodes, collectionProps); - return collectionProps; - } + clusterStatus.add("collections", collectionPropsWriter); } private void addAliasMap(Aliases aliases, NamedList clusterStatus) { @@ -414,4 +341,77 @@ public static Map postProcessCollectionJSON(Map collection.put("health", Health.combine(healthStates).toString()); return collection; } + + private class SolrCollectionProperiesIterator implements Iterator> { + + final Iterator it; + Map> collectionVsAliases; + String routeKey; + List liveNodes; + String shard; + + public SolrCollectionProperiesIterator( + Iterator it, + Map> collectionVsAliases, + String routeKey, + List liveNodes, + String shard) { + this.it = it; + this.collectionVsAliases = collectionVsAliases; + this.routeKey = routeKey; + this.liveNodes = liveNodes; + this.shard = shard; + } + + @Override + public boolean hasNext() { + return it.hasNext(); + } + + @Override + public NamedList next() { + NamedList collectionProps = new SimpleOrderedMap<>(); + DocCollection clusterStateCollection = it.next(); + Map collectionStatus; + String name = clusterStateCollection.getName(); + + Set requestedShards = new HashSet<>(); + if (routeKey != null) { + DocRouter router = clusterStateCollection.getRouter(); + Collection slices = router.getSearchSlices(routeKey, null, clusterStateCollection); + for (Slice slice : slices) { + requestedShards.add(slice.getName()); + } + } + if (shard != null) { + String[] paramShards = shard.split(","); + requestedShards.addAll(Arrays.asList(paramShards)); + } + + byte[] bytes = Utils.toJSON(clusterStateCollection); + @SuppressWarnings("unchecked") + Map docCollection = (Map) Utils.fromJSON(bytes); + collectionStatus = getCollectionStatus(docCollection, name, requestedShards); + + collectionStatus.put("znodeVersion", clusterStateCollection.getZNodeVersion()); + collectionStatus.put( + "creationTimeMillis", clusterStateCollection.getCreationTime().toEpochMilli()); + + if (collectionVsAliases.containsKey(name) && !collectionVsAliases.get(name).isEmpty()) { + collectionStatus.put("aliases", collectionVsAliases.get(name)); + } + String configName = clusterStateCollection.getConfigName(); + collectionStatus.put("configName", configName); + if (solrParams.getBool("prs", false) && clusterStateCollection.isPerReplicaState()) { + PerReplicaStates prs = clusterStateCollection.getPerReplicaStates(); + collectionStatus.put("PRS", prs); + } + collectionProps.add(name, collectionStatus); + + // now we need to walk the collectionProps tree to cross-check replica state with live + // nodes + crossCheckReplicaStateWithLiveNodes(liveNodes, collectionProps); + return collectionProps; + } + } } diff --git a/solr/core/src/test/org/apache/solr/cloud/api/collections/TestCollectionAPI.java b/solr/core/src/test/org/apache/solr/cloud/api/collections/TestCollectionAPI.java index 760bf0bfe93..56dfeb2440d 100644 --- a/solr/core/src/test/org/apache/solr/cloud/api/collections/TestCollectionAPI.java +++ b/solr/core/src/test/org/apache/solr/cloud/api/collections/TestCollectionAPI.java @@ -129,10 +129,12 @@ private void testModifyCollection() throws Exception { .getResponse(); NamedList cluster = (NamedList) rsp.get("cluster"); assertNotNull("Cluster state should not be null", cluster); - NamedList collections = (NamedList) cluster.get("collections"); + Map collections = (Map) cluster.get("collections"); assertNotNull("Collections should not be null in cluster state", collections); assertEquals(1, collections.size()); - assertEquals("25", collections._getStr(List.of(COLLECTION_NAME, "replicationFactor"), null)); + Map collectionProperties = (Map) collections.get(COLLECTION_NAME); + collectionProperties.get("replicationFactor"); + assertEquals("25", collectionProperties.get("replicationFactor")); params = new ModifiableSolrParams(); params.set("action", CollectionParams.CollectionAction.MODIFYCOLLECTION.toString()); @@ -151,10 +153,12 @@ private void testModifyCollection() throws Exception { System.out.println(rsp); cluster = (NamedList) rsp.get("cluster"); assertNotNull("Cluster state should not be null", cluster); - collections = (NamedList) cluster.get("collections"); + collections = (Map) cluster.get("collections"); assertNotNull("Collections should not be null in cluster state", collections); assertEquals(1, collections.size()); - assertNull(collections._getStr(List.of(COLLECTION_NAME, "replicationFactor"), null)); + collectionProperties = (Map) collections.get(COLLECTION_NAME); + collectionProperties.get("replicationFactor"); + assertNull(collectionProperties.get("replicationFactor")); params = new ModifiableSolrParams(); params.set("action", CollectionParams.CollectionAction.MODIFYCOLLECTION.toString()); @@ -253,7 +257,7 @@ private void testNoConfigset() throws Exception { NamedList rsp = client.request(req); NamedList cluster = (NamedList) rsp.get("cluster"); assertNotNull("Cluster state should not be null", cluster); - NamedList collections = (NamedList) cluster.get("collections"); + Map collections = (Map) cluster.get("collections"); assertNotNull("Collections should not be null in cluster state", collections); assertNotNull( "Testing to insure collections are returned", collections.get(COLLECTION_NAME1)); @@ -280,7 +284,7 @@ private void assertCountsForRepFactorAndNrtReplicas(CloudSolrClient client, Stri NamedList rsp = client.request(request); NamedList cluster = (NamedList) rsp.get("cluster"); assertNotNull("Cluster state should not be null", cluster); - NamedList collections = (NamedList) cluster.get("collections"); + Map collections = (Map) cluster.get("collections"); assertNotNull("Collections should not be null in cluster state", collections); assertEquals(1, collections.size()); @SuppressWarnings({"unchecked"}) @@ -302,7 +306,7 @@ private void clusterStatusWithCollectionAndShard() throws IOException, SolrServe NamedList rsp = client.request(request); NamedList cluster = (NamedList) rsp.get("cluster"); assertNotNull("Cluster state should not be null", cluster); - NamedList collections = (NamedList) cluster.get("collections"); + Map collections = (Map) cluster.get("collections"); assertNotNull("Collections should not be null in cluster state", collections); assertNotNull(collections.get(COLLECTION_NAME)); assertEquals(1, collections.size()); @@ -328,7 +332,7 @@ private void clusterStatusWithCollectionAndMultipleShards() NamedList rsp = request.process(client).getResponse(); NamedList cluster = (NamedList) rsp.get("cluster"); assertNotNull("Cluster state should not be null", cluster); - NamedList collections = (NamedList) cluster.get("collections"); + Map collections = (Map) cluster.get("collections"); assertNotNull("Collections should not be null in cluster state", collections); assertNotNull(collections.get(COLLECTION_NAME)); assertEquals(1, collections.size()); @@ -463,7 +467,7 @@ private void clusterStatusNoCollection() throws Exception { NamedList rsp = client.request(request); NamedList cluster = (NamedList) rsp.get("cluster"); assertNotNull("Cluster state should not be null", cluster); - NamedList collections = (NamedList) cluster.get("collections"); + Map collections = (Map) cluster.get("collections"); assertNotNull("Collections should not be null in cluster state", collections); assertNotNull(collections.get(COLLECTION_NAME1)); assertEquals(4, collections.size()); @@ -485,7 +489,7 @@ private void clusterStatusWithCollection() throws IOException, SolrServerExcepti NamedList rsp = client.request(request); NamedList cluster = (NamedList) rsp.get("cluster"); assertNotNull("Cluster state should not be null", cluster); - NamedList collections = (NamedList) cluster.get("collections"); + Map collections = (Map) cluster.get("collections"); assertNotNull("Collections should not be null in cluster state", collections); assertEquals(1, collections.size()); @SuppressWarnings({"unchecked"}) @@ -515,7 +519,7 @@ private void clusterStatusZNodeVersion() throws Exception { NamedList rsp = client.request(request); NamedList cluster = (NamedList) rsp.get("cluster"); assertNotNull("Cluster state should not be null", cluster); - NamedList collections = (NamedList) cluster.get("collections"); + Map collections = (Map) cluster.get("collections"); assertNotNull("Collections should not be null in cluster state", collections); assertEquals(1, collections.size()); Map collection = (Map) collections.get(cname); @@ -531,7 +535,7 @@ private void clusterStatusZNodeVersion() throws Exception { rsp = client.request(request); cluster = (NamedList) rsp.get("cluster"); - collections = (NamedList) cluster.get("collections"); + collections = (Map) cluster.get("collections"); collection = (Map) collections.get(cname); Integer newVersion = (Integer) collection.get("znodeVersion"); assertNotNull(newVersion); @@ -558,7 +562,7 @@ private void clusterStatusWithRouteKey() throws IOException, SolrServerException NamedList cluster = (NamedList) rsp.get("cluster"); assertNotNull("Cluster state should not be null", cluster); @SuppressWarnings({"unchecked"}) - NamedList collections = (NamedList) cluster.get("collections"); + Map collections = (Map) cluster.get("collections"); assertNotNull("Collections should not be null in cluster state", collections); assertNotNull(collections.get(DEFAULT_COLLECTION)); assertEquals(1, collections.size()); @@ -605,7 +609,7 @@ private void clusterStatusAliasTest() throws Exception { DEFAULT_COLLECTION + "," + COLLECTION_NAME, aliases.get("myalias")); - NamedList collections = (NamedList) cluster.get("collections"); + Map collections = (Map) cluster.get("collections"); assertNotNull("Collections should not be null in cluster state", collections); assertNotNull(collections.get(DEFAULT_COLLECTION)); Map collection = (Map) collections.get(DEFAULT_COLLECTION); @@ -625,7 +629,7 @@ private void clusterStatusAliasTest() throws Exception { cluster = (NamedList) rsp.get("cluster"); assertNotNull("Cluster state should not be null", cluster); - collections = (NamedList) cluster.get("collections"); + collections = (Map) cluster.get("collections"); assertNotNull("Collections should not be null in cluster state", collections); assertNotNull(collections.get(DEFAULT_COLLECTION)); assertNotNull(collections.get(COLLECTION_NAME)); diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/BaseHttpClusterStateProvider.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/BaseHttpClusterStateProvider.java index 40183109178..f50678a3d2e 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/BaseHttpClusterStateProvider.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/BaseHttpClusterStateProvider.java @@ -41,7 +41,6 @@ import org.apache.solr.common.params.ModifiableSolrParams; import org.apache.solr.common.util.CollectionUtil; import org.apache.solr.common.util.EnvUtils; -import org.apache.solr.common.util.NamedList; import org.apache.solr.common.util.SimpleOrderedMap; import org.apache.solr.common.util.Utils; import org.slf4j.Logger; @@ -140,11 +139,11 @@ private ClusterState fetchClusterState(SolrClient client) liveNodesTimestamp = System.nanoTime(); } - var collectionsNl = (NamedList>) cluster.get("collections"); + var collectionsNl = (Map>) cluster.get("collections"); Map collStateByName = CollectionUtil.newLinkedHashMap(collectionsNl.size()); - for (Entry> entry : collectionsNl) { + for (Entry> entry : collectionsNl.entrySet()) { collStateByName.put( entry.getKey(), getDocCollectionFromObjects(entry.getKey(), entry.getValue())); } @@ -185,7 +184,9 @@ private DocCollection fetchCollectionState(SolrClient client, String collection) SimpleOrderedMap cluster = submitClusterStateRequest(client, collection, ClusterStateRequestType.FETCH_COLLECTION); - var collStateMap = (Map) cluster.findRecursive("collections", collection); + var collState = (Map) cluster.findRecursive("collections"); + var collStateMap = (Map) collState.get(collection); + if (collStateMap == null) { throw new NotACollectionException(); // probably an alias } diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/ClusterStateProviderTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/ClusterStateProviderTest.java index ea8225307da..aa0f3b56a1c 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/ClusterStateProviderTest.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/ClusterStateProviderTest.java @@ -145,7 +145,7 @@ private Instant getCreationTimeFromClusterStatus(String collectionName) NamedList response = clusterStatusResponse.getResponse(); NamedList cluster = (NamedList) response.get("cluster"); - NamedList collections = (NamedList) cluster.get("collections"); + Map collections = (Map) cluster.get("collections"); Map collection = (Map) collections.get(collectionName); return Instant.ofEpochMilli((long) collection.get("creationTimeMillis")); } From 8b8d37d4ca37c3a42fe7d7227ded65f4e2d31323 Mon Sep 17 00:00:00 2001 From: Matthew Biscocho Date: Thu, 19 Dec 2024 12:26:40 -0500 Subject: [PATCH 5/7] Address PR comments --- .../solr/handler/admin/ClusterStatus.java | 133 ++++++++---------- .../impl/BaseHttpClusterStateProvider.java | 13 +- .../apache/solr/common/util/NamedList.java | 10 +- .../solr/common/util/NamedListTest.java | 33 +++++ 4 files changed, 104 insertions(+), 85 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/handler/admin/ClusterStatus.java b/solr/core/src/java/org/apache/solr/handler/admin/ClusterStatus.java index db7910bb210..1e629140e94 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/ClusterStatus.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/ClusterStatus.java @@ -183,6 +183,12 @@ private void fetchClusterStatusForCollOrAlias( String routeKey = solrParams.get(ShardParams._ROUTE_); String shard = solrParams.get(ZkStateReader.SHARD_ID_PROP); + Set requestedShards = new HashSet<>(); + if (shard != null) { + String[] paramShards = shard.split(","); + requestedShards.addAll(Arrays.asList(paramShards)); + } + Stream collectionStream; if (collection == null) { collectionStream = clusterState.collectionStream(); @@ -210,12 +216,20 @@ private void fetchClusterStatusForCollOrAlias( MapWriter collectionPropsWriter = ew -> { - SolrCollectionProperiesIterator collectionProps = - new SolrCollectionProperiesIterator( - collectionStream.iterator(), collectionVsAliases, routeKey, liveNodes, shard); - while (collectionProps.hasNext()) { - Map collectionPropsMap = (collectionProps.next().asMap()); - collectionPropsMap.forEach( + Iterator> it = + collectionStream + .map( + (collectionState) -> + collectionPropsResponse( + collectionState, + collectionVsAliases, + routeKey, + liveNodes, + requestedShards)) + .iterator(); + while (it.hasNext()) { + Map props = it.next(); + props.forEach( (key, value) -> { try { ew.put(key, value); @@ -280,8 +294,8 @@ private Map getCollectionStatus( */ @SuppressWarnings("unchecked") protected void crossCheckReplicaStateWithLiveNodes( - List liveNodes, NamedList collectionProps) { - for (Map.Entry next : collectionProps) { + List liveNodes, Map collectionProps) { + for (Map.Entry next : collectionProps.entrySet()) { Map collMap = (Map) next.getValue(); Map shards = (Map) collMap.get("shards"); for (Object nextShard : shards.values()) { @@ -342,76 +356,47 @@ public static Map postProcessCollectionJSON(Map return collection; } - private class SolrCollectionProperiesIterator implements Iterator> { - - final Iterator it; - Map> collectionVsAliases; - String routeKey; - List liveNodes; - String shard; - - public SolrCollectionProperiesIterator( - Iterator it, - Map> collectionVsAliases, - String routeKey, - List liveNodes, - String shard) { - this.it = it; - this.collectionVsAliases = collectionVsAliases; - this.routeKey = routeKey; - this.liveNodes = liveNodes; - this.shard = shard; - } - - @Override - public boolean hasNext() { - return it.hasNext(); - } - - @Override - public NamedList next() { - NamedList collectionProps = new SimpleOrderedMap<>(); - DocCollection clusterStateCollection = it.next(); - Map collectionStatus; - String name = clusterStateCollection.getName(); - - Set requestedShards = new HashSet<>(); - if (routeKey != null) { - DocRouter router = clusterStateCollection.getRouter(); - Collection slices = router.getSearchSlices(routeKey, null, clusterStateCollection); - for (Slice slice : slices) { - requestedShards.add(slice.getName()); - } - } - if (shard != null) { - String[] paramShards = shard.split(","); - requestedShards.addAll(Arrays.asList(paramShards)); + private Map collectionPropsResponse( + DocCollection clusterStateCollection, + Map> collectionVsAliases, + String routeKey, + List liveNodes, + Set requestedShards) { + Map collectionProps = new HashMap<>(); + Map collectionStatus; + String name = clusterStateCollection.getName(); + + if (routeKey != null) { + DocRouter router = clusterStateCollection.getRouter(); + Collection slices = router.getSearchSlices(routeKey, null, clusterStateCollection); + for (Slice slice : slices) { + requestedShards.add(slice.getName()); } + } - byte[] bytes = Utils.toJSON(clusterStateCollection); - @SuppressWarnings("unchecked") - Map docCollection = (Map) Utils.fromJSON(bytes); - collectionStatus = getCollectionStatus(docCollection, name, requestedShards); + byte[] bytes = Utils.toJSON(clusterStateCollection); + @SuppressWarnings("unchecked") + Map docCollection = (Map) Utils.fromJSON(bytes); + collectionStatus = getCollectionStatus(docCollection, name, requestedShards); - collectionStatus.put("znodeVersion", clusterStateCollection.getZNodeVersion()); - collectionStatus.put( - "creationTimeMillis", clusterStateCollection.getCreationTime().toEpochMilli()); + collectionStatus.put("znodeVersion", clusterStateCollection.getZNodeVersion()); + collectionStatus.put( + "creationTimeMillis", clusterStateCollection.getCreationTime().toEpochMilli()); - if (collectionVsAliases.containsKey(name) && !collectionVsAliases.get(name).isEmpty()) { - collectionStatus.put("aliases", collectionVsAliases.get(name)); - } - String configName = clusterStateCollection.getConfigName(); - collectionStatus.put("configName", configName); - if (solrParams.getBool("prs", false) && clusterStateCollection.isPerReplicaState()) { - PerReplicaStates prs = clusterStateCollection.getPerReplicaStates(); - collectionStatus.put("PRS", prs); - } - collectionProps.add(name, collectionStatus); - - // now we need to walk the collectionProps tree to cross-check replica state with live - // nodes - crossCheckReplicaStateWithLiveNodes(liveNodes, collectionProps); - return collectionProps; + if (collectionVsAliases.containsKey(name) && !collectionVsAliases.get(name).isEmpty()) { + collectionStatus.put("aliases", collectionVsAliases.get(name)); + } + String configName = clusterStateCollection.getConfigName(); + collectionStatus.put("configName", configName); + if (solrParams.getBool("prs", false) && clusterStateCollection.isPerReplicaState()) { + PerReplicaStates prs = clusterStateCollection.getPerReplicaStates(); + collectionStatus.put("PRS", prs); } + collectionProps.put(name, collectionStatus); + + // now we need to walk the collectionProps tree to cross-check replica state with live + // nodes + crossCheckReplicaStateWithLiveNodes(liveNodes, collectionProps); + return collectionProps; } } diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/BaseHttpClusterStateProvider.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/BaseHttpClusterStateProvider.java index f50678a3d2e..d4b9dbd99e2 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/BaseHttpClusterStateProvider.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/BaseHttpClusterStateProvider.java @@ -139,11 +139,11 @@ private ClusterState fetchClusterState(SolrClient client) liveNodesTimestamp = System.nanoTime(); } - var collectionsNl = (Map>) cluster.get("collections"); + var collectionsMap = (Map>) cluster.get("collections"); Map collStateByName = - CollectionUtil.newLinkedHashMap(collectionsNl.size()); - for (Entry> entry : collectionsNl.entrySet()) { + CollectionUtil.newLinkedHashMap(collectionsMap.size()); + for (Entry> entry : collectionsMap.entrySet()) { collStateByName.put( entry.getKey(), getDocCollectionFromObjects(entry.getKey(), entry.getValue())); } @@ -184,13 +184,12 @@ private DocCollection fetchCollectionState(SolrClient client, String collection) SimpleOrderedMap cluster = submitClusterStateRequest(client, collection, ClusterStateRequestType.FETCH_COLLECTION); - var collState = (Map) cluster.findRecursive("collections"); - var collStateMap = (Map) collState.get(collection); + var collState = (Map) cluster.findRecursive("collections", collection); - if (collStateMap == null) { + if (collState == null) { throw new NotACollectionException(); // probably an alias } - return getDocCollectionFromObjects(collection, collStateMap); + return getDocCollectionFromObjects(collection, collState); } private SimpleOrderedMap submitClusterStateRequest( diff --git a/solr/solrj/src/java/org/apache/solr/common/util/NamedList.java b/solr/solrj/src/java/org/apache/solr/common/util/NamedList.java index 123a5e81d91..579d0a7d157 100644 --- a/solr/solrj/src/java/org/apache/solr/common/util/NamedList.java +++ b/solr/solrj/src/java/org/apache/solr/common/util/NamedList.java @@ -319,7 +319,7 @@ private void killAll(String name) { /** * Recursively parses the NamedList structure to arrive at a specific element. As you descend the * NamedList tree, the last element can be any type, including NamedList, but the previous - * elements MUST be NamedList objects themselves. A null value is returned if the indicated + * elements MUST be NamedList or Map objects themselves. A null value is returned if the indicated * hierarchy doesn't exist, but NamedList allows null values so that could be the actual value at * the end of the path. * @@ -348,9 +348,9 @@ public Object findRecursive(String... args) { * it to this list. Then we retrieve the first key from this list and * assign it to value. * - * On the next loop, we check whether the retrieved value is a NamedList. - * If it is, then we drop to that NamedList, grab the value of the - * next key, and start the loop over. If it is not a NamedList, then we + * On the next loop, we check whether the retrieved value is a NamedList or Map. + * If it is, then we drop to that NamedList/Map, grab the value of the + * next key, and start the loop over. If it is not a NamedList/Map, then we * assign the value to null and break out of the loop. * * Assigning the value to null and then breaking out of the loop seems @@ -363,6 +363,8 @@ public Object findRecursive(String... args) { } else { if (value instanceof NamedList) { currentList = (NamedList) value; + } else if (value instanceof Map) { + currentList = new NamedList<>((Map) value); } else { value = null; break; diff --git a/solr/solrj/src/test/org/apache/solr/common/util/NamedListTest.java b/solr/solrj/src/test/org/apache/solr/common/util/NamedListTest.java index 31924a4afbe..df8ac97f76c 100644 --- a/solr/solrj/src/test/org/apache/solr/common/util/NamedListTest.java +++ b/solr/solrj/src/test/org/apache/solr/common/util/NamedListTest.java @@ -17,6 +17,7 @@ package org.apache.solr.common.util; import java.util.ArrayList; +import java.util.HashMap; import java.util.List; import java.util.Map; import org.apache.solr.SolrTestCase; @@ -209,4 +210,36 @@ public void testShallowMap() { assertEquals("Val2", nl.get("key2")); assertEquals("Val2", m.get("key2")); } + + @Test + @SuppressWarnings("unchecked") + public void testFindRecursiveWithMap() { + + // key1 (NamedList) + // - key2 (Map) + // -- key3 (NamedList) + // --- key4 (Map) + + NamedList nl1 = new NamedList<>(); + Map map2 = new HashMap<>(); + NamedList nl3 = new NamedList<>(); + Map map4 = new HashMap<>(); + + map4.put("key4", "value4"); + nl3.add("key3", map4); + map2.put("key2", nl3); + nl1.add("key1", map2); + + Map test1 = (Map) nl1.findRecursive("key1"); + assertNotNull(test1); + + NamedList test2 = (NamedList) nl1.findRecursive("key1", "key2"); + assertNotNull(test2); + + Map test3 = (Map) nl1.findRecursive("key1", "key2", "key3"); + assertNotNull(test3); + + String test4 = (String) nl1.findRecursive("key1", "key2", "key3", "key4"); + assertEquals("value4", test4); + } } From 457da45e585fe313b09a6c73f9cdd224af9c9acb Mon Sep 17 00:00:00 2001 From: Matthew Biscocho Date: Fri, 20 Dec 2024 09:54:04 -0500 Subject: [PATCH 6/7] Next round of PR review --- .../solr/handler/admin/ClusterStatus.java | 91 ++++++++----------- .../impl/BaseHttpClusterStateProvider.java | 7 +- .../apache/solr/common/util/NamedList.java | 10 +- .../solr/common/util/NamedListTest.java | 33 ------- 4 files changed, 48 insertions(+), 93 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/handler/admin/ClusterStatus.java b/solr/core/src/java/org/apache/solr/handler/admin/ClusterStatus.java index 1e629140e94..53cdc27e596 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/ClusterStatus.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/ClusterStatus.java @@ -18,12 +18,10 @@ import java.io.IOException; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; -import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Objects; @@ -183,10 +181,12 @@ private void fetchClusterStatusForCollOrAlias( String routeKey = solrParams.get(ShardParams._ROUTE_); String shard = solrParams.get(ZkStateReader.SHARD_ID_PROP); - Set requestedShards = new HashSet<>(); + Set requestedShards; if (shard != null) { String[] paramShards = shard.split(","); - requestedShards.addAll(Arrays.asList(paramShards)); + requestedShards = Set.of(paramShards); + } else { + requestedShards = Set.of(); } Stream collectionStream; @@ -216,28 +216,21 @@ private void fetchClusterStatusForCollOrAlias( MapWriter collectionPropsWriter = ew -> { - Iterator> it = - collectionStream - .map( - (collectionState) -> - collectionPropsResponse( - collectionState, - collectionVsAliases, - routeKey, - liveNodes, - requestedShards)) - .iterator(); - while (it.hasNext()) { - Map props = it.next(); - props.forEach( - (key, value) -> { - try { - ew.put(key, value); - } catch (IOException e) { - throw new RuntimeException(e); - } - }); - } + collectionStream.forEach( + (collectionState) -> { + try { + ew.put( + collectionState.getName(), + buildResponseForCollection( + collectionState, + collectionVsAliases, + routeKey, + liveNodes, + requestedShards)); + } catch (IOException e) { + throw new RuntimeException(e); + } + }); }; clusterStatus.add("collections", collectionPropsWriter); } @@ -295,22 +288,19 @@ private Map getCollectionStatus( @SuppressWarnings("unchecked") protected void crossCheckReplicaStateWithLiveNodes( List liveNodes, Map collectionProps) { - for (Map.Entry next : collectionProps.entrySet()) { - Map collMap = (Map) next.getValue(); - Map shards = (Map) collMap.get("shards"); - for (Object nextShard : shards.values()) { - Map shardMap = (Map) nextShard; - Map replicas = (Map) shardMap.get("replicas"); - for (Object nextReplica : replicas.values()) { - Map replicaMap = (Map) nextReplica; - if (Replica.State.getState((String) replicaMap.get(ZkStateReader.STATE_PROP)) - != Replica.State.DOWN) { - // not down, so verify the node is live - String node_name = (String) replicaMap.get(ZkStateReader.NODE_NAME_PROP); - if (!liveNodes.contains(node_name)) { - // node is not live, so this replica is actually down - replicaMap.put(ZkStateReader.STATE_PROP, Replica.State.DOWN.toString()); - } + Map shards = (Map) collectionProps.get("shards"); + for (Object nextShard : shards.values()) { + Map shardMap = (Map) nextShard; + Map replicas = (Map) shardMap.get("replicas"); + for (Object nextReplica : replicas.values()) { + Map replicaMap = (Map) nextReplica; + if (Replica.State.getState((String) replicaMap.get(ZkStateReader.STATE_PROP)) + != Replica.State.DOWN) { + // not down, so verify the node is live + String node_name = (String) replicaMap.get(ZkStateReader.NODE_NAME_PROP); + if (!liveNodes.contains(node_name)) { + // node is not live, so this replica is actually down + replicaMap.put(ZkStateReader.STATE_PROP, Replica.State.DOWN.toString()); } } } @@ -356,28 +346,28 @@ public static Map postProcessCollectionJSON(Map return collection; } - private Map collectionPropsResponse( + private Map buildResponseForCollection( DocCollection clusterStateCollection, Map> collectionVsAliases, String routeKey, List liveNodes, Set requestedShards) { - Map collectionProps = new HashMap<>(); Map collectionStatus; + Set shards = new HashSet<>(requestedShards); String name = clusterStateCollection.getName(); if (routeKey != null) { DocRouter router = clusterStateCollection.getRouter(); Collection slices = router.getSearchSlices(routeKey, null, clusterStateCollection); for (Slice slice : slices) { - requestedShards.add(slice.getName()); + shards.add(slice.getName()); } } byte[] bytes = Utils.toJSON(clusterStateCollection); @SuppressWarnings("unchecked") Map docCollection = (Map) Utils.fromJSON(bytes); - collectionStatus = getCollectionStatus(docCollection, name, requestedShards); + collectionStatus = getCollectionStatus(docCollection, name, shards); collectionStatus.put("znodeVersion", clusterStateCollection.getZNodeVersion()); collectionStatus.put( @@ -392,11 +382,10 @@ private Map collectionPropsResponse( PerReplicaStates prs = clusterStateCollection.getPerReplicaStates(); collectionStatus.put("PRS", prs); } - collectionProps.put(name, collectionStatus); - // now we need to walk the collectionProps tree to cross-check replica state with live - // nodes - crossCheckReplicaStateWithLiveNodes(liveNodes, collectionProps); - return collectionProps; + // now we need to walk the collectionProps tree to cross-check replica state with live nodes + crossCheckReplicaStateWithLiveNodes(liveNodes, collectionStatus); + + return collectionStatus; } } diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/BaseHttpClusterStateProvider.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/BaseHttpClusterStateProvider.java index d4b9dbd99e2..d9b3a94d8ec 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/BaseHttpClusterStateProvider.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/BaseHttpClusterStateProvider.java @@ -184,12 +184,13 @@ private DocCollection fetchCollectionState(SolrClient client, String collection) SimpleOrderedMap cluster = submitClusterStateRequest(client, collection, ClusterStateRequestType.FETCH_COLLECTION); - var collState = (Map) cluster.findRecursive("collections", collection); + var collState = (Map) cluster.findRecursive("collections"); + var collStateMap = (Map) collState.get(collection); - if (collState == null) { + if (collStateMap == null) { throw new NotACollectionException(); // probably an alias } - return getDocCollectionFromObjects(collection, collState); + return getDocCollectionFromObjects(collection, collStateMap); } private SimpleOrderedMap submitClusterStateRequest( diff --git a/solr/solrj/src/java/org/apache/solr/common/util/NamedList.java b/solr/solrj/src/java/org/apache/solr/common/util/NamedList.java index 579d0a7d157..123a5e81d91 100644 --- a/solr/solrj/src/java/org/apache/solr/common/util/NamedList.java +++ b/solr/solrj/src/java/org/apache/solr/common/util/NamedList.java @@ -319,7 +319,7 @@ private void killAll(String name) { /** * Recursively parses the NamedList structure to arrive at a specific element. As you descend the * NamedList tree, the last element can be any type, including NamedList, but the previous - * elements MUST be NamedList or Map objects themselves. A null value is returned if the indicated + * elements MUST be NamedList objects themselves. A null value is returned if the indicated * hierarchy doesn't exist, but NamedList allows null values so that could be the actual value at * the end of the path. * @@ -348,9 +348,9 @@ public Object findRecursive(String... args) { * it to this list. Then we retrieve the first key from this list and * assign it to value. * - * On the next loop, we check whether the retrieved value is a NamedList or Map. - * If it is, then we drop to that NamedList/Map, grab the value of the - * next key, and start the loop over. If it is not a NamedList/Map, then we + * On the next loop, we check whether the retrieved value is a NamedList. + * If it is, then we drop to that NamedList, grab the value of the + * next key, and start the loop over. If it is not a NamedList, then we * assign the value to null and break out of the loop. * * Assigning the value to null and then breaking out of the loop seems @@ -363,8 +363,6 @@ public Object findRecursive(String... args) { } else { if (value instanceof NamedList) { currentList = (NamedList) value; - } else if (value instanceof Map) { - currentList = new NamedList<>((Map) value); } else { value = null; break; diff --git a/solr/solrj/src/test/org/apache/solr/common/util/NamedListTest.java b/solr/solrj/src/test/org/apache/solr/common/util/NamedListTest.java index df8ac97f76c..31924a4afbe 100644 --- a/solr/solrj/src/test/org/apache/solr/common/util/NamedListTest.java +++ b/solr/solrj/src/test/org/apache/solr/common/util/NamedListTest.java @@ -17,7 +17,6 @@ package org.apache.solr.common.util; import java.util.ArrayList; -import java.util.HashMap; import java.util.List; import java.util.Map; import org.apache.solr.SolrTestCase; @@ -210,36 +209,4 @@ public void testShallowMap() { assertEquals("Val2", nl.get("key2")); assertEquals("Val2", m.get("key2")); } - - @Test - @SuppressWarnings("unchecked") - public void testFindRecursiveWithMap() { - - // key1 (NamedList) - // - key2 (Map) - // -- key3 (NamedList) - // --- key4 (Map) - - NamedList nl1 = new NamedList<>(); - Map map2 = new HashMap<>(); - NamedList nl3 = new NamedList<>(); - Map map4 = new HashMap<>(); - - map4.put("key4", "value4"); - nl3.add("key3", map4); - map2.put("key2", nl3); - nl1.add("key1", map2); - - Map test1 = (Map) nl1.findRecursive("key1"); - assertNotNull(test1); - - NamedList test2 = (NamedList) nl1.findRecursive("key1", "key2"); - assertNotNull(test2); - - Map test3 = (Map) nl1.findRecursive("key1", "key2", "key3"); - assertNotNull(test3); - - String test4 = (String) nl1.findRecursive("key1", "key2", "key3", "key4"); - assertEquals("value4", test4); - } } From f6e77cd9d1133c4eff7019b41c892389dc080567 Mon Sep 17 00:00:00 2001 From: Matthew Biscocho Date: Fri, 20 Dec 2024 15:46:40 -0500 Subject: [PATCH 7/7] Cleanup some code in ClusterStatus --- .../solr/handler/admin/ClusterStatus.java | 55 +++++++------------ 1 file changed, 21 insertions(+), 34 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/handler/admin/ClusterStatus.java b/solr/core/src/java/org/apache/solr/handler/admin/ClusterStatus.java index 53cdc27e596..294a4c77715 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/ClusterStatus.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/ClusterStatus.java @@ -16,7 +16,6 @@ */ package org.apache.solr.handler.admin; -import java.io.IOException; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; @@ -32,10 +31,8 @@ import org.apache.solr.common.cloud.Aliases; import org.apache.solr.common.cloud.ClusterState; import org.apache.solr.common.cloud.DocCollection; -import org.apache.solr.common.cloud.DocRouter; import org.apache.solr.common.cloud.PerReplicaStates; import org.apache.solr.common.cloud.Replica; -import org.apache.solr.common.cloud.Slice; import org.apache.solr.common.cloud.ZkStateReader; import org.apache.solr.common.params.ShardParams; import org.apache.solr.common.params.SolrParams; @@ -181,13 +178,7 @@ private void fetchClusterStatusForCollOrAlias( String routeKey = solrParams.get(ShardParams._ROUTE_); String shard = solrParams.get(ZkStateReader.SHARD_ID_PROP); - Set requestedShards; - if (shard != null) { - String[] paramShards = shard.split(","); - requestedShards = Set.of(paramShards); - } else { - requestedShards = Set.of(); - } + Set requestedShards = (shard != null) ? Set.of(shard.split(",")) : null; Stream collectionStream; if (collection == null) { @@ -218,18 +209,14 @@ private void fetchClusterStatusForCollOrAlias( ew -> { collectionStream.forEach( (collectionState) -> { - try { - ew.put( - collectionState.getName(), - buildResponseForCollection( - collectionState, - collectionVsAliases, - routeKey, - liveNodes, - requestedShards)); - } catch (IOException e) { - throw new RuntimeException(e); - } + ew.putNoEx( + collectionState.getName(), + buildResponseForCollection( + collectionState, + collectionVsAliases, + routeKey, + liveNodes, + requestedShards)); }); }; clusterStatus.add("collections", collectionPropsWriter); @@ -288,12 +275,12 @@ private Map getCollectionStatus( @SuppressWarnings("unchecked") protected void crossCheckReplicaStateWithLiveNodes( List liveNodes, Map collectionProps) { - Map shards = (Map) collectionProps.get("shards"); + var shards = (Map) collectionProps.get("shards"); for (Object nextShard : shards.values()) { - Map shardMap = (Map) nextShard; - Map replicas = (Map) shardMap.get("replicas"); + var shardMap = (Map) nextShard; + var replicas = (Map) shardMap.get("replicas"); for (Object nextReplica : replicas.values()) { - Map replicaMap = (Map) nextReplica; + var replicaMap = (Map) nextReplica; if (Replica.State.getState((String) replicaMap.get(ZkStateReader.STATE_PROP)) != Replica.State.DOWN) { // not down, so verify the node is live @@ -353,16 +340,16 @@ private Map buildResponseForCollection( List liveNodes, Set requestedShards) { Map collectionStatus; - Set shards = new HashSet<>(requestedShards); + Set shards = new HashSet<>(); String name = clusterStateCollection.getName(); - if (routeKey != null) { - DocRouter router = clusterStateCollection.getRouter(); - Collection slices = router.getSearchSlices(routeKey, null, clusterStateCollection); - for (Slice slice : slices) { - shards.add(slice.getName()); - } - } + if (routeKey != null) + clusterStateCollection + .getRouter() + .getSearchSlices(routeKey, null, clusterStateCollection) + .forEach((slice) -> shards.add(slice.getName())); + + if (requestedShards != null) shards.addAll(requestedShards); byte[] bytes = Utils.toJSON(clusterStateCollection); @SuppressWarnings("unchecked")