From a592f5f3efc6be957ca8ca73a7ecabcac62b8f65 Mon Sep 17 00:00:00 2001 From: aparnasuresh85 Date: Tue, 28 Nov 2023 16:46:22 -0500 Subject: [PATCH 1/4] If all shards fail, fail the request despite shards.tolerant --- .../solr/handler/component/SearchHandler.java | 20 ++++++++-- .../org/apache/solr/TestTolerantSearch.java | 40 +++++++++++++++++++ 2 files changed, 57 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 6f831f67714..2c7f9924436 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 @@ -587,9 +587,23 @@ public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp) throw SolrException.ErrorCode.SERVER_ERROR, srsp.getException()); } } else { - rsp.getResponseHeader() - .asShallowMap() - .put(SolrQueryResponse.RESPONSE_HEADER_PARTIAL_RESULTS_KEY, Boolean.TRUE); + // Check if the purpose includes 'PURPOSE_GET_TOP_IDS' + boolean includesTopIdsPurpose = + (srsp.getShardRequest().purpose & ShardRequest.PURPOSE_GET_TOP_IDS) != 0; + // Check if all responses have exceptions + boolean allResponsesHaveExceptions = + srsp.getShardRequest().responses.stream() + .allMatch(response -> response.getException() != null); + // Check if all shards have failed for PURPOSE_GET_TOP_IDS + boolean allShardsFailed = includesTopIdsPurpose && allResponsesHaveExceptions; + // if all shards fail, fail the request despite shards.tolerant + if (allShardsFailed) { + throw (SolrException) srsp.getException(); + } else { + rsp.getResponseHeader() + .asShallowMap() + .put(SolrQueryResponse.RESPONSE_HEADER_PARTIAL_RESULTS_KEY, Boolean.TRUE); + } } } diff --git a/solr/core/src/test/org/apache/solr/TestTolerantSearch.java b/solr/core/src/test/org/apache/solr/TestTolerantSearch.java index 29658475281..fe4887aebea 100644 --- a/solr/core/src/test/org/apache/solr/TestTolerantSearch.java +++ b/solr/core/src/test/org/apache/solr/TestTolerantSearch.java @@ -117,6 +117,7 @@ public static void destroyThings() throws Exception { public void testGetFieldsPhaseError() throws SolrServerException, IOException { BadResponseWriter.failOnGetFields = true; BadResponseWriter.failOnGetTopIds = false; + BadResponseWriter.failAllShards = false; SolrQuery query = new SolrQuery(); query.setQuery("subject:batman OR subject:superman"); query.addField("id"); @@ -166,6 +167,7 @@ public void testGetFieldsPhaseError() throws SolrServerException, IOException { public void testGetTopIdsPhaseError() throws SolrServerException, IOException { BadResponseWriter.failOnGetTopIds = true; BadResponseWriter.failOnGetFields = false; + BadResponseWriter.failAllShards = false; SolrQuery query = new SolrQuery(); query.setQuery("subject:batman OR subject:superman"); query.addField("id"); @@ -212,10 +214,44 @@ public void testGetTopIdsPhaseError() throws SolrServerException, IOException { unIgnoreException("Dummy exception in BadResponseWriter"); } + @SuppressWarnings("unchecked") + public void testAllShardsFail() throws SolrServerException, IOException { + BadResponseWriter.failOnGetTopIds = false; + BadResponseWriter.failOnGetFields = false; + BadResponseWriter.failAllShards = true; + SolrQuery query = new SolrQuery(); + query.setQuery("subject:batman OR subject:superman"); + query.addField("id"); + query.addField("subject"); + query.set("distrib", "true"); + query.set("shards", shard1 + "," + shard2); + query.set(ShardParams.SHARDS_INFO, "true"); + query.set("debug", "true"); + query.set("stats", "true"); + query.set("stats.field", "id"); + query.set("mlt", "true"); + query.set("mlt.fl", "title"); + query.set("mlt.count", "1"); + query.set("mlt.mintf", "0"); + query.set("mlt.mindf", "0"); + query.setHighlight(true); + query.addFacetField("id"); + query.setFacet(true); + + ignoreException("Dummy exception in BadResponseWriter"); + + expectThrows(SolrException.class, () -> collection1.query(query)); + + query.set(ShardParams.SHARDS_TOLERANT, "true"); + + expectThrows(SolrException.class, () -> collection1.query(query)); + } + public static class BadResponseWriter extends BinaryResponseWriter { private static boolean failOnGetFields = false; private static boolean failOnGetTopIds = false; + private static boolean failAllShards = false; public BadResponseWriter() { super(); @@ -240,6 +276,10 @@ public void write(OutputStream out, SolrQueryRequest req, SolrQueryResponse resp && req.getParams().getBool("isShard", false) == true) { throw new SolrException( SolrException.ErrorCode.SERVER_ERROR, "Dummy exception in BadResponseWriter"); + } else if (failAllShards) { + // fail on every shard + throw new SolrException( + SolrException.ErrorCode.SERVER_ERROR, "Dummy exception in BadResponseWriter"); } super.write(out, req, response); } From 130aa7ba9069c4bc5f29d67af740e0971f310890 Mon Sep 17 00:00:00 2001 From: aparnasuresh85 Date: Wed, 29 Nov 2023 10:36:53 -0500 Subject: [PATCH 2/4] SOLR-17053: add to CHANGES.txt --- solr/CHANGES.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 68fea8ec086..3b1119eaa94 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -87,6 +87,8 @@ New Features Improvements --------------------- +* SOLR-17053: Distributed search with shards.tolerant: if all shards fail, fail the request + * SOLR-16924: RESTORECORE now sets the UpdateLog to ACTIVE state instead of requiring a separate REQUESTAPPLYUPDATES call in Collection restore. (Julia Lamoine, David Smiley) From c5fa7885d36d2c3bc430aacd746478afe8b81e29 Mon Sep 17 00:00:00 2001 From: aparnasuresh85 Date: Wed, 29 Nov 2023 10:40:22 -0500 Subject: [PATCH 3/4] CHANGES.txt: add author name --- solr/CHANGES.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 3b1119eaa94..f860d51b031 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -87,7 +87,7 @@ New Features Improvements --------------------- -* SOLR-17053: Distributed search with shards.tolerant: if all shards fail, fail the request +* SOLR-17053: Distributed search with shards.tolerant: if all shards fail, fail the request (Aparna Suresh) * SOLR-16924: RESTORECORE now sets the UpdateLog to ACTIVE state instead of requiring a separate REQUESTAPPLYUPDATES call in Collection restore. (Julia Lamoine, David Smiley) From b671db329a54207edd44058192a8d685289d6fea Mon Sep 17 00:00:00 2001 From: aparnasuresh85 Date: Wed, 29 Nov 2023 10:59:15 -0500 Subject: [PATCH 4/4] CHANGES.txt: Add author, reviewer --- solr/CHANGES.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index f860d51b031..bea6f83fa18 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -87,7 +87,7 @@ New Features Improvements --------------------- -* SOLR-17053: Distributed search with shards.tolerant: if all shards fail, fail the request (Aparna Suresh) +* SOLR-17053: Distributed search with shards.tolerant: if all shards fail, fail the request (Aparna Suresh via David Smiley) * SOLR-16924: RESTORECORE now sets the UpdateLog to ACTIVE state instead of requiring a separate REQUESTAPPLYUPDATES call in Collection restore. (Julia Lamoine, David Smiley)