lucene-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From da...@apache.org
Subject [35/36] lucene-solr:jira/http2: SOLR-12477: Return server error(500) for AlreadyClosedException instead of client Errors(400) . This closes PR #402
Date Tue, 31 Jul 2018 02:32:51 GMT
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/jira/http2
Commit: 8d28bbc905a1d79503c40314f1223de787937c32
Parents: ea22106
Author: Varun Thacker <varun@apache.org>
Authored: Mon Jul 30 17:41:27 2018 -0700
Committer: Varun Thacker <varun@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;
   }


Mime
View raw message