dsmiley commented on code in PR #2599:
URL: https://github.com/apache/solr/pull/2599#discussion_r1706026056
##########
solr/core/src/java/org/apache/solr/handler/admin/ClusterStatus.java:
##########
@@ -89,14 +93,61 @@ public static Health combine(Collection<Health> states) {
}
}
- public ClusterStatus(ZkStateReader zkStateReader, ZkNodeProps props) {
+ public ClusterStatus(ZkStateReader zkStateReader, SolrParams params) {
this.zkStateReader = zkStateReader;
- this.message = props;
- collection = props.getStr(ZkStateReader.COLLECTION_PROP);
+ this.solrParams = params;
+ collection = params.get(ZkStateReader.COLLECTION_PROP);
}
public void getClusterStatus(NamedList<Object> results)
throws KeeperException, InterruptedException {
+ NamedList<Object> clusterStatus = new SimpleOrderedMap<>();
+
+ boolean includeAll = solrParams.getBool(INCLUDE_ALL, true);
+ boolean withLiveNodes = solrParams.getBool(LIVENODES_PROP, includeAll);
+ boolean withClusterProperties = solrParams.getBool(CLUSTER_PROP,
includeAll);
+ boolean withRoles = solrParams.getBool(ZkStateReader.ROLES_PROP,
includeAll);
+ boolean withCollection = includeAll || (collection != null);
+
+ List<String> liveNodes = null;
+ if (withLiveNodes || collection != null) {
+ liveNodes =
+
zkStateReader.getZkClient().getChildren(ZkStateReader.LIVE_NODES_ZKNODE, null,
true);
+ // add live_nodes
+ if (withLiveNodes) clusterStatus.add("live_nodes", liveNodes);
+ }
+ if (withCollection) {
+ assert liveNodes != null;
+ fetchClusterStatusForCollOrAlias(clusterStatus, liveNodes);
+ }
+
+ if (withClusterProperties) {
+ Map<String, Object> clusterProps = Collections.emptyMap();
+ // read cluster properties
+ clusterProps = zkStateReader.getClusterProperties();
+ if (clusterProps == null || clusterProps.isEmpty()) {
Review Comment:
isEmpty check isn't accomplishing anything here
##########
solr/core/src/java/org/apache/solr/handler/admin/ClusterStatus.java:
##########
@@ -89,14 +93,61 @@ public static Health combine(Collection<Health> states) {
}
}
- public ClusterStatus(ZkStateReader zkStateReader, ZkNodeProps props) {
+ public ClusterStatus(ZkStateReader zkStateReader, SolrParams params) {
this.zkStateReader = zkStateReader;
- this.message = props;
- collection = props.getStr(ZkStateReader.COLLECTION_PROP);
+ this.solrParams = params;
+ collection = params.get(ZkStateReader.COLLECTION_PROP);
Review Comment:
isn't it a little awkward that this param is initialized here despite lots
of params being taken in getClusterStatus? (change or not as you wish)
##########
solr/core/src/java/org/apache/solr/handler/admin/ClusterStatus.java:
##########
@@ -191,43 +234,23 @@ public void getClusterStatus(NamedList<Object> results)
}
String configName = clusterStateCollection.getConfigName();
collectionStatus.put("configName", configName);
- if (message.getBool("prs", false) &&
clusterStateCollection.isPerReplicaState()) {
+ if (solrParams.getBool("prs", false) &&
clusterStateCollection.isPerReplicaState()) {
PerReplicaStates prs = clusterStateCollection.getPerReplicaStates();
collectionStatus.put("PRS", prs);
}
collectionProps.add(name, collectionStatus);
}
- List<String> liveNodes =
-
zkStateReader.getZkClient().getChildren(ZkStateReader.LIVE_NODES_ZKNODE, null,
true);
-
- // now we need to walk the collectionProps tree to cross-check replica
state with live nodes
- crossCheckReplicaStateWithLiveNodes(liveNodes, collectionProps);
-
- NamedList<Object> clusterStatus = new SimpleOrderedMap<>();
- clusterStatus.add("collections", collectionProps);
-
- // read cluster properties
- Map<String, Object> clusterProps = zkStateReader.getClusterProperties();
- if (clusterProps != null && !clusterProps.isEmpty()) {
- clusterStatus.add("properties", clusterProps);
- }
-
// add the alias map too
Map<String, String> collectionAliasMap = aliases.getCollectionAliasMap();
// comma delim
if (!collectionAliasMap.isEmpty()) {
clusterStatus.add("aliases", collectionAliasMap);
Review Comment:
oh wait; should there be withAliases ?
##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/BaseHttpClusterStateProvider.java:
##########
@@ -84,6 +84,11 @@ public void init(List<String> solrUrls) throws Exception {
/** Create a SolrClient implementation that uses the specified Solr node URL
*/
protected abstract SolrClient getSolrClient(String baseUrl);
+ @Override
Review Comment:
this would be a good code comment
##########
solr/solr-ref-guide/modules/deployment-guide/pages/cluster-node-management.adoc:
##########
@@ -99,6 +99,41 @@ Multiple shard names can be specified as a comma-separated
list.
+
This can be used if you need the details of the shard where a particular
document belongs to and you don't know which shard it falls under.
+`liveNodes`::
++
+[%autowidth,frame=none]
+|===
+|Optional |Default: will default to the default value of `includeAll`
parameter specified below
+|===
++
+If set to true, returns the status of live nodes in the cluster.
+
+`clusterProperties`::
++
+[%autowidth,frame=none]
+|===
+|Optional |Default: will default to the default value of `includeAll`
parameter specified below
+|===
++
+If set to true, returns the properties of the cluster.
+
+`roles`::
++
+[%autowidth,frame=none]
+|===
+|Optional |Default: will default to the default value of `includeAll`
parameter specified below
+|===
++
+If set to true, returns the roles within the cluster.
+
+`includeAll`::
++
+[%autowidth,frame=none]
+|===
+|Optional |Default: true
+|===
Review Comment:
just a simple sentence to list the sections
##########
solr/solrj/src/java/org/apache/solr/client/solrj/impl/BaseHttpClusterStateProvider.java:
##########
@@ -335,21 +362,18 @@ public ClusterState getClusterState() {
+ " solrUrl(s) or zkHost(s).");
}
+ @SuppressWarnings("unchecked")
@Override
public Map<String, Object> getClusterProperties() {
+ // Map<String, Object> clusterPropertiesMap = new HashMap<>();
for (String nodeName : liveNodes) {
String baseUrl = Utils.getBaseUrlForNodeName(nodeName, urlScheme);
try (SolrClient client = getSolrClient(baseUrl)) {
- Map<String, Object> clusterProperties = new HashMap<>();
- fetchClusterState(client, null, clusterProperties);
- return clusterProperties;
+ SimpleOrderedMap<?> cluster =
+ submitClusterStateRequest(client, null,
ClusterStateRequestType.FETCH_CLUSTER_PROP);
+ return (Map<String, Object>) cluster.get("properties");
} catch (SolrServerException | BaseHttpSolrClient.RemoteSolrException |
IOException e) {
log.warn("Attempt to fetch cluster state from {} failed.", baseUrl, e);
- } catch (NotACollectionException e) {
Review Comment:
nice to fix this :-)
##########
solr/core/src/java/org/apache/solr/handler/admin/ClusterStatus.java:
##########
@@ -139,6 +184,7 @@ public void getClusterStatus(NamedList<Object> results)
if (didNotFindCollection && isAlias) {
// In this case this.collection is an alias name not a collection
// get all collections and filter out collections not in the alias
+ // clusterState.getCollectionsMap() should be replaced with an
inexpensive call
Review Comment:
(waiting)
--
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
