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
commit c9f8e74f0863a7e770b8622c24b7b138c09f1f30 Author: pjmcarthur <[email protected]> AuthorDate: Wed Apr 10 14:43:41 2024 +0200 SOLR-17206: Don't respond with status=-1 (#2368) It is possible for a response status code of -1 to be used for an update request that is distributed, and when all distributed requests fail with an exception that is not a SolrException (e.g. IOException) Co-authored-by: Paul McArthur <[email protected]> (cherry picked from commit 4d57b44dbc161d1ea9d3ee38e5099ea59ee2dc93) --- solr/CHANGES.txt | 5 ++++- .../processor/DistributedUpdateProcessor.java | 4 ++-- .../processor/DistributedUpdateProcessorTest.java | 22 ++++++++++++++++++++++ 3 files changed, 28 insertions(+), 3 deletions(-) diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index b45f346ec38..a9c5766322e 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -68,7 +68,7 @@ Bug Fixes * SOLR-17198: AffinityPlacementFactory can fail if Shard leadership changes occur while it is collecting metrics. (Paul McArthur) - + * SOLR-17018: Add QueryLimits support to Learning To Rank rescoring. (Alessandro Benedetti) @@ -85,6 +85,9 @@ Bug Fixes * SOLR-17218: Fix indexFetcher logging to include MDC details (hossman) +* SOLR-17206: Eliminate the possibility of a -1 status code for SolrCloud update requests that are distributed. + (Paul McArthur) + Dependency Upgrades --------------------- diff --git a/solr/core/src/java/org/apache/solr/update/processor/DistributedUpdateProcessor.java b/solr/core/src/java/org/apache/solr/update/processor/DistributedUpdateProcessor.java index 48d637d3a39..8588a0a34de 100644 --- a/solr/core/src/java/org/apache/solr/update/processor/DistributedUpdateProcessor.java +++ b/solr/core/src/java/org/apache/solr/update/processor/DistributedUpdateProcessor.java @@ -1289,8 +1289,8 @@ public class DistributedUpdateProcessor extends UpdateRequestProcessor { maxCode = Math.max(error.statusCode, maxCode); } if (minCode == maxCode) { - // all codes are consistent, use that... - return minCode; + // all codes are consistent, use that, as long as it's not the default value of -1 + return minCode != -1 ? minCode : ErrorCode.SERVER_ERROR.code; } else if (400 <= minCode && maxCode < 500) { // all codes are 4xx, use 400 return ErrorCode.BAD_REQUEST.code; diff --git a/solr/core/src/test/org/apache/solr/update/processor/DistributedUpdateProcessorTest.java b/solr/core/src/test/org/apache/solr/update/processor/DistributedUpdateProcessorTest.java index a1289db36a7..dc430437121 100644 --- a/solr/core/src/test/org/apache/solr/update/processor/DistributedUpdateProcessorTest.java +++ b/solr/core/src/test/org/apache/solr/update/processor/DistributedUpdateProcessorTest.java @@ -24,6 +24,7 @@ import static org.mockito.Mockito.doReturn; import java.io.IOException; import java.util.ArrayList; import java.util.Collection; +import java.util.List; import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; @@ -37,6 +38,7 @@ import org.apache.solr.request.LocalSolrQueryRequest; import org.apache.solr.request.SolrQueryRequest; import org.apache.solr.update.AddUpdateCommand; import org.apache.solr.update.DeleteUpdateCommand; +import org.apache.solr.update.SolrCmdDistributor; import org.apache.solr.update.TimedVersionBucket; import org.apache.solr.update.UpdateLog; import org.apache.solr.update.VersionInfo; @@ -137,6 +139,26 @@ public class DistributedUpdateProcessorTest extends SolrTestCaseJ4 { assertThat(succeeded, is(threads)); } + @Test + public void testStatusCodeOnDistribError_NotSolrException() { + + // SolrCmdDistributor defaults to a status code of -1, and sets it to a legal value only if + // the distributed exception is a SolrException instance. If it isn't it remains as -1, + // which should be replaced with a 500 + final String message = "some communication issue"; + SolrCmdDistributor.SolrError e = new SolrCmdDistributor.SolrError(); + e.e = new IOException(message); + + DistributedUpdateProcessor.DistributedUpdatesAsyncException distribError = + new DistributedUpdateProcessor.DistributedUpdatesAsyncException(List.of(e)); + assertEquals( + "Expected HTTP 500 status code for distributed update IOException", + 500, + distribError.code()); + assertEquals( + "Async exception during distributed update: " + message, distribError.getMessage()); + } + /** * @return how many requests succeeded */
