lucene-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From no...@apache.org
Subject [20/50] [abbrv] lucene-solr:apiv2: SOLR-9445: Admin requests are retried by CloudSolrClient and LBHttpSolrClient on failure
Date Wed, 07 Sep 2016 09:57:08 GMT
SOLR-9445: Admin requests are retried by CloudSolrClient and LBHttpSolrClient on failure


Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/ae40929f
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/ae40929f
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/ae40929f

Branch: refs/heads/apiv2
Commit: ae40929f0b13f5cbf83b0700bab694fd7a65f660
Parents: b3d12d2
Author: Shalin Shekhar Mangar <shalin@apache.org>
Authored: Sat Aug 27 09:08:02 2016 +0530
Committer: Shalin Shekhar Mangar <shalin@apache.org>
Committed: Sat Aug 27 09:08:02 2016 +0530

----------------------------------------------------------------------
 solr/CHANGES.txt                                |  2 +
 .../solr/client/solrj/impl/CloudSolrClient.java | 13 +---
 .../client/solrj/impl/LBHttpSolrClient.java     | 20 +++---
 .../apache/solr/common/params/CommonParams.java | 10 +++
 .../client/solrj/impl/CloudSolrClientTest.java  | 68 ++++++++++++++++++--
 5 files changed, 87 insertions(+), 26 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/ae40929f/solr/CHANGES.txt
----------------------------------------------------------------------
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index b502bf0..2dee6ab 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -79,6 +79,8 @@ Bug Fixes
 * SOLR-6744: fl renaming / alias of uniqueKey field generates null pointer exception in SolrCloud
configuration
   (Mike Drob via Tomás Fernández Löbbe)
 
+* SOLR-9445: Admin requests are retried by CloudSolrClient and LBHttpSolrClient on failure.
(shalin)
+
 Optimizations
 ----------------------
 

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/ae40929f/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java
----------------------------------------------------------------------
diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java
index 1f1c675..958cf14 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/CloudSolrClient.java
@@ -22,7 +22,6 @@ import java.net.ConnectException;
 import java.net.SocketException;
 import java.nio.file.Path;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
@@ -85,11 +84,7 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.slf4j.MDC;
 
