SOLR-14985: Slow indexing and search performance when using HttpClusterStateProvider
This commit fixes BaseCloudSolrClient to cache collection states returned by HttpClusterStateProvider, reduce the number of calls to getClusterProperty in case of admin requests, replace usage of getClusterStateProvider().getState() with getDocCollection() which caches the collection state. Therefore the number of clusterstatus calls are reduced from 4 for each query/indexing to either one at max and usually 0 (if data is cached already).
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 53aff7c..57d40e3 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -199,6 +199,8 @@
* SOLR-14983: Fix response returning original score instead of reranked score due to query and filter combining.
(Krishan Goyal, Jason Baik, Christine Poerschke)
+* SOLR-14985: Slow indexing and search performance when using HttpClusterStateProvider. (shalin)
+
Other Changes
---------------------
diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/BaseCloudSolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/BaseCloudSolrClient.java
index ef96be7..106f787 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/BaseCloudSolrClient.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/BaseCloudSolrClient.java
@@ -497,9 +497,9 @@
throw new SolrServerException("No collection param specified on request and no default collection has been set.");
}
- //Check to see if the collection is an alias. Updates to multi-collection aliases are ok as long
+ // Check to see if the collection is an alias. Updates to multi-collection aliases are ok as long
// as they are routed aliases
- List<String> aliasedCollections = getClusterStateProvider().resolveAlias(collection);
+ List<String> aliasedCollections = new ArrayList<>(resolveAliases(Collections.singletonList(collection), false));
if (getClusterStateProvider().isRoutedAlias(collection) || aliasedCollections.size() == 1) {
collection = aliasedCollections.get(0); // pick 1st (consistent with HttpSolrCall behavior)
} else {
@@ -1098,9 +1098,10 @@
}
} else if (ADMIN_PATHS.contains(request.getPath())) {
+ // fetch scheme outside the loop because it can be expensive in case of HttpClusterStateProvider
+ String urlScheme = getClusterStateProvider().getClusterProperty(ZkStateReader.URL_SCHEME, "http");
for (String liveNode : liveNodes) {
- theUrlList.add(Utils.getBaseUrlForNodeName(liveNode,
- getClusterStateProvider().getClusterProperty(ZkStateReader.URL_SCHEME,"http")));
+ theUrlList.add(Utils.getBaseUrlForNodeName(liveNode, urlScheme));
}
} else { // Typical...
@@ -1178,7 +1179,7 @@
}
LinkedHashSet<String> uniqueNames = new LinkedHashSet<>(); // consistent ordering
for (String collectionName : inputCollections) {
- if (getClusterStateProvider().getState(collectionName) == null) {
+ if (getDocCollection(collectionName, -1) == null) {
// perhaps it's an alias
uniqueNames.addAll(getClusterStateProvider().resolveAlias(collectionName));
} else {
@@ -1226,7 +1227,7 @@
//no such collection exists
return null;
}
- if (!ref.isLazilyLoaded()) {
+ if (getClusterStateProvider() instanceof ZkClientClusterStateProvider && !ref.isLazilyLoaded()) {
//it is readily available just return it
return ref.get();
}
diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CountingHttpClusterStateProvider.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CountingHttpClusterStateProvider.java
new file mode 100644
index 0000000..13fb3bf
--- /dev/null
+++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CountingHttpClusterStateProvider.java
@@ -0,0 +1,72 @@
+/*
+ * 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.solrj.impl;
+
+import org.apache.http.client.HttpClient;
+import org.apache.solr.client.solrj.ResponseParser;
+import org.apache.solr.client.solrj.SolrClient;
+import org.apache.solr.client.solrj.SolrRequest;
+import org.apache.solr.client.solrj.SolrServerException;
+import org.apache.solr.common.util.NamedList;
+
+import java.io.IOException;
+import java.util.List;
+import java.util.concurrent.atomic.AtomicInteger;
+
+@SuppressWarnings({"unchecked"})
+public class CountingHttpClusterStateProvider extends BaseHttpClusterStateProvider {
+
+ private final HttpClient httpClient;
+ private final boolean clientIsInternal;
+
+ private final AtomicInteger counter = new AtomicInteger(0);
+
+ public CountingHttpClusterStateProvider(List<String> solrUrls, HttpClient httpClient) throws Exception {
+ this.httpClient = httpClient == null ? HttpClientUtil.createClient(null) : httpClient;
+ this.clientIsInternal = httpClient == null;
+ init(solrUrls);
+ }
+
+ @Override
+ protected SolrClient getSolrClient(String baseUrl) {
+ return new AssertingHttpSolrClient(new HttpSolrClient.Builder().withBaseSolrUrl(baseUrl).withHttpClient(httpClient));
+ }
+
+ @Override
+ public void close() throws IOException {
+ if (this.clientIsInternal && this.httpClient != null) {
+ HttpClientUtil.close(httpClient);
+ }
+ }
+
+ public int getRequestCount() {
+ return counter.get();
+ }
+
+ class AssertingHttpSolrClient extends HttpSolrClient {
+ public AssertingHttpSolrClient(Builder builder) {
+ super(builder);
+ }
+
+ @Override
+ public NamedList<Object> request(@SuppressWarnings({"rawtypes"}) SolrRequest request, ResponseParser processor, String collection) throws SolrServerException, IOException {
+ new Exception().printStackTrace();
+ counter.incrementAndGet();
+ return super.request(request, processor, collection);
+ }
+ }
+}
diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpClusterStateProviderTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpClusterStateProviderTest.java
new file mode 100644
index 0000000..182b3ab
--- /dev/null
+++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/HttpClusterStateProviderTest.java
@@ -0,0 +1,95 @@
+/*
+ * 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.solrj.impl;
+
+import org.apache.solr.client.solrj.SolrQuery;
+import org.apache.solr.client.solrj.request.CollectionAdminRequest;
+import org.apache.solr.client.solrj.response.CollectionAdminResponse;
+import org.apache.solr.client.solrj.response.QueryResponse;
+import org.apache.solr.cloud.SolrCloudTestCase;
+import org.apache.solr.common.SolrInputDocument;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.lang.invoke.MethodHandles;
+import java.util.ArrayList;
+import java.util.List;
+
+public class HttpClusterStateProviderTest extends SolrCloudTestCase {
+ private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+ private static final int NODE_COUNT = 1;
+
+ private CloudSolrClient cloudSolrClient;
+ private CountingHttpClusterStateProvider httpClusterStateProvider;
+
+ @Before
+ public void setupCluster() throws Exception {
+ configureCluster(NODE_COUNT)
+ .addConfig("conf", getFile("solrj").toPath().resolve("solr").resolve("configsets").resolve("streaming").resolve("conf"))
+ .configure();
+
+ final List<String> solrUrls = new ArrayList<>();
+ solrUrls.add(cluster.getJettySolrRunner(0).getBaseUrl().toString());
+
+ httpClusterStateProvider = new CountingHttpClusterStateProvider(solrUrls, null);
+ cloudSolrClient = new CloudSolrClient(new CloudSolrClient.Builder(httpClusterStateProvider));
+ CollectionAdminRequest.Create create = CollectionAdminRequest.Create.createCollection("x", 1, 1);
+ CollectionAdminResponse adminResponse = create.process(cloudSolrClient);
+ assertTrue(adminResponse.isSuccess());
+ cloudSolrClient.setDefaultCollection("x");
+ }
+
+ @After
+ public void tearDown() throws Exception {
+ cloudSolrClient.close();
+ httpClusterStateProvider.close();
+ super.tearDown();
+ }
+
+ @Test
+ public void test() throws Exception {
+ // the constructor of HttpClusterStateProvider fetches live nodes
+ // and creating a collection fetches the cluster state
+ // so we can expect exactly 2 http calls to have been made already at this point
+ assertEquals(2, httpClusterStateProvider.getRequestCount());
+
+ QueryResponse queryResponse = cloudSolrClient.query(new SolrQuery("*:*"));
+ assertEquals(0, queryResponse.getResults().getNumFound());
+ // we can expect 1 extra call to fetch and cache the collection state
+ assertEquals(3, httpClusterStateProvider.getRequestCount());
+ queryResponse = cloudSolrClient.query(new SolrQuery("*:*"));
+ assertEquals(0, queryResponse.getResults().getNumFound());
+ // the collection state should already be in the cache so we do not expect another call
+ assertEquals(3, httpClusterStateProvider.getRequestCount());
+
+ cloudSolrClient.add(new SolrInputDocument("id", "a"));
+ // we can expect another call to check if the collection is a routed alias
+ assertEquals(4, httpClusterStateProvider.getRequestCount());
+ cloudSolrClient.add(List.of(new SolrInputDocument("id", "b"), new SolrInputDocument("id", "c")));
+ assertEquals(4, httpClusterStateProvider.getRequestCount());
+ cloudSolrClient.commit();
+ assertEquals(4, httpClusterStateProvider.getRequestCount());
+
+ queryResponse = cloudSolrClient.query(new SolrQuery("*:*"));
+ assertEquals(3, queryResponse.getResults().getNumFound());
+ // the collection state should already be in the cache so we do not expect another call
+ assertEquals(4, httpClusterStateProvider.getRequestCount());
+ }
+}
