diff --git a/solr/core/src/java/org/apache/solr/core/CoreContainer.java b/solr/core/src/java/org/apache/solr/core/CoreContainer.java index 17f1cdb37fbc..f23cd920106f 100644 --- a/solr/core/src/java/org/apache/solr/core/CoreContainer.java +++ b/solr/core/src/java/org/apache/solr/core/CoreContainer.java @@ -1806,7 +1806,11 @@ public boolean repairCoreProperty(CoreDescriptor cd, String prop) { return false; } - public void checkTragicException(SolrCore solrCore) { + /** + * @param solrCore + * @return whether this solr core has tragic exception + */ + public boolean checkTragicException(SolrCore solrCore) { Throwable tragicException = null; try { tragicException = solrCore.getSolrCoreState().getTragicException(); @@ -1820,6 +1824,8 @@ public void checkTragicException(SolrCore solrCore) { getZkController().giveupLeadership(solrCore.getCoreDescriptor(), tragicException); } } + + return tragicException != null; } } diff --git a/solr/core/src/java/org/apache/solr/handler/RequestHandlerBase.java b/solr/core/src/java/org/apache/solr/handler/RequestHandlerBase.java index af8d3be70626..a398eb73d3e8 100644 --- a/solr/core/src/java/org/apache/solr/handler/RequestHandlerBase.java +++ b/solr/core/src/java/org/apache/solr/handler/RequestHandlerBase.java @@ -209,7 +209,16 @@ public void handleRequest(SolrQueryRequest req, SolrQueryResponse rsp) { } } catch (Exception e) { if (req.getCore() != null) { - req.getCore().getCoreContainer().checkTragicException(req.getCore()); + boolean isTragic = req.getCore().getCoreContainer().checkTragicException(req.getCore()); + if (isTragic) { + if (e instanceof SolrException) { + // Tragic exceptions should always throw a server error + assert ((SolrException) e).code() == 500; + } else { + // wrap it in a solr exception + e = new SolrException(SolrException.ErrorCode.SERVER_ERROR, e.getMessage(), e); + } + } } boolean incrementErrors = true; boolean isServerError = true; diff --git a/solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java b/solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java index c91350916bea..ca39ea2815fb 100644 --- a/solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java +++ b/solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java @@ -26,6 +26,8 @@ import java.util.concurrent.atomic.LongAdder; import com.codahale.metrics.Meter; +import com.google.common.annotations.VisibleForTesting; + import org.apache.lucene.document.Document; import org.apache.lucene.document.Field; import org.apache.lucene.index.CodecReader; @@ -41,6 +43,7 @@ import org.apache.lucene.search.MatchAllDocsQuery; import org.apache.lucene.search.Query; import org.apache.lucene.search.TermQuery; +import org.apache.lucene.store.AlreadyClosedException; import org.apache.lucene.util.BytesRefHash; import org.apache.solr.cloud.ZkController; import org.apache.solr.common.SolrException; @@ -233,6 +236,9 @@ public int addDoc(AddUpdateCommand cmd) throws IOException { return addDoc0(cmd); } catch (SolrException e) { throw e; + } catch (AlreadyClosedException e) { + throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, + String.format(Locale.ROOT, "Server error writing document id %s to the index", cmd.getPrintableId()), e); } catch (IllegalArgumentException iae) { throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, String.format(Locale.ROOT, "Exception writing document id %s to the index; possible analysis error: " @@ -253,7 +259,8 @@ public int addDoc(AddUpdateCommand cmd) throws IOException { * @param cmd the command. * @return the count. */ - private int addDoc0(AddUpdateCommand cmd) throws IOException { + @VisibleForTesting + int addDoc0(AddUpdateCommand cmd) throws IOException { int rc = -1; addCommands.increment(); diff --git a/solr/core/src/test/org/apache/solr/cloud/LeaderTragicEventTest.java b/solr/core/src/test/org/apache/solr/cloud/LeaderTragicEventTest.java index 747555bbbb36..e43249330a09 100644 --- a/solr/core/src/test/org/apache/solr/cloud/LeaderTragicEventTest.java +++ b/solr/core/src/test/org/apache/solr/cloud/LeaderTragicEventTest.java @@ -17,18 +17,23 @@ package org.apache.solr.cloud; +import static org.hamcrest.CoreMatchers.anyOf; +import static org.hamcrest.CoreMatchers.is; + import java.io.FileNotFoundException; import java.io.IOException; import java.lang.invoke.MethodHandles; import java.util.ArrayList; import java.util.Collections; import java.util.List; - +import org.apache.lucene.store.AlreadyClosedException; import org.apache.lucene.store.MockDirectoryWrapper; import org.apache.solr.client.solrj.embedded.JettySolrRunner; import org.apache.solr.client.solrj.impl.HttpSolrClient; +import org.apache.solr.client.solrj.impl.HttpSolrClient.RemoteSolrException; 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.ClusterStateUtil; import org.apache.solr.common.cloud.DocCollection; import org.apache.solr.common.cloud.Replica; @@ -133,8 +138,17 @@ private Replica corruptLeader(String collection, List addedIds) throws I } } } catch (Exception e) { - log.info("Corrupt leader ex: ",e); - // Expected + log.info("Corrupt leader ex: ", e); + + // solrClient.add/commit would throw RemoteSolrException with error code 500 or + // 404(when the leader replica is already deleted by giveupLeadership) + if (e instanceof RemoteSolrException) { + SolrException se = (SolrException) e; + assertThat(se.code(), anyOf(is(500), is(404))); + } else if (!(e instanceof AlreadyClosedException)) { + throw new RuntimeException("Unexpected exception", e); + } + //else expected } return oldLeader; }