epugh commented on code in PR #4952:
URL: https://github.com/apache/solr/pull/4952#discussion_r4113387706
##########
solr/core/src/java/org/apache/solr/handler/admin/ClusterStatus.java:
##########
@@ -143,13 +146,49 @@ public void getClusterStatus(NamedList<Object> results,
SolrVersion solrVersion)
results.add("cluster", clusterStatus);
}
- private void fetchClusterStatusForCollOrAlias(
+ /**
+ * Collections, shards, and replicas selected by this request's {@code
collection}, {@code shard},
+ * and {@code _route_} parameters.
+ *
+ * <p>Live nodes are read so replica state can be cross-checked, and aliases
are read so a
+ * collection alias in {@code collection} can be resolved. Each collection
still lists the aliases
+ * that point at it. The live-node list, the alias map, and cluster
properties are not returned.
+ */
+ public Map<String, Object> getCollectionStatuses() throws KeeperException,
InterruptedException {
+ List<String> liveNodes =
+
zkStateReader.getZkClient().getChildren(ZkStateReader.LIVE_NODES_ZKNODE, null);
+ Aliases aliases = zkStateReader.getAliases();
+ Map<String, Object> collections = new LinkedHashMap<>();
+ forEachCollectionStatus(
+ liveNodes, aliases, (name, status) -> collections.put(name,
copyForJackson(status)));
+ return collections;
+ }
+
+ private void addCollectionStatuses(
NamedList<Object> clusterStatus,
List<String> liveNodes,
Aliases aliases,
SolrVersion solrVersion) {
+ // Resolve the collection list now so a missing name fails the request,
rather than during
+ // response writing. The per-collection JSON is still built while the
response is written.
+ PreparedCollections prepared = prepareCollections(aliases);
+ if (solrVersion == null ||
solrVersion.greaterThanOrEqualTo(SolrVersion.valueOf("9.9.0"))) {
Review Comment:
huh.... is this part of a rolling upgrade feature?
##########
solr/core/src/java/org/apache/solr/handler/ClusterAPI.java:
##########
@@ -210,13 +208,6 @@ public static SolrQueryRequest wrapParams(SolrQueryRequest
req, Map<String, Obje
return req;
}
- @EndPoint(method = GET, path = "/cluster", permission = COLL_READ_PERM)
Review Comment:
is there also a registerV2 Api method on this guy which can be removed?
##########
solr/core/src/java/org/apache/solr/handler/admin/api/GetClusterStatus.java:
##########
@@ -0,0 +1,91 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.handler.admin.api;
+
+import static
org.apache.solr.security.PermissionNameProvider.Name.COLL_READ_PERM;
+
+import jakarta.inject.Inject;
+import java.util.Map;
+import org.apache.solr.client.api.endpoint.GetClusterStatusApi;
+import org.apache.solr.client.api.model.GetClusterStatusResponse;
+import
org.apache.solr.client.api.model.GetClusterStatusResponse.CollectionState;
+import org.apache.solr.common.params.ModifiableSolrParams;
+import org.apache.solr.common.params.ShardParams;
+import org.apache.solr.common.util.CollectionUtil;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.handler.admin.ClusterStatus;
+import org.apache.solr.jersey.PermissionName;
+import org.apache.solr.jersey.SolrJacksonMapper;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+
+/**
+ * V2 API for the collections, shards, and replicas tree.
+ *
+ * <p>{@code GET /api/cluster} returns that tree and nothing else hanging off
the cluster object.
+ * Live nodes, aliases, and cluster properties stay on their own endpoints. v1
{@code CLUSTERSTATUS}
+ * is unchanged.
+ */
+public class GetClusterStatus extends AdminAPIBase implements
GetClusterStatusApi {
+
+ @Inject
+ public GetClusterStatus(
+ CoreContainer coreContainer, SolrQueryRequest req, SolrQueryResponse
rsp) {
+ super(coreContainer, req, rsp);
+ }
+
+ @Override
+ @PermissionName(COLL_READ_PERM)
+ public GetClusterStatusResponse getClusterStatus(
+ String collection, String shard, String routeKey, Boolean prs) throws
Exception {
+ final GetClusterStatusResponse response =
+ instantiateJerseyResponse(GetClusterStatusResponse.class);
+ validateZooKeeperAwareCoreContainer(coreContainer);
+ if (collection != null) {
+ recordCollectionForLogAndTracing(collection, solrQueryRequest);
+ }
+
+ // Only the parameters this API documents. includeAll, liveNodes, aliases,
and
Review Comment:
yeah, not sure we should still be documenting the includeAll and liveNodes
etc at this level. One comment, okay, but seems like too many
##########
solr/solr-ref-guide/modules/deployment-guide/pages/cluster-node-management.adoc:
##########
@@ -148,7 +165,7 @@ The response will include the status of the request and the
status of the cluste
http://localhost:8983/solr/admin/collections?action=CLUSTERSTATUS
----
-*Output*
+*V1 output*
Review Comment:
do we need a seperate v2 api output?
##########
solr/solr-ref-guide/modules/upgrade-notes/pages/major-changes-in-solr-11.adoc:
##########
@@ -41,3 +41,21 @@ bin/solr start -Dsolr.node.roles=data:on,overseer:preferred
A node started this way asks the Overseer to re-run its node prioritization,
so a preferred node takes over without waiting for the current Overseer to
restart.
Note that node roles are fixed for the lifetime of a node: unlike `ADDROLE`,
they cannot be changed on a running node.
+
+=== V2 cluster status
+
+`GET /api/cluster` is now a typed API.
Review Comment:
this may get backported to solr 10, and honestly, since we are changing v1,
i don't know if this ends up being a major change? since i don't think it's
widely used yet?
##########
solr/core/src/test/org/apache/solr/handler/admin/api/GetClusterStatusTest.java:
##########
@@ -0,0 +1,236 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.handler.admin.api;
+
+import static org.apache.solr.client.solrj.SolrRequest.METHOD.GET;
+
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import org.apache.solr.client.api.model.GetClusterStatusResponse;
+import
org.apache.solr.client.api.model.GetClusterStatusResponse.CollectionState;
+import org.apache.solr.client.api.model.GetClusterStatusResponse.ReplicaState;
+import org.apache.solr.client.solrj.SolrRequest.SolrRequestType;
+import org.apache.solr.client.solrj.request.ClusterApi;
+import org.apache.solr.client.solrj.request.CollectionAdminRequest;
+import org.apache.solr.client.solrj.request.GenericSolrRequest;
+import org.apache.solr.cloud.SolrCloudTestCase;
+import org.apache.solr.common.params.ModifiableSolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.common.util.Utils;
+import org.eclipse.jetty.client.ContentResponse;
+import org.eclipse.jetty.client.HttpClient;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+/**
+ * HTTP tests for {@code GET /api/cluster}.
+ *
+ * <p>The response is the collections, shards, and replicas tree. Live nodes,
the alias map, and
+ * cluster properties stay on their own endpoints. v1 {@code CLUSTERSTATUS}
still returns them.
+ */
+public class GetClusterStatusTest extends SolrCloudTestCase {
+
+ private static final String COLLECTION = "clusterstatuscoll";
+ private static final String ALIAS = "clusterstatusalias";
+
+ @BeforeClass
+ public static void setupCluster() throws Exception {
+ configureCluster(1).addConfig("conf",
configset("cloud-minimal")).configure();
+ CollectionAdminRequest.createCollection(COLLECTION, "conf", 1, 1)
+ .process(cluster.getSolrClient());
+ CollectionAdminRequest.createAlias(ALIAS,
COLLECTION).process(cluster.getSolrClient());
+ }
+
+ @Test
+ public void testReturnsCollectionTreeWithoutClusterLevelExtras() throws
Exception {
+ Map<String, Object> clusterState = clusterObject(getCluster(""));
+ assertNull(clusterState.get("live_nodes"));
+ assertNull(clusterState.get("aliases"));
+ assertNull(clusterState.get("properties"));
+
+ Map<String, Object> collection = collection(clusterState, COLLECTION);
+ assertNotNull(collection.get("health"));
+ assertEquals("conf", collection.get("configName"));
+ assertNotNull(collection.get("router"));
+ assertEquals(Set.of("shard1"), shards(collection).keySet());
+ assertNotNull(replica(collection).get("node_name"));
+ assertNotNull(replica(collection).get("state"));
+ assertTrue(aliasesOf(collection).contains(ALIAS));
+ }
+
+ @Test
+ @SuppressWarnings("unchecked")
+ public void testGeneratedClientReadsTheTree() throws Exception {
+ GetClusterStatusResponse response =
+ new ClusterApi.GetClusterStatus().process(cluster.getSolrClient());
+ assertNull(response.error);
+ CollectionState collection = response.cluster.collections.get(COLLECTION);
+ assertNotNull(collection);
+ assertEquals("conf", collection.configName);
+ assertNotNull(collection.health);
+ assertTrue(collection.aliases.contains(ALIAS));
+ Map<String, Object> router = (Map<String, Object>)
collection.unknownProperties().get("router");
+ assertEquals("compositeId", router.get("name"));
+
+ ReplicaState replica =
collection.shards.get("shard1").replicas.values().iterator().next();
+ assertNotNull(replica.nodeName);
+ assertNotNull(replica.state);
+ assertEquals("true", replica.leader);
+ }
+
+ @Test
+ public void testV1SelectionParamsAreIgnored() throws Exception {
+ Map<String, Object> clusterState =
+ clusterObject(
+
getCluster("?liveNodes=true&aliases=true&clusterProperties=true&includeAll=true"));
+ assertNull(clusterState.get("live_nodes"));
Review Comment:
wht happens when random params are provided? do we blow up or do we ignore
them? what is the Jersey "right" answer for that?
##########
solr/solr-ref-guide/modules/configuration-guide/pages/v2-api.adoc:
##########
Review Comment:
I think we need to double check this page... do we actually want this
level of doc on this page? every api will be a v2 api, and we have the tabs
pattern.... I think maybe we are reducing this page (in another PR?) content
to just maybe what is the v2 api and what a user would need to know.
##########
solr/core/src/test/org/apache/solr/handler/admin/api/GetClusterStatusTest.java:
##########
@@ -0,0 +1,236 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.handler.admin.api;
+
+import static org.apache.solr.client.solrj.SolrRequest.METHOD.GET;
+
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import org.apache.solr.client.api.model.GetClusterStatusResponse;
+import
org.apache.solr.client.api.model.GetClusterStatusResponse.CollectionState;
+import org.apache.solr.client.api.model.GetClusterStatusResponse.ReplicaState;
+import org.apache.solr.client.solrj.SolrRequest.SolrRequestType;
+import org.apache.solr.client.solrj.request.ClusterApi;
+import org.apache.solr.client.solrj.request.CollectionAdminRequest;
+import org.apache.solr.client.solrj.request.GenericSolrRequest;
+import org.apache.solr.cloud.SolrCloudTestCase;
+import org.apache.solr.common.params.ModifiableSolrParams;
+import org.apache.solr.common.util.NamedList;
+import org.apache.solr.common.util.Utils;
+import org.eclipse.jetty.client.ContentResponse;
+import org.eclipse.jetty.client.HttpClient;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+/**
+ * HTTP tests for {@code GET /api/cluster}.
+ *
+ * <p>The response is the collections, shards, and replicas tree. Live nodes,
the alias map, and
+ * cluster properties stay on their own endpoints. v1 {@code CLUSTERSTATUS}
still returns them.
+ */
+public class GetClusterStatusTest extends SolrCloudTestCase {
+
+ private static final String COLLECTION = "clusterstatuscoll";
+ private static final String ALIAS = "clusterstatusalias";
+
+ @BeforeClass
+ public static void setupCluster() throws Exception {
+ configureCluster(1).addConfig("conf",
configset("cloud-minimal")).configure();
+ CollectionAdminRequest.createCollection(COLLECTION, "conf", 1, 1)
+ .process(cluster.getSolrClient());
+ CollectionAdminRequest.createAlias(ALIAS,
COLLECTION).process(cluster.getSolrClient());
+ }
+
+ @Test
+ public void testReturnsCollectionTreeWithoutClusterLevelExtras() throws
Exception {
+ Map<String, Object> clusterState = clusterObject(getCluster(""));
+ assertNull(clusterState.get("live_nodes"));
Review Comment:
yeah, I suppose!
##########
solr/solr-ref-guide/modules/deployment-guide/pages/cluster-node-management.adoc:
##########
@@ -106,6 +111,9 @@ This can be used if you need the details of the shard where
a particular documen
|Optional |Default: will default to the default value of `includeAll`
parameter specified below
|===
+
+v1 only.
+If set to true, returns the cluster alias map.
+The v2 API does not return this map; use `GET /api/aliases`.
Review Comment:
nice!
##########
solr/api/src/java/org/apache/solr/client/api/model/GetClusterStatusResponse.java:
##########
@@ -0,0 +1,134 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.client.api.model;
+
+import com.fasterxml.jackson.annotation.JsonAnyGetter;
+import com.fasterxml.jackson.annotation.JsonAnySetter;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import io.swagger.v3.oas.annotations.media.Schema;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+
+/**
+ * Response body for {@code GET /api/cluster}.
+ *
+ * <p>Shards, replicas, health, and the other stable collection-state fields
are typed. A collection
+ * state document is otherwise open: router settings, replica placement
counts, user properties, and
+ * per-replica state are preserved on the object rather than dropped. Live
nodes, the alias map, and
+ * cluster properties are not part of this response. Each collection still
lists the aliases that
+ * point at it.
+ */
+public class GetClusterStatusResponse extends SolrJerseyResponse {
+
+ @Schema(
+ description =
+ "Cluster geometry. Does not include live nodes, the alias map, or
cluster properties.")
Review Comment:
i think we don't need the the Does Not aspects in the descritiption. We
highlight that int he docs..
##########
solr/core/src/java/org/apache/solr/handler/admin/api/GetClusterStatus.java:
##########
@@ -0,0 +1,91 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.handler.admin.api;
+
+import static
org.apache.solr.security.PermissionNameProvider.Name.COLL_READ_PERM;
+
+import jakarta.inject.Inject;
+import java.util.Map;
+import org.apache.solr.client.api.endpoint.GetClusterStatusApi;
+import org.apache.solr.client.api.model.GetClusterStatusResponse;
+import
org.apache.solr.client.api.model.GetClusterStatusResponse.CollectionState;
+import org.apache.solr.common.params.ModifiableSolrParams;
+import org.apache.solr.common.params.ShardParams;
+import org.apache.solr.common.util.CollectionUtil;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.handler.admin.ClusterStatus;
+import org.apache.solr.jersey.PermissionName;
+import org.apache.solr.jersey.SolrJacksonMapper;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.response.SolrQueryResponse;
+
+/**
+ * V2 API for the collections, shards, and replicas tree.
+ *
+ * <p>{@code GET /api/cluster} returns that tree and nothing else hanging off
the cluster object.
+ * Live nodes, aliases, and cluster properties stay on their own endpoints. v1
{@code CLUSTERSTATUS}
+ * is unchanged.
+ */
+public class GetClusterStatus extends AdminAPIBase implements
GetClusterStatusApi {
+
+ @Inject
+ public GetClusterStatus(
+ CoreContainer coreContainer, SolrQueryRequest req, SolrQueryResponse
rsp) {
+ super(coreContainer, req, rsp);
+ }
+
+ @Override
+ @PermissionName(COLL_READ_PERM)
+ public GetClusterStatusResponse getClusterStatus(
+ String collection, String shard, String routeKey, Boolean prs) throws
Exception {
+ final GetClusterStatusResponse response =
+ instantiateJerseyResponse(GetClusterStatusResponse.class);
+ validateZooKeeperAwareCoreContainer(coreContainer);
+ if (collection != null) {
+ recordCollectionForLogAndTracing(collection, solrQueryRequest);
+ }
+
+ // Only the parameters this API documents. includeAll, liveNodes, aliases,
and
Review Comment:
what happens if you pass them in? does it get run?
--
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]
