This is an automated email from the ASF dual-hosted git repository.
dsmiley pushed a commit to branch branch_9x
in repository https://gitbox.apache.org/repos/asf/solr.git
The following commit(s) were added to refs/heads/branch_9x by this push:
new a9e7506c618 SOLR-17381: Fix CloudSolrClient.getClusterState when HTTP
CSP (not ZK) (#2853)
a9e7506c618 is described below
commit a9e7506c618e392d357dd5cb636a0c8772e79fc5
Author: aparnasuresh85 <aparna.sur...@salesforce.com>
AuthorDate: Mon Dec 9 11:30:41 2024 -0500
SOLR-17381: Fix CloudSolrClient.getClusterState when HTTP CSP (not ZK)
(#2853)
When using the HTTP ClusterStateProvider (not ZK), getClusterState() wasn't
working correctly; a regression from the first PR for this JIRA issue (not
released).
Also,
* Optimization: fix O(N^2) algorithm to be O(N) for the number of
collections when calling getClusterState
* liveNodes is now immutable, and probably a non-sorted ordering
* removed "health" and some other keys from DocCollection that aren't
present when using ZK CSP
Minor details:
* clearly differentiate internal ClusterState processing from getting one
DocCollection
* Use GenericSolrRequest not QueryRequest
* test the both CSPs better
---------
Co-authored-by: David Smiley <dsmi...@salesforce.com>
(cherry picked from commit d2045f679e55e15cf4e974947b9417c4465fccd2)
---
solr/CHANGES.txt | 4 +-
.../solrj/impl/BaseHttpClusterStateProvider.java | 160 ++++++++++-----------
.../solrj/impl/ClusterStateProviderTest.java | 80 ++++++++++-
3 files changed, 156 insertions(+), 88 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index e332efe7bea..d948bb5801a 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -64,7 +64,9 @@ Optimizations
that which consumes almost no memory, saving 1MB of memory per SolrCore.
(David Smiley)
* SOLR-17381: Make CLUSTERSTATUS request configurable to improve performance
by allowing retrieval of specific information,
- reducing unnecessary data fetching. (Aparna Suresh, David Smiley)
+ reducing unnecessary data fetching. Enhanced CloudSolrClient's HTTP
ClusterStateProvider to use it, and to scale to
+ more collections better as well.
+ (Aparna Suresh, David Smiley)
* SOLR-17396: Reduce thread contention in
ZkStateReader.getCollectionProperties(). (Aparna Suresh, David Smiley, Paul
McArthur)
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 75c50167d2b..ed77397145d 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
@@ -23,22 +23,23 @@ import java.io.IOException;
import java.lang.invoke.MethodHandles;
import java.time.Instant;
import java.util.Collections;
-import java.util.HashMap;
-import java.util.HashSet;
import java.util.List;
import java.util.Map;
+import java.util.Map.Entry;
import java.util.Set;
import java.util.concurrent.TimeUnit;
import org.apache.solr.client.solrj.SolrClient;
+import org.apache.solr.client.solrj.SolrRequest.METHOD;
import org.apache.solr.client.solrj.SolrServerException;
import org.apache.solr.client.solrj.request.CollectionAdminRequest;
-import org.apache.solr.client.solrj.request.QueryRequest;
+import org.apache.solr.client.solrj.request.GenericSolrRequest;
import org.apache.solr.client.solrj.response.CollectionAdminResponse;
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.PerReplicaStates;
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;
@@ -98,8 +99,8 @@ public abstract class BaseHttpClusterStateProvider implements
ClusterStateProvid
for (String nodeName : liveNodes) {
String baseUrl = Utils.getBaseUrlForNodeName(nodeName, urlScheme);
try (SolrClient client = getSolrClient(baseUrl)) {
- ClusterState cs = fetchClusterState(client, collection, null);
- return cs.getCollectionRef(collection);
+ DocCollection docCollection = fetchCollectionState(client, collection);
+ return new ClusterState.CollectionRef(docCollection);
} catch (SolrServerException | IOException e) {
log.warn(
"Attempt to fetch cluster state from {} failed.",
@@ -128,90 +129,43 @@ public abstract class BaseHttpClusterStateProvider
implements ClusterStateProvid
}
@SuppressWarnings("unchecked")
- private ClusterState fetchClusterState(
- SolrClient client, String collection, Map<String, Object>
clusterProperties)
+ private ClusterState fetchClusterState(SolrClient client)
throws SolrServerException, IOException, NotACollectionException {
SimpleOrderedMap<?> cluster =
- submitClusterStateRequest(client, collection,
ClusterStateRequestType.FETCH_COLLECTION);
-
- Map<String, Object> collectionsMap;
- if (collection != null) {
- collectionsMap =
- Collections.singletonMap(
- collection, ((NamedList<?>)
cluster.get("collections")).get(collection));
- } else {
- collectionsMap = ((NamedList<?>) cluster.get("collections")).asMap(10);
- }
- int znodeVersion;
- Map<String, Object> collFromStatus = (Map<String, Object>)
(collectionsMap).get(collection);
- if (collection != null && collFromStatus == null) {
- throw new NotACollectionException(); // probably an alias
- }
- if (collection != null) { // can be null if alias
- znodeVersion = (int) collFromStatus.get("znodeVersion");
- } else {
- znodeVersion = -1;
- }
+ submitClusterStateRequest(client, null,
ClusterStateRequestType.FETCH_CLUSTER_STATE);
- ClusterState cs = new ClusterState(this.liveNodes, new HashMap<>());
List<String> liveNodesList = (List<String>) cluster.get("live_nodes");
if (liveNodesList != null) {
- Set<String> liveNodes = new HashSet<>(liveNodesList);
- this.liveNodes = liveNodes;
+ this.liveNodes = Set.copyOf(liveNodesList);
liveNodesTimestamp = System.nanoTime();
- cs = new ClusterState(liveNodes, new HashMap<>());
}
- for (Map.Entry<String, Object> e : collectionsMap.entrySet()) {
- @SuppressWarnings("rawtypes")
- Map m = (Map) e.getValue();
- Long creationTimeMillisFromClusterStatus = (Long)
m.get("creationTimeMillis");
- Instant creationTime =
- creationTimeMillisFromClusterStatus == null
- ? Instant.EPOCH
- : Instant.ofEpochMilli(creationTimeMillisFromClusterStatus);
- cs = cs.copyWith(e.getKey(), fillPrs(znodeVersion, e, creationTime, m));
- }
+ var collectionsNl = (NamedList<Map<String, Object>>)
cluster.get("collections");
- if (clusterProperties != null) {
- Map<String, Object> properties = (Map<String, Object>)
cluster.get("properties");
- if (properties != null) {
- clusterProperties.putAll(properties);
- }
+ Map<String, DocCollection> collStateByName =
+ CollectionUtil.newLinkedHashMap(collectionsNl.size());
+ for (Entry<String, Map<String, Object>> entry : collectionsNl) {
+ collStateByName.put(
+ entry.getKey(), getDocCollectionFromObjects(entry.getKey(),
entry.getValue()));
}
- return cs;
- }
- private SimpleOrderedMap<?> submitClusterStateRequest(
- SolrClient client, String collection, ClusterStateRequestType
requestType)
- throws SolrServerException, IOException {
+ return new ClusterState(this.liveNodes, collStateByName);
+ }
- ModifiableSolrParams params = new ModifiableSolrParams();
- params.set("action", "CLUSTERSTATUS");
+ @SuppressWarnings("unchecked")
+ private DocCollection getDocCollectionFromObjects(
+ String collectionName, Map<String, Object> collStateMap) {
+ collStateMap.remove("health");
- if (requestType == ClusterStateRequestType.FETCH_COLLECTION && collection
!= null) {
- params.set("collection", collection);
- } else if (requestType == ClusterStateRequestType.FETCH_LIVE_NODES) {
- params.set("liveNodes", "true");
- } else if (requestType == ClusterStateRequestType.FETCH_CLUSTER_PROP) {
- params.set("clusterProperties", "true");
- } else if (requestType == ClusterStateRequestType.FETCH_NODE_ROLES) {
- params.set("roles", "true");
- }
+ int zNodeVersion = (int) collStateMap.remove("znodeVersion");
- params.set("includeAll", "false");
- params.set("prs", "true");
- QueryRequest request = new QueryRequest(params);
- request.setPath("/admin/collections");
- return (SimpleOrderedMap<?>) client.request(request).get("cluster");
- }
+ Long creationTimeMillis = (Long) collStateMap.remove("creationTimeMillis");
+ Instant creationTime =
+ creationTimeMillis == null ? Instant.EPOCH :
Instant.ofEpochMilli(creationTimeMillis);
- @SuppressWarnings({"rawtypes", "unchecked"})
- private DocCollection fillPrs(
- int znodeVersion, Map.Entry<String, Object> e, Instant creationTime, Map
m) {
DocCollection.PrsSupplier prsSupplier = null;
- if (m.containsKey("PRS")) {
- Map prs = (Map) m.remove("PRS");
+ Map<String, Object> prs = (Map<String, Object>) collStateMap.remove("PRS");
+ if (prs != null) {
prsSupplier =
() ->
new PerReplicaStates(
@@ -221,7 +175,51 @@ public abstract class BaseHttpClusterStateProvider
implements ClusterStateProvid
}
return ClusterState.collectionFromObjects(
- e.getKey(), m, znodeVersion, creationTime, prsSupplier);
+ collectionName, collStateMap, zNodeVersion, creationTime, prsSupplier);
+ }
+
+ @SuppressWarnings("unchecked")
+ private DocCollection fetchCollectionState(SolrClient client, String
collection)
+ throws SolrServerException, IOException, NotACollectionException {
+
+ SimpleOrderedMap<?> cluster =
+ submitClusterStateRequest(client, collection,
ClusterStateRequestType.FETCH_COLLECTION);
+
+ var collStateMap = (Map<String, Object>)
cluster.findRecursive("collections", collection);
+ if (collStateMap == null) {
+ throw new NotACollectionException(); // probably an alias
+ }
+ return getDocCollectionFromObjects(collection, collStateMap);
+ }
+
+ private SimpleOrderedMap<?> submitClusterStateRequest(
+ SolrClient client, String collection, ClusterStateRequestType
requestType)
+ throws SolrServerException, IOException {
+
+ ModifiableSolrParams params = new ModifiableSolrParams();
+ params.set("action", "CLUSTERSTATUS");
+
+ params.set("includeAll", false); // will flip flor CLUSTER_STATE
+ switch (requestType) {
+ case FETCH_CLUSTER_STATE:
+ params.set("includeAll", true);
+ break;
+ case FETCH_COLLECTION:
+ if (collection != null) params.set("collection", collection);
+ break;
+ case FETCH_LIVE_NODES:
+ params.set("liveNodes", true);
+ break;
+ case FETCH_CLUSTER_PROP:
+ params.set("clusterProperties", true);
+ break;
+ case FETCH_NODE_ROLES:
+ params.set("roles", true);
+ break;
+ }
+ params.set("prs", true);
+ var request = new GenericSolrRequest(METHOD.GET, "/admin/collections",
params);
+ return (SimpleOrderedMap<?>) client.request(request).get("cluster");
}
@Override
@@ -260,12 +258,11 @@ public abstract class BaseHttpClusterStateProvider
implements ClusterStateProvid
}
}
- @SuppressWarnings({"rawtypes", "unchecked"})
+ @SuppressWarnings({"unchecked"})
private Set<String> fetchLiveNodes(SolrClient client) throws Exception {
-
SimpleOrderedMap<?> cluster =
submitClusterStateRequest(client, null,
ClusterStateRequestType.FETCH_LIVE_NODES);
- return (Set<String>) new HashSet((List<String>)
(cluster.get("live_nodes")));
+ return Set.copyOf((List<String>) cluster.get("live_nodes"));
}
@Override
@@ -346,8 +343,8 @@ public abstract class BaseHttpClusterStateProvider
implements ClusterStateProvid
for (String nodeName : liveNodes) {
String baseUrl = Utils.getBaseUrlForNodeName(nodeName, urlScheme);
try (SolrClient client = getSolrClient(baseUrl)) {
- return fetchClusterState(client, null, null);
- } catch (SolrServerException | BaseHttpSolrClient.RemoteSolrException |
IOException e) {
+ return fetchClusterState(client);
+ } catch (SolrServerException | RemoteSolrException | IOException e) {
log.warn("Attempt to fetch cluster state from {} failed.", baseUrl, e);
} catch (NotACollectionException e) {
// not possible! (we passed in null for collection, so it can't be an
alias)
@@ -376,7 +373,7 @@ public abstract class BaseHttpClusterStateProvider
implements ClusterStateProvid
SimpleOrderedMap<?> cluster =
submitClusterStateRequest(client, null,
ClusterStateRequestType.FETCH_CLUSTER_PROP);
return (Map<String, Object>) cluster.get("properties");
- } catch (SolrServerException | BaseHttpSolrClient.RemoteSolrException |
IOException e) {
+ } catch (SolrServerException | RemoteSolrException | IOException e) {
log.warn("Attempt to fetch cluster state from {} failed.", baseUrl, e);
}
}
@@ -428,6 +425,7 @@ public abstract class BaseHttpClusterStateProvider
implements ClusterStateProvid
FETCH_LIVE_NODES,
FETCH_CLUSTER_PROP,
FETCH_NODE_ROLES,
- FETCH_COLLECTION
+ FETCH_COLLECTION,
+ FETCH_CLUSTER_STATE
}
}
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 c181a59e7da..4c7741c8ee4 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
@@ -17,10 +17,17 @@
package org.apache.solr.client.solrj.impl;
+import static org.hamcrest.Matchers.containsInAnyOrder;
+import static org.hamcrest.Matchers.equalTo;
+
+import com.carrotsearch.randomizedtesting.annotations.ParametersFactory;
import java.io.IOException;
+import java.lang.reflect.InvocationTargetException;
import java.time.Instant;
import java.util.List;
import java.util.Map;
+import java.util.function.Supplier;
+import java.util.stream.Collectors;
import org.apache.solr.client.solrj.SolrServerException;
import org.apache.solr.client.solrj.request.CollectionAdminRequest;
import org.apache.solr.client.solrj.response.CollectionAdminResponse;
@@ -28,6 +35,7 @@ import org.apache.solr.cloud.SolrCloudTestCase;
import org.apache.solr.common.cloud.ClusterState;
import org.apache.solr.common.cloud.DocCollection;
import org.apache.solr.common.util.NamedList;
+import org.hamcrest.Matchers;
import org.junit.BeforeClass;
import org.junit.Test;
@@ -47,19 +55,44 @@ public class ClusterStateProviderTest extends
SolrCloudTestCase {
.configure();
}
- private ClusterStateProvider createClusterStateProvider() throws Exception {
- return !usually() ? http2ClusterStateProvider() :
zkClientClusterStateProvider();
+ @ParametersFactory
+ public static Iterable<String[]> parameters() throws NoSuchMethodException {
+ return List.of(
+ new String[] {"http2ClusterStateProvider"}, new String[]
{"zkClientClusterStateProvider"});
}
- private ClusterStateProvider http2ClusterStateProvider() throws Exception {
- return new Http2ClusterStateProvider(
- List.of(cluster.getJettySolrRunner(0).getBaseUrl().toString()), null);
+ private static ClusterStateProvider http2ClusterStateProvider() {
+ try {
+ return new Http2ClusterStateProvider(
+ List.of(cluster.getJettySolrRunner(0).getBaseUrl().toString()),
null);
+ } catch (Exception e) {
+ throw new RuntimeException(e);
+ }
}
- private ClusterStateProvider zkClientClusterStateProvider() {
+ private static ClusterStateProvider zkClientClusterStateProvider() {
return new ZkClientClusterStateProvider(cluster.getZkStateReader());
}
+ private final Supplier<ClusterStateProvider> cspSupplier;
+
+ public ClusterStateProviderTest(String method) throws Exception {
+ this.cspSupplier =
+ () -> {
+ try {
+ return (ClusterStateProvider)
getClass().getDeclaredMethod(method).invoke(getClass());
+ } catch (IllegalAccessException | InvocationTargetException |
NoSuchMethodException e) {
+ throw new RuntimeException(e);
+ }
+ };
+
+ cluster.deleteAllCollections();
+ }
+
+ private ClusterStateProvider createClusterStateProvider() {
+ return cspSupplier.get();
+ }
+
@Test
public void testGetClusterState() throws Exception {
@@ -117,4 +150,39 @@ public class ClusterStateProviderTest extends
SolrCloudTestCase {
Map<String, Object> collection = (Map<String, Object>)
collections.get(collectionName);
return Instant.ofEpochMilli((long) collection.get("creationTimeMillis"));
}
+
+ @Test
+ public void testClusterStateProvider() throws SolrServerException,
IOException {
+ // we'll test equivalency of the two cluster state providers
+
+ CollectionAdminRequest.setClusterProperty("ext.foo",
"bar").process(cluster.getSolrClient());
+
+ createCollection("col1");
+ createCollection("col2");
+
+ try (var cspZk = zkClientClusterStateProvider();
+ var cspHttp = http2ClusterStateProvider()) {
+
+ assertThat(cspZk.getClusterProperties(), Matchers.hasEntry("ext.foo",
"bar"));
+ assertThat(
+ cspZk.getClusterProperties().entrySet(),
+
containsInAnyOrder(cspHttp.getClusterProperties().entrySet().toArray()));
+
+ 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().collect(Collectors.toList()),
+ containsInAnyOrder(clusterStateHttp.collectionStream().toArray()));
+
+ assertThat(
+ clusterStateZk.getCollection("col2"),
equalTo(clusterStateHttp.getCollection("col2")));
+ }
+ }
}