-import static org.apache.solr.common.params.CommonParams.AUTHC_PATH;
-import static org.apache.solr.common.params.CommonParams.AUTHZ_PATH;
-import static org.apache.solr.common.params.CommonParams.COLLECTIONS_HANDLER_PATH;
-import static org.apache.solr.common.params.CommonParams.CONFIGSETS_HANDLER_PATH;
-import static org.apache.solr.common.params.CommonParams.CORES_HANDLER_PATH;
+import static org.apache.solr.common.params.CommonParams.ADMIN_PATHS;
 
 /**
  * SolrJ client class to communicate with SolrCloud.
@@ -996,12 +991,6 @@ public class CloudSolrClient extends SolrClient {
       collection = (reqParams != null) ? reqParams.get("collection", getDefaultCollection())
: getDefaultCollection();
     return requestWithRetryOnStaleState(request, 0, collection);
   }
-  private static final Set<String> ADMIN_PATHS = new HashSet<>(Arrays.asList(
-      CORES_HANDLER_PATH,
-      COLLECTIONS_HANDLER_PATH,
-      CONFIGSETS_HANDLER_PATH,
-      AUTHC_PATH,
-      AUTHZ_PATH));
 
   /**
    * As this class doesn't watch external collections on the client side,

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/ae40929f/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttpSolrClient.java
----------------------------------------------------------------------
diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttpSolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttpSolrClient.java
index c65a328..74b0943 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttpSolrClient.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBHttpSolrClient.java
@@ -54,6 +54,8 @@ import org.apache.solr.common.util.NamedList;
 import org.apache.solr.common.util.SolrjNamedThreadFactory;
 import org.slf4j.MDC;
 
+import static org.apache.solr.common.params.CommonParams.ADMIN_PATHS;
+
 /**
  * LBHttpSolrClient or "LoadBalanced HttpSolrClient" is a load balancing wrapper around
  * {@link HttpSolrClient}. This is useful when you
@@ -331,7 +333,7 @@ public class LBHttpSolrClient extends SolrClient {
   public Rsp request(Req req) throws SolrServerException, IOException {
     Rsp rsp = new Rsp();
     Exception ex = null;
-    boolean isUpdate = req.request instanceof IsUpdateRequest;
+    boolean isNonRetryable = req.request instanceof IsUpdateRequest || ADMIN_PATHS.contains(req.request.getPath());
     List<ServerWrapper> skipped = null;
 
     long timeAllowedNano = getTimeAllowedInNanos(req.getRequest());
@@ -362,7 +364,7 @@ public class LBHttpSolrClient extends SolrClient {
         MDC.put("LBHttpSolrClient.url", serverStr);
         HttpSolrClient client = makeSolrClient(serverStr);
 
-        ex = doRequest(client, req, rsp, isUpdate, false, null);
+        ex = doRequest(client, req, rsp, isNonRetryable, false, null);
         if (ex == null) {
           return rsp; // SUCCESS
         }
@@ -378,7 +380,7 @@ public class LBHttpSolrClient extends SolrClient {
           break;
         }
 
-        ex = doRequest(wrapper.client, req, rsp, isUpdate, true, wrapper.getKey());
+        ex = doRequest(wrapper.client, req, rsp, isNonRetryable, true, wrapper.getKey());
         if (ex == null) {
           return rsp; // SUCCESS
         }
@@ -405,7 +407,7 @@ public class LBHttpSolrClient extends SolrClient {
     return e;
   }  
 
-  protected Exception doRequest(HttpSolrClient client, Req req, Rsp rsp, boolean isUpdate,
+  protected Exception doRequest(HttpSolrClient client, Req req, Rsp rsp, boolean isNonRetryable,
       boolean isZombie, String zombieKey) throws SolrServerException, IOException {
     Exception ex = null;
     try {
@@ -417,7 +419,7 @@ public class LBHttpSolrClient extends SolrClient {
     } catch (SolrException e) {
       // we retry on 404 or 403 or 503 or 500
       // unless it's an update - then we only retry on connect exception
-      if (!isUpdate && RETRY_CODES.contains(e.code())) {
+      if (!isNonRetryable && RETRY_CODES.contains(e.code())) {
         ex = (!isZombie) ? addZombie(client, e) : e;
       } else {
         // Server is alive but the request was likely malformed or invalid
@@ -427,22 +429,22 @@ public class LBHttpSolrClient extends SolrClient {
         throw e;
       }
     } catch (SocketException e) {
-      if (!isUpdate || e instanceof ConnectException) {
+      if (!isNonRetryable || e instanceof ConnectException) {
         ex = (!isZombie) ? addZombie(client, e) : e;
       } else {
         throw e;
       }
     } catch (SocketTimeoutException e) {
-      if (!isUpdate) {
+      if (!isNonRetryable) {
         ex = (!isZombie) ? addZombie(client, e) : e;
       } else {
         throw e;
       }
     } catch (SolrServerException e) {
       Throwable rootCause = e.getRootCause();
-      if (!isUpdate && rootCause instanceof IOException) {
+      if (!isNonRetryable && rootCause instanceof IOException) {
         ex = (!isZombie) ? addZombie(client, e) : e;
-      } else if (isUpdate && rootCause instanceof ConnectException) {
+      } else if (isNonRetryable && rootCause instanceof ConnectException) {
         ex = (!isZombie) ? addZombie(client, e) : e;
       } else {
         throw e;

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/ae40929f/solr/solrj/src/java/org/apache/solr/common/params/CommonParams.java
----------------------------------------------------------------------
diff --git a/solr/solrj/src/java/org/apache/solr/common/params/CommonParams.java b/solr/solrj/src/java/org/apache/solr/common/params/CommonParams.java
index 5ccd70f..b830b41 100644
--- a/solr/solrj/src/java/org/apache/solr/common/params/CommonParams.java
+++ b/solr/solrj/src/java/org/apache/solr/common/params/CommonParams.java
@@ -16,7 +16,10 @@
  */
 package org.apache.solr.common.params;
 
+import java.util.Arrays;
+import java.util.HashSet;
 import java.util.Locale;
+import java.util.Set;
 
 
 /**
@@ -178,6 +181,13 @@ public interface CommonParams {
   public static final String AUTHC_PATH = "/admin/authentication";
   public static final String ZK_PATH = "/admin/zookeeper";
 
+  public static final Set<String> ADMIN_PATHS = new HashSet<>(Arrays.asList(
+      CORES_HANDLER_PATH,
+      COLLECTIONS_HANDLER_PATH,
+      CONFIGSETS_HANDLER_PATH,
+      AUTHC_PATH,
+      AUTHZ_PATH));
+
   /** valid values for: <code>echoParams</code> */
   public enum EchoParamStyle {
     EXPLICIT,

http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/ae40929f/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientTest.java
----------------------------------------------------------------------
diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientTest.java
b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientTest.java
index cf12036..5e8f6ce 100644
--- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientTest.java
+++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/CloudSolrClientTest.java
@@ -22,6 +22,7 @@ import java.net.URL;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
@@ -38,11 +39,13 @@ import org.apache.lucene.util.TestUtil;
 import org.apache.solr.client.solrj.SolrClient;
 import org.apache.solr.client.solrj.SolrQuery;
 import org.apache.solr.client.solrj.SolrServerException;
+import org.apache.solr.client.solrj.embedded.JettySolrRunner;
 import org.apache.solr.client.solrj.request.AbstractUpdateRequest;
 import org.apache.solr.client.solrj.request.CollectionAdminRequest;
 import org.apache.solr.client.solrj.request.QueryRequest;
 import org.apache.solr.client.solrj.request.UpdateRequest;
 import org.apache.solr.client.solrj.response.QueryResponse;
+import org.apache.solr.client.solrj.response.RequestStatusState;
 import org.apache.solr.client.solrj.response.UpdateResponse;
 import org.apache.solr.cloud.AbstractDistribZkTestBase;
 import org.apache.solr.cloud.SolrCloudTestCase;
@@ -60,6 +63,9 @@ import org.apache.solr.common.params.ModifiableSolrParams;
 import org.apache.solr.common.params.ShardParams;
 import org.apache.solr.common.util.NamedList;
 import org.apache.solr.common.util.SimpleOrderedMap;
+import org.apache.solr.handler.admin.CollectionsHandler;
+import org.apache.solr.handler.admin.ConfigSetsHandler;
+import org.apache.solr.handler.admin.CoreAdminHandler;
 import org.junit.Before;
 import org.junit.BeforeClass;
 import org.junit.Rule;
@@ -80,10 +86,11 @@ public class CloudSolrClientTest extends SolrCloudTestCase {
   private static final String id = "id";
 
   private static final int TIMEOUT = 30;
+  private static final int NODE_COUNT = 3;
 
   @BeforeClass
   public static void setupCluster() throws Exception {
-    configureCluster(3)
+    configureCluster(NODE_COUNT)
         .addConfig("conf", getFile("solrj").toPath().resolve("solr").resolve("configsets").resolve("streaming").resolve("conf"))
         .configure();
 
@@ -384,6 +391,11 @@ public class CloudSolrClientTest extends SolrCloudTestCase {
 
   private Long getNumRequests(String baseUrl, String collectionName) throws
       SolrServerException, IOException {
+    return getNumRequests(baseUrl, collectionName, "QUERYHANDLER", "standard", false);
+  }
+
+  private Long getNumRequests(String baseUrl, String collectionName, String category, String
key, boolean returnNumErrors) throws
+      SolrServerException, IOException {
 
     NamedList<Object> resp;
     try (HttpSolrClient client = getHttpSolrClient(baseUrl + "/"+ collectionName)) {
@@ -392,14 +404,60 @@ public class CloudSolrClientTest extends SolrCloudTestCase {
       ModifiableSolrParams params = new ModifiableSolrParams();
       params.set("qt", "/admin/mbeans");
       params.set("stats", "true");
-      params.set("key", "standard");
-      params.set("cat", "QUERYHANDLER");
+      params.set("key", key);
+      params.set("cat", category);
       // use generic request to avoid extra processing of queries
       QueryRequest req = new QueryRequest(params);
       resp = client.request(req);
     }
-    return (Long) resp.findRecursive("solr-mbeans", "QUERYHANDLER",
-        "standard", "stats", "requests");
+    return (Long) resp.findRecursive("solr-mbeans", category, key, "stats", returnNumErrors
? "errors" : "requests");
+  }
+
+  @Test
+  public void testNonRetryableRequests() throws Exception {
+    try (CloudSolrClient client = getCloudSolrClient(cluster.getZkServer().getZkAddress()))
{
+      // important to have one replica on each node
+      RequestStatusState state = CollectionAdminRequest.createCollection("foo", "conf", 1,
NODE_COUNT).processAndWait(client, 60);
+      if (state == RequestStatusState.COMPLETED) {
+        AbstractDistribZkTestBase.waitForRecoveriesToFinish("foo", client.getZkStateReader(),
true, true, TIMEOUT);
+        client.setDefaultCollection("foo");
+
+        Map<String, String> adminPathToMbean = new HashMap<>(CommonParams.ADMIN_PATHS.size());
+        adminPathToMbean.put(CommonParams.COLLECTIONS_HANDLER_PATH, CollectionsHandler.class.getName());
+        adminPathToMbean.put(CommonParams.CORES_HANDLER_PATH, CoreAdminHandler.class.getName());
+        adminPathToMbean.put(CommonParams.CONFIGSETS_HANDLER_PATH, ConfigSetsHandler.class.getName());
+        // we do not add the authc/authz handlers because they do not currently expose any
mbeans
+
+        for (String adminPath : adminPathToMbean.keySet()) {
+          long errorsBefore = 0;
+          for (JettySolrRunner runner : cluster.getJettySolrRunners()) {
+            Long numRequests = getNumRequests(runner.getBaseUrl().toString(), "foo", "QUERYHANDLER",
adminPathToMbean.get(adminPath), true);
+            errorsBefore += numRequests;
+            log.info("Found {} requests to {} on {}", numRequests, adminPath, runner.getBaseUrl());
+          }
+
+          ModifiableSolrParams params = new ModifiableSolrParams();
+          params.set("qt", adminPath);
+          params.set("action", "foobar"); // this should cause an error
+          QueryRequest req = new QueryRequest(params);
+          try {
+            NamedList<Object> resp = client.request(req);
+            fail("call to foo for admin path " + adminPath + " should have failed");
+          } catch (Exception e) {
+            // expected
+          }
+          long errorsAfter = 0;
+          for (JettySolrRunner runner : cluster.getJettySolrRunners()) {
+            Long numRequests = getNumRequests(runner.getBaseUrl().toString(), "foo", "QUERYHANDLER",
adminPathToMbean.get(adminPath), true);
+            errorsAfter += numRequests;
+            log.info("Found {} requests to {} on {}", numRequests, adminPath, runner.getBaseUrl());
+          }
+          assertEquals(errorsBefore + 1, errorsAfter);
+        }
+      } else {
+        fail("Collection could not be created within 60 seconds");
+      }
+    }
   }
 
   @Test


Mime
View raw message