You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by is...@apache.org on 2019/10/12 07:29:16 UTC
[lucene-solr] 01/01: SOLR-13793: Limiting number of forwards to
total replicas in collection to avoid deadly forwarding loops
This is an automated email from the ASF dual-hosted git repository.
ishan pushed a commit to branch jira/SOLR-13793
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git
commit 71f32a293159fbff4118c51467a8277d138e80b6
Author: Ishan Chattopadhyaya <is...@apache.org>
AuthorDate: Sat Oct 12 05:20:12 2019 +0530
SOLR-13793: Limiting number of forwards to total replicas in collection to avoid deadly forwarding loops
---
solr/CHANGES.txt | 4 +
.../java/org/apache/solr/servlet/HttpSolrCall.java | 26 ++++-
.../solr/cloud/TestQueryingOnDownCollection.java | 109 +++++++++++++++++++++
3 files changed, 136 insertions(+), 3 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 3418d17..d38cc9a 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -314,6 +314,10 @@ Bug Fixes
* SOLR-13829: RecursiveEvaluator casts Continuous numbers to Discrete Numbers, causing mismatch (Trey Grainger, Joel Bernstein)
+* SOLR-13793: HttpSolrCall now maintains internal request count (solr.internalRequestCount) for remote queries and limits
+ them to the number of replicas. This avoids making too many cascading calls to remote servers, which, if not restricted, can
+ bring down nodes containing the said collection (Kesharee Nandan Vishwakarma, Ishan Chattopadhyaya)
+
Other Changes
----------------------
diff --git a/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java b/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java
index 546a195..039f3fd 100644
--- a/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java
+++ b/solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java
@@ -139,6 +139,8 @@ public class HttpSolrCall {
public static final String ORIGINAL_USER_PRINCIPAL_HEADER = "originalUserPrincipal";
+ public static final String INTERNAL_REQUEST_COUNT = "solr.internalRequestCount";
+
public static final Random random;
static {
// We try to make things reproducible in the context of our tests by initializing the random instance
@@ -446,7 +448,7 @@ public class HttpSolrCall {
}
}
- protected void extractRemotePath(String collectionName, String origCorename) throws UnsupportedEncodingException, KeeperException, InterruptedException {
+ protected void extractRemotePath(String collectionName, String origCorename) throws UnsupportedEncodingException, KeeperException, InterruptedException, SolrException {
assert core == null;
coreUrl = getRemoteCoreUrl(collectionName, origCorename);
// don't proxy for internal update requests
@@ -644,12 +646,19 @@ public class HttpSolrCall {
}
}
+ private String getQueryStingWithInternalRequestCount(){
+ int internalRequestCount = queryParams.getInt(INTERNAL_REQUEST_COUNT, 0);
+ ModifiableSolrParams updatedQueryParams = new ModifiableSolrParams(queryParams);
+ updatedQueryParams.set(INTERNAL_REQUEST_COUNT, internalRequestCount + 1);
+ return updatedQueryParams.toQueryString();
+ }
+
//TODO using Http2Client
private void remoteQuery(String coreUrl, HttpServletResponse resp) throws IOException {
HttpRequestBase method;
HttpEntity httpEntity = null;
try {
- String urlstr = coreUrl + queryParams.toQueryString();
+ String urlstr = coreUrl + getQueryStingWithInternalRequestCount();
boolean isPostOrPutRequest = "POST".equals(req.getMethod()) || "PUT".equals(req.getMethod());
if ("GET".equals(req.getMethod())) {
@@ -963,13 +972,15 @@ public class HttpSolrCall {
}
}
- protected String getRemoteCoreUrl(String collectionName, String origCorename) {
+ protected String getRemoteCoreUrl(String collectionName, String origCorename) throws SolrException {
ClusterState clusterState = cores.getZkController().getClusterState();
final DocCollection docCollection = clusterState.getCollectionOrNull(collectionName);
Slice[] slices = (docCollection != null) ? docCollection.getActiveSlicesArr() : null;
List<Slice> activeSlices = new ArrayList<>();
boolean byCoreName = false;
+ int totalReplicas = 0;
+
if (slices == null) {
byCoreName = true;
activeSlices = new ArrayList<>();
@@ -983,6 +994,9 @@ public class HttpSolrCall {
}
}
+ for (Slice s: activeSlices) {
+ totalReplicas += s.getReplicas().size();
+ }
if (activeSlices.isEmpty()) {
return null;
}
@@ -996,7 +1010,13 @@ public class HttpSolrCall {
String coreUrl = getCoreUrl(collectionName, origCorename, clusterState,
activeSlices, byCoreName, true);
+ // Avoid getting into a recursive loop of requests being forwarded by
+ // stopping forwarding and erroring out after (totalReplicas) forwards
if (coreUrl == null) {
+ if(queryParams.getInt(INTERNAL_REQUEST_COUNT, 0) > totalReplicas){
+ throw new SolrException(SolrException.ErrorCode.INVALID_STATE,
+ "No active replicas found for collection: " + collectionName);
+ }
coreUrl = getCoreUrl(collectionName, origCorename, clusterState,
activeSlices, byCoreName, false);
}
diff --git a/solr/core/src/test/org/apache/solr/cloud/TestQueryingOnDownCollection.java b/solr/core/src/test/org/apache/solr/cloud/TestQueryingOnDownCollection.java
new file mode 100644
index 0000000..040d1ca
--- /dev/null
+++ b/solr/core/src/test/org/apache/solr/cloud/TestQueryingOnDownCollection.java
@@ -0,0 +1,109 @@
+/*
+ * 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.cloud;
+
+import java.lang.invoke.MethodHandles;
+import java.util.List;
+import java.util.Map;
+
+import org.apache.solr.client.solrj.SolrClient;
+import org.apache.solr.client.solrj.SolrQuery;
+import org.apache.solr.client.solrj.request.CollectionAdminRequest;
+import org.apache.solr.client.solrj.request.UpdateRequest;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.common.cloud.Replica;
+import org.apache.solr.common.util.Utils;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class TestQueryingOnDownCollection extends SolrCloudTestCase {
+
+ private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+ private static final String COLLECTION_NAME = "infected";
+
+ @BeforeClass
+ public static void setupCluster() throws Exception {
+ configureCluster(3)
+ .addConfig("conf", configset("cloud-minimal"))
+ .configure();
+ }
+
+ @Test
+ /**
+ * Assert that requests to "down collection", i.e. a collection which has all replicas in down state
+ * (but are hosted on nodes that are live), fail fast and throw meaningful exceptions
+ */
+ public void testQueryToDownCollectionShouldFailFast() throws Exception {
+
+ CollectionAdminRequest.createCollection(COLLECTION_NAME, "conf", 2, 1)
+ .process(cluster.getSolrClient());
+
+ // Add some dummy documents
+ UpdateRequest update = new UpdateRequest();
+ for (int i = 0; i < 100; i++) {
+ update.add("id", Integer.toString(i));
+ }
+ update.commit(cluster.getSolrClient(), COLLECTION_NAME);
+
+ // Bring down replicas but keep nodes up. This could've been done by some combinations of collections API operations;
+ // however, to make it faster, altering cluster state directly! ;-)
+ downAllReplicas();
+
+ // assert all replicas are in down state
+ List<Replica> replicas = getCollectionState(COLLECTION_NAME).getReplicas();
+ for (Replica replica: replicas){
+ assertEquals(replica.getState(), Replica.State.DOWN);
+ }
+
+ // assert all nodes as active
+ assertEquals(3, cluster.getSolrClient().getClusterStateProvider().getLiveNodes().size());
+
+ SolrClient client = cluster.getJettySolrRunner(0).newClient();
+
+ // Without the SOLR-13793 fix, this causes requests to "down collection" to pile up (until the nodes run out
+ // of serviceable threads and they crash, even for other collections hosted on the nodes).
+ SolrException error = expectThrows(SolrException.class,
+ "Request should fail after trying all replica nodes once",
+ () -> client.query(COLLECTION_NAME, new SolrQuery("*:*").setRows(0))
+ );
+
+ client.close();
+
+ assertEquals(error.code(), SolrException.ErrorCode.INVALID_STATE.code);
+ assertTrue(error.getMessage().contains("No active replicas found for collection: " + COLLECTION_NAME));
+ }
+
+ private void downAllReplicas() throws Exception {
+ byte[] collectionState = cluster.getZkClient().getData("/collections/" + COLLECTION_NAME + "/state.json",
+ null, null, true);
+
+ Map<String,Map<String,?>> infectedState = (Map<String,Map<String,?>>) Utils.fromJSON(collectionState);
+ Map<String, Object> shards = (Map<String, Object>) infectedState.get(COLLECTION_NAME).get("shards");
+ for(Map.Entry<String, Object> shard: shards.entrySet()) {
+ Map<String, Object> replicas = (Map<String, Object>) ((Map<String, Object>) shard.getValue() ).get("replicas");
+ for (Map.Entry<String, Object> replica : replicas.entrySet()) {
+ ((Map<String, Object>) replica.getValue()).put("state", Replica.State.DOWN.toString());
+ }
+ }
+
+ cluster.getZkClient().setData("/collections/" + COLLECTION_NAME + "/state.json", Utils.toJSON(infectedState)
+ , true);
+ }
+
+}