From f4b6c6fbc3cc5c747871e3b3ae4614fa780229e3 Mon Sep 17 00:00:00 2001 From: Matthew Biscocho Date: Mon, 3 Feb 2025 10:53:26 -0500 Subject: [PATCH 1/3] Stream CLUSTERSTATUS API for SolrJ version >= 9.9 --- .../java/org/apache/solr/cli/StatusTool.java | 2 +- .../solr/handler/admin/ClusterStatus.java | 35 ++++++++-------- .../handler/admin/CollectionsHandler.java | 2 +- .../api/collections/TestCollectionAPI.java | 32 +++++++------- .../impl/BaseHttpClusterStateProvider.java | 9 ++-- .../solrj/impl/ClusterStateProviderTest.java | 42 ++++++++++++++++++- 6 files changed, 81 insertions(+), 41 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 fa6c2cbf2bd..a935a7b364f 100644 --- a/solr/core/src/java/org/apache/solr/cli/StatusTool.java +++ b/solr/core/src/java/org/apache/solr/cli/StatusTool.java @@ -353,7 +353,7 @@ protected Map getCloudStatus(SolrClient solrClient, String zkHos cloudStatus.put("liveNodes", String.valueOf(liveNodes.size())); // TODO get this as a metric from the metrics API instead, or something else. - var collections = (NamedList) json.findRecursive("cluster", "collections"); + var collections = (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 7a8ecf9c850..20f3022b2a8 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 @@ -26,6 +26,7 @@ import java.util.Objects; import java.util.Set; import java.util.stream.Stream; +import org.apache.solr.client.api.util.SolrVersion; import org.apache.solr.common.MapWriter; import org.apache.solr.common.SolrException; import org.apache.solr.common.cloud.Aliases; @@ -34,7 +35,6 @@ import org.apache.solr.common.cloud.PerReplicaStates; import org.apache.solr.common.cloud.Replica; import org.apache.solr.common.cloud.ZkStateReader; -import org.apache.solr.common.params.CommonParams; import org.apache.solr.common.params.ShardParams; import org.apache.solr.common.params.SolrParams; import org.apache.solr.common.util.NamedList; @@ -101,7 +101,7 @@ public ClusterStatus(ZkStateReader zkStateReader, SolrParams params) { collection = params.get(ZkStateReader.COLLECTION_PROP); } - public void getClusterStatus(NamedList results) + public void getClusterStatus(NamedList results, SolrVersion solrVersion) throws KeeperException, InterruptedException { NamedList clusterStatus = new SimpleOrderedMap<>(); @@ -127,7 +127,7 @@ public void getClusterStatus(NamedList results) if (withCollection) { assert liveNodes != null; - fetchClusterStatusForCollOrAlias(clusterStatus, liveNodes, aliases); + fetchClusterStatusForCollOrAlias(clusterStatus, liveNodes, aliases, solrVersion); } if (withAliases) { @@ -158,7 +158,10 @@ public void getClusterStatus(NamedList results) } private void fetchClusterStatusForCollOrAlias( - NamedList clusterStatus, List liveNodes, Aliases aliases) { + NamedList clusterStatus, + List liveNodes, + Aliases aliases, + SolrVersion solrVersion) { // read aliases Map> collectionVsAliases = new HashMap<>(); @@ -206,19 +209,7 @@ private void fetchClusterStatusForCollOrAlias( } } - // Because of back-compat for SolrJ, create the whole response into a NamedList - // Otherwise stream with MapWriter to save memory - if (CommonParams.JAVABIN.equals(solrParams.get(CommonParams.WT))) { - NamedList collectionProps = new SimpleOrderedMap<>(); - collectionStream.forEach( - collectionState -> { - collectionProps.add( - collectionState.getName(), - buildResponseForCollection( - collectionState, collectionVsAliases, routeKey, liveNodes, requestedShards)); - }); - clusterStatus.add("collections", collectionProps); - } else { + if (solrVersion.greaterThanOrEqualTo(SolrVersion.valueOf("9.9.0"))) { MapWriter collectionPropsWriter = ew -> { collectionStream.forEach( @@ -234,6 +225,16 @@ private void fetchClusterStatusForCollOrAlias( }); }; clusterStatus.add("collections", collectionPropsWriter); + } else { + NamedList collectionProps = new SimpleOrderedMap<>(); + collectionStream.forEach( + collectionState -> { + collectionProps.add( + collectionState.getName(), + buildResponseForCollection( + collectionState, collectionVsAliases, routeKey, liveNodes, requestedShards)); + }); + clusterStatus.add("collections", collectionProps); } } diff --git a/solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java b/solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java index aefc1033d5e..3b22b3072e5 100644 --- a/solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java +++ b/solr/core/src/java/org/apache/solr/handler/admin/CollectionsHandler.java @@ -975,7 +975,7 @@ public Map execute( CLUSTERSTATUS, (req, rsp, h) -> { new ClusterStatus(h.coreContainer.getZkController().getZkStateReader(), req.getParams()) - .getClusterStatus(rsp.getValues()); + .getClusterStatus(rsp.getValues(), req.getHttpSolrCall().getUserAgentSolrVersion()); return null; }), ADDREPLICAPROP_OP( 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 0aa5d4ae2d0..ce605c70271 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 @@ -131,10 +131,11 @@ 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); + assertEquals("25", collectionProperties.get("replicationFactor")); params = new ModifiableSolrParams(); params.set("action", CollectionParams.CollectionAction.MODIFYCOLLECTION.toString()); @@ -153,10 +154,11 @@ 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); + assertNull(collectionProperties.get("replicationFactor")); params = new ModifiableSolrParams(); params.set("action", CollectionParams.CollectionAction.MODIFYCOLLECTION.toString()); @@ -255,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)); @@ -282,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"}) @@ -304,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()); @@ -330,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()); @@ -465,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()); @@ -487,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"}) @@ -517,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); @@ -533,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); @@ -560,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()); @@ -607,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); @@ -627,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 a09d936582e..c7dc6333b80 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 @@ -46,7 +46,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.URLUtil; import org.apache.solr.common.util.Utils; @@ -160,11 +159,11 @@ private ClusterState fetchClusterState(SolrClient client) liveNodesTimestamp = System.nanoTime(); } - var collectionsNl = (NamedList>) cluster.get("collections"); + var collectionsMap = (Map>) cluster.get("collections"); Map collStateByName = - CollectionUtil.newLinkedHashMap(collectionsNl.size()); - for (Entry> entry : collectionsNl) { + CollectionUtil.newLinkedHashMap(collectionsMap.size()); + for (Entry> entry : collectionsMap.entrySet()) { collStateByName.put( entry.getKey(), getDocCollectionFromObjects(entry.getKey(), entry.getValue())); } @@ -205,7 +204,7 @@ private DocCollection fetchCollectionState(SolrClient client, String collection) SimpleOrderedMap cluster = submitClusterStateRequest(client, collection, ClusterStateRequestType.FETCH_COLLECTION); - var collStateMap = (Map) cluster.findRecursive("collections", collection); + var collStateMap = (Map) cluster._get(List.of("collections", collection), null); 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 f3ee722ee58..0d884a5271d 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 @@ -23,6 +23,7 @@ import com.carrotsearch.randomizedtesting.annotations.ParametersFactory; import java.io.IOException; +import java.lang.invoke.MethodHandles; import java.lang.reflect.InvocationTargetException; import java.time.Instant; import java.util.List; @@ -36,6 +37,8 @@ import org.apache.solr.common.cloud.ClusterState; import org.apache.solr.common.cloud.DocCollection; import org.apache.solr.common.util.NamedList; +import org.eclipse.jetty.http.HttpField; +import org.eclipse.jetty.http.HttpHeader; import org.hamcrest.Matchers; import org.junit.After; import org.junit.BeforeClass; @@ -135,7 +138,6 @@ public void testGetState() throws Exception { try (ClusterStateProvider provider = createClusterStateProvider()) { ClusterState.CollectionRef collectionRef = provider.getState("testGetState"); - DocCollection docCollection = collectionRef.get(); assertNotNull(docCollection); assertEquals( @@ -159,7 +161,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")); } @@ -198,6 +200,42 @@ public void testClusterStateProvider() throws SolrServerException, IOException { } } + @Test + public void testClusterStateProviderBackwardCompatability() + throws SolrServerException, IOException { + CollectionAdminRequest.setClusterProperty("ext.foo", "bar").process(cluster.getSolrClient()); + createCollection("col1"); + createCollection("col2"); + + try (var cspZk = zkClientClusterStateProvider(); + var cspHttp = http2ClusterStateProvider()) { + // SolrJ < version 9.9.0 for non streamed response + cspHttp + .getHttpClient() + .getHttpClient() + .setUserAgentField( + new HttpField( + HttpHeader.USER_AGENT, + "Solr[" + MethodHandles.lookup().lookupClass().getName() + "] " + "9.8.0")); + + assertThat(cspHttp.getCollection("col1"), equalTo(cspZk.getCollection("col1"))); + + final var clusterStateZk = cspZk.getClusterState(); + final var clusterStateHttp = cspHttp.getClusterState(); + assertThat( + clusterStateHttp.getLiveNodes(), + containsInAnyOrder(clusterStateHttp.getLiveNodes().toArray())); + assertEquals(2, clusterStateZk.size()); + assertEquals(clusterStateZk.size(), clusterStateHttp.size()); + assertThat( + clusterStateHttp.collectionStream().toList(), + containsInAnyOrder(clusterStateHttp.collectionStream().toArray())); + + assertThat( + clusterStateZk.getCollection("col2"), equalTo(clusterStateHttp.getCollection("col2"))); + } + } + @Test public void testClusterStateProviderDownedInitialLiveNodes() throws Exception { try (var cspHttp = http2ClusterStateProvider()) { From f0fbbf5447e06688205c2216f40de37aabb3b2e7 Mon Sep 17 00:00:00 2001 From: Matthew Biscocho Date: Mon, 3 Feb 2025 11:23:04 -0500 Subject: [PATCH 2/3] in-line map loop --- .../client/solrj/impl/BaseHttpClusterStateProvider.java | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) 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 c7dc6333b80..150b4fe0de2 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 @@ -29,7 +29,6 @@ import java.util.Collections; import java.util.List; import java.util.Map; -import java.util.Map.Entry; import java.util.Set; import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; @@ -163,11 +162,8 @@ private ClusterState fetchClusterState(SolrClient client) Map collStateByName = CollectionUtil.newLinkedHashMap(collectionsMap.size()); - for (Entry> entry : collectionsMap.entrySet()) { - collStateByName.put( - entry.getKey(), getDocCollectionFromObjects(entry.getKey(), entry.getValue())); - } - + collectionsMap.forEach( + (key, value) -> collStateByName.put(key, getDocCollectionFromObjects(key, value))); return new ClusterState(this.liveNodes, collStateByName); } From ba76edc6b1afa2ffd446f028d0b55c7a9dd5bb97 Mon Sep 17 00:00:00 2001 From: Matthew Biscocho Date: Mon, 3 Feb 2025 12:35:41 -0500 Subject: [PATCH 3/3] Add solrVersion null check and test --- .../solr/handler/admin/ClusterStatus.java | 2 +- .../solrj/impl/ClusterStateProviderTest.java | 32 +++++++++++++++++-- 2 files changed, 31 insertions(+), 3 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 20f3022b2a8..6ef549698d3 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 @@ -209,7 +209,7 @@ private void fetchClusterStatusForCollOrAlias( } } - if (solrVersion.greaterThanOrEqualTo(SolrVersion.valueOf("9.9.0"))) { + if (solrVersion == null || solrVersion.greaterThanOrEqualTo(SolrVersion.valueOf("9.9.0"))) { MapWriter collectionPropsWriter = ew -> { collectionStream.forEach( 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 0d884a5271d..5c6ce5fbdb9 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 @@ -201,8 +201,7 @@ public void testClusterStateProvider() throws SolrServerException, IOException { } @Test - public void testClusterStateProviderBackwardCompatability() - throws SolrServerException, IOException { + public void testClusterStateProviderOldVersion() throws SolrServerException, IOException { CollectionAdminRequest.setClusterProperty("ext.foo", "bar").process(cluster.getSolrClient()); createCollection("col1"); createCollection("col2"); @@ -236,6 +235,35 @@ public void testClusterStateProviderBackwardCompatability() } } + @Test + public void testClusterStateProviderEmptySolrVersion() throws SolrServerException, IOException { + CollectionAdminRequest.setClusterProperty("ext.foo", "bar").process(cluster.getSolrClient()); + createCollection("col1"); + createCollection("col2"); + + try (var cspZk = zkClientClusterStateProvider(); + var cspHttp = http2ClusterStateProvider()) { + + cspHttp.getHttpClient().getHttpClient().setUserAgentField(null); + + assertThat(cspHttp.getCollection("col1"), equalTo(cspZk.getCollection("col1"))); + + final var clusterStateZk = cspZk.getClusterState(); + final var clusterStateHttp = cspHttp.getClusterState(); + assertThat( + clusterStateHttp.getLiveNodes(), + containsInAnyOrder(clusterStateHttp.getLiveNodes().toArray())); + assertEquals(2, clusterStateZk.size()); + assertEquals(clusterStateZk.size(), clusterStateHttp.size()); + assertThat( + clusterStateHttp.collectionStream().toList(), + containsInAnyOrder(clusterStateHttp.collectionStream().toArray())); + + assertThat( + clusterStateZk.getCollection("col2"), equalTo(clusterStateHttp.getCollection("col2"))); + } + } + @Test public void testClusterStateProviderDownedInitialLiveNodes() throws Exception { try (var cspHttp = http2ClusterStateProvider()) {