You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by va...@apache.org on 2018/07/31 01:12:56 UTC
lucene-solr:master: SOLR-12477: Return server error(500) for
AlreadyClosedException instead of client Errors(400) . This closes PR #402
Repository: lucene-solr
Updated Branches:
refs/heads/master ea221069c -> 8d28bbc90
SOLR-12477: Return server error(500) for AlreadyClosedException instead of client Errors(400) . This closes PR #402
Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/8d28bbc9
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/8d28bbc9
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/8d28bbc9
Branch: refs/heads/master
Commit: 8d28bbc905a1d79503c40314f1223de787937c32
Parents: ea22106
Author: Varun Thacker <va...@apache.org>
Authored: Mon Jul 30 17:41:27 2018 -0700
Committer: Varun Thacker <va...@apache.org>
Committed: Mon Jul 30 17:42:03 2018 -0700
----------------------------------------------------------------------
solr/CHANGES.txt | 3 +++
.../org/apache/solr/core/CoreContainer.java | 10 ++++++++--
.../apache/solr/handler/RequestHandlerBase.java | 11 ++++++++++-
.../solr/update/DirectUpdateHandler2.java | 4 ++++
.../solr/cloud/LeaderTragicEventTest.java | 20 +++++++++++++++++---
5 files changed, 42 insertions(+), 6 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/8d28bbc9/solr/CHANGES.txt
----------------------------------------------------------------------
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index bf0ab5d..89e6938 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -181,6 +181,9 @@ Bug Fixes
* SOLR-12597: Migrate API should fail requests that do not specify split.key parameter (shalin)
+* SOLR-12477: An update would return a client error(400) if it hit a AlreadyClosedException.
+ We now return the error as a server error(500) instead (Jeffery via Varun Thacker)
+
Optimizations
----------------------
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/8d28bbc9/solr/core/src/java/org/apache/solr/core/CoreContainer.java
----------------------------------------------------------------------
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 17f1cdb..8659e04 100644
--- a/solr/core/src/java/org/apache/solr/core/CoreContainer.java
+++ b/solr/core/src/java/org/apache/solr/core/CoreContainer.java
@@ -1806,8 +1806,12 @@ public class CoreContainer {
return false;
}
- public void checkTragicException(SolrCore solrCore) {
- Throwable tragicException = null;
+ /**
+ * @param solrCore te core against which we check if there has been a tragic exception
+ * @return whether this solr core has tragic exception
+ */
+ public boolean checkTragicException(SolrCore solrCore) {
+ Throwable tragicException;
try {
tragicException = solrCore.getSolrCoreState().getTragicException();
} catch (IOException e) {
@@ -1820,6 +1824,8 @@ public class CoreContainer {
getZkController().giveupLeadership(solrCore.getCoreDescriptor(), tragicException);
}
}
+
+ return tragicException != null;
}
}
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/8d28bbc9/solr/core/src/java/org/apache/solr/handler/RequestHandlerBase.java
----------------------------------------------------------------------
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 af8d3be..a398eb7 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 abstract class RequestHandlerBase implements SolrRequestHandler, SolrInfo
}
} 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;
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/8d28bbc9/solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java
----------------------------------------------------------------------
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 c913509..bcc97eb 100644
--- a/solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java
+++ b/solr/core/src/java/org/apache/solr/update/DirectUpdateHandler2.java
@@ -41,6 +41,7 @@ import org.apache.lucene.search.BooleanQuery;
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 +234,9 @@ public class DirectUpdateHandler2 extends UpdateHandler implements SolrCoreState
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: "
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/8d28bbc9/solr/core/src/test/org/apache/solr/cloud/LeaderTragicEventTest.java
----------------------------------------------------------------------
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 747555b..e432493 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 @@ public class LeaderTragicEventTest extends SolrCloudTestCase {
}
}
} 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;
}