From 1c9ef2d98711a6cb21f869f04fe2cb255ab2321b Mon Sep 17 00:00:00 2001 From: Justin Sweeney Date: Wed, 6 Sep 2023 14:19:40 -0400 Subject: [PATCH 1/3] Handling BAD_REQUEST exceptions as failures even with shard.tolerant as true --- .../solr/handler/component/SearchHandler.java | 9 ++++++++- .../solr/handler/component/ShardResponse.java | 4 ++++ .../solr/handler/component/SearchHandlerTest.java | 13 +++++++++++++ 3 files changed, 25 insertions(+), 1 deletion(-) diff --git a/solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java b/solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java index 89316672cba..212d421cbf0 100644 --- a/solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java +++ b/solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java @@ -38,6 +38,7 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicLong; import java.util.stream.StreamSupport; +import org.apache.commons.lang3.ArrayUtils; import org.apache.lucene.index.ExitableDirectoryReader; import org.apache.lucene.queryparser.surround.query.TooManyBasicQueries; import org.apache.lucene.search.IndexSearcher; @@ -92,6 +93,9 @@ public class SearchHandler extends RequestHandlerBase protected static final String SHARD_HANDLER_SUFFIX = "[shard]"; + private static final SolrException.ErrorCode[] NONTOLERANT_ERROR_CODES = + new SolrException.ErrorCode[] {SolrException.ErrorCode.BAD_REQUEST}; + private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); /** A counter to ensure that no RID is equal, even if they fall in the same millisecond */ @@ -572,7 +576,10 @@ public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp) throw if (srsp.getException() != null) { log.warn("Shard request failed : {}", srsp); // If things are not tolerant, abort everything and rethrow - if (!tolerant) { + if (!tolerant + || ArrayUtils.contains( + NONTOLERANT_ERROR_CODES, + SolrException.ErrorCode.getErrorCode(srsp.getRspCode()))) { shardHandler1.cancelAll(); if (srsp.getException() instanceof SolrException) { throw (SolrException) srsp.getException(); diff --git a/solr/core/src/java/org/apache/solr/handler/component/ShardResponse.java b/solr/core/src/java/org/apache/solr/handler/component/ShardResponse.java index dee5fcdb803..9cc33328134 100644 --- a/solr/core/src/java/org/apache/solr/handler/component/ShardResponse.java +++ b/solr/core/src/java/org/apache/solr/handler/component/ShardResponse.java @@ -81,6 +81,10 @@ void setResponseCode(int rspCode) { this.rspCode = rspCode; } + public int getRspCode() { + return rspCode; + } + void setNodeName(String nodeName) { this.nodeName = nodeName; } diff --git a/solr/core/src/test/org/apache/solr/handler/component/SearchHandlerTest.java b/solr/core/src/test/org/apache/solr/handler/component/SearchHandlerTest.java index da861b79d99..c7bce23e0ac 100644 --- a/solr/core/src/test/org/apache/solr/handler/component/SearchHandlerTest.java +++ b/solr/core/src/test/org/apache/solr/handler/component/SearchHandlerTest.java @@ -360,6 +360,19 @@ public void testLuceneIOExceptionHandling() throws Exception { assertTrue( e.getMessage() .contains("Query contains too many nested clauses; maxClauseCount is set to 1")); + + solrQuery = new SolrQuery("{!surround maxBasicQueries=1000 df=title}10W(tes*,test*)"); + solrQuery.add(ShardParams.SHARDS_TOLERANT, "true"); + final QueryRequest req4 = new QueryRequest(solrQuery); + req4.setMethod(SolrRequest.METHOD.POST); + e = + assertThrows( + BaseHttpSolrClient.RemoteSolrException.class, + () -> req4.process(cloudSolrClient, collectionName)); + assertEquals(400, e.code()); + assertTrue( + e.getMessage() + .contains("Query contains too many nested clauses; maxClauseCount is set to 1")); } finally { if (initialMaxBooleanClauses != null) { System.setProperty("solr.max.booleanClauses", initialMaxBooleanClauses); From 2409b121f27d94588821da03d10767af8280ba3f Mon Sep 17 00:00:00 2001 From: Justin Sweeney Date: Wed, 6 Sep 2023 14:39:44 -0400 Subject: [PATCH 2/3] Fixing forbiddenapi use --- .../org/apache/solr/handler/component/SearchHandler.java | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java b/solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java index 212d421cbf0..e5827bf5b8d 100644 --- a/solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java +++ b/solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java @@ -27,6 +27,7 @@ import java.lang.invoke.MethodHandles; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collection; import java.util.Collections; import java.util.HashSet; import java.util.Iterator; @@ -38,7 +39,6 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicLong; import java.util.stream.StreamSupport; -import org.apache.commons.lang3.ArrayUtils; import org.apache.lucene.index.ExitableDirectoryReader; import org.apache.lucene.queryparser.surround.query.TooManyBasicQueries; import org.apache.lucene.search.IndexSearcher; @@ -93,8 +93,8 @@ public class SearchHandler extends RequestHandlerBase protected static final String SHARD_HANDLER_SUFFIX = "[shard]"; - private static final SolrException.ErrorCode[] NONTOLERANT_ERROR_CODES = - new SolrException.ErrorCode[] {SolrException.ErrorCode.BAD_REQUEST}; + private static final Collection NONTOLERANT_ERROR_CODES = + List.of(SolrException.ErrorCode.BAD_REQUEST); private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); @@ -577,8 +577,7 @@ public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp) throw log.warn("Shard request failed : {}", srsp); // If things are not tolerant, abort everything and rethrow if (!tolerant - || ArrayUtils.contains( - NONTOLERANT_ERROR_CODES, + || NONTOLERANT_ERROR_CODES.contains( SolrException.ErrorCode.getErrorCode(srsp.getRspCode()))) { shardHandler1.cancelAll(); if (srsp.getException() instanceof SolrException) { From 202769e619bdadbd1b7ee0224195a3d51b867644 Mon Sep 17 00:00:00 2001 From: Justin Sweeney Date: Thu, 7 Sep 2023 06:55:50 -0400 Subject: [PATCH 3/3] Updating the set of error codes to just be a Set --- .../org/apache/solr/handler/component/SearchHandler.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java b/solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java index e5827bf5b8d..e7093c44b58 100644 --- a/solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java +++ b/solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java @@ -27,7 +27,6 @@ import java.lang.invoke.MethodHandles; import java.util.ArrayList; import java.util.Arrays; -import java.util.Collection; import java.util.Collections; import java.util.HashSet; import java.util.Iterator; @@ -93,8 +92,8 @@ public class SearchHandler extends RequestHandlerBase protected static final String SHARD_HANDLER_SUFFIX = "[shard]"; - private static final Collection NONTOLERANT_ERROR_CODES = - List.of(SolrException.ErrorCode.BAD_REQUEST); + private static final Set NONTOLERANT_ERROR_CODES = + Set.of(SolrException.ErrorCode.BAD_REQUEST); private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());