You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by sh...@apache.org on 2017/08/08 12:44:05 UTC
[18/50] [abbrv] lucene-solr:jira/solr-11165: SOLR-11178: Change error
handling in AutoScalingHandler to be consistent w/ other APIs
SOLR-11178: Change error handling in AutoScalingHandler to be consistent w/ other APIs
Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/c1d28c3e
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/c1d28c3e
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/c1d28c3e
Branch: refs/heads/jira/solr-11165
Commit: c1d28c3ece276ec7bc5376b111cb0e99042e27a0
Parents: 7dde798
Author: Noble Paul <no...@apache.org>
Authored: Fri Aug 4 17:25:11 2017 +0930
Committer: Noble Paul <no...@apache.org>
Committed: Fri Aug 4 23:41:30 2017 +0930
----------------------------------------------------------------------
solr/CHANGES.txt | 2 +
.../cloud/autoscaling/AutoScalingHandler.java | 41 +++++++++++++++-----
.../org/apache/solr/servlet/ResponseUtils.java | 10 ++---
.../autoscaling/AutoScalingHandlerTest.java | 25 +++++++++++-
.../solr/client/solrj/impl/HttpSolrClient.java | 11 +++---
.../java/org/apache/solr/common/util/Utils.java | 2 +-
6 files changed, 68 insertions(+), 23 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/c1d28c3e/solr/CHANGES.txt
----------------------------------------------------------------------
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 012ac86..b5bf725 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -616,6 +616,8 @@ Other Changes
* SOLR-10033: When attempting to facet with facet.mincount=0 over points fields, raise mincount to 1
and log a warning. (Steve Rowe)
+* SOLR-11178: Change error handling in AutoScalingHandler to be consistent w/ other APIs (noble)
+
================== 6.7.0 ==================
Consult the LUCENE_CHANGES.txt file for additional, low level, changes in this release.
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/c1d28c3e/solr/core/src/java/org/apache/solr/cloud/autoscaling/AutoScalingHandler.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/cloud/autoscaling/AutoScalingHandler.java b/solr/core/src/java/org/apache/solr/cloud/autoscaling/AutoScalingHandler.java
index 356ce37..5ed5f8e 100644
--- a/solr/core/src/java/org/apache/solr/cloud/autoscaling/AutoScalingHandler.java
+++ b/solr/core/src/java/org/apache/solr/cloud/autoscaling/AutoScalingHandler.java
@@ -21,6 +21,7 @@ import java.io.IOException;
import java.lang.invoke.MethodHandles;
import java.util.ArrayList;
import java.util.Collection;
+import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
@@ -167,16 +168,31 @@ public class AutoScalingHandler extends RequestHandlerBase implements Permission
private void handleSetClusterPolicy(SolrQueryRequest req, SolrQueryResponse rsp, CommandOperation op) throws KeeperException, InterruptedException, IOException {
List clusterPolicy = (List) op.getCommandData();
if (clusterPolicy == null || !(clusterPolicy instanceof List)) {
- throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "A list of cluster policies was not found");
+ op.addError("A list of cluster policies was not found");
+ checkErr(op);
+ }
+
+ try {
+ zkSetClusterPolicy(container.getZkController().getZkStateReader(), clusterPolicy);
+ } catch (Exception e) {
+ log.warn("error persisting policies");
+ op.addError(e.getMessage());
+ checkErr(op);
+
}
- zkSetClusterPolicy(container.getZkController().getZkStateReader(), clusterPolicy);
rsp.getValues().add("result", "success");
}
+ private void checkErr(CommandOperation op) {
+ if (!op.hasError()) return;
+ throw new ApiBag.ExceptionWithErrObject(SolrException.ErrorCode.BAD_REQUEST, "Error in command payload", CommandOperation.captureErrors(Collections.singletonList(op)));
+ }
+
private void handleSetClusterPreferences(SolrQueryRequest req, SolrQueryResponse rsp, CommandOperation op) throws KeeperException, InterruptedException, IOException {
List preferences = (List) op.getCommandData();
if (preferences == null || !(preferences instanceof List)) {
- throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "A list of cluster preferences not found");
+ op.addError("A list of cluster preferences not found");
+ checkErr(op);
}
zkSetPreferences(container.getZkController().getZkStateReader(), preferences);
rsp.getValues().add("result", "success");
@@ -185,15 +201,13 @@ public class AutoScalingHandler extends RequestHandlerBase implements Permission
private void handleRemovePolicy(SolrQueryRequest req, SolrQueryResponse rsp, CommandOperation op) throws KeeperException, InterruptedException, IOException {
String policyName = (String) op.getCommandData();
- if (policyName.trim().length() == 0) {
- throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "The policy name cannot be empty");
- }
+ if (op.hasError()) checkErr(op);
Map<String, Object> autoScalingConf = zkReadAutoScalingConf(container.getZkController().getZkStateReader());
Map<String, Object> policies = (Map<String, Object>) autoScalingConf.get("policies");
if (policies == null || !policies.containsKey(policyName)) {
- throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "No policy exists with name: " + policyName);
+ op.addError("No policy exists with name: " + policyName);
}
-
+ checkErr(op);
zkSetPolicies(container.getZkController().getZkStateReader(), policyName, null);
rsp.getValues().add("result", "success");
}
@@ -203,11 +217,18 @@ public class AutoScalingHandler extends RequestHandlerBase implements Permission
for (Map.Entry<String, Object> policy : policies.entrySet()) {
String policyName = policy.getKey();
if (policyName == null || policyName.trim().length() == 0) {
- throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "The policy name cannot be null or empty");
+ op.addError("The policy name cannot be null or empty");
}
}
+ checkErr(op);
- zkSetPolicies(container.getZkController().getZkStateReader(), null, policies);
+ try {
+ zkSetPolicies(container.getZkController().getZkStateReader(), null, policies);
+ } catch (Exception e) {
+ log.warn("error persisting policies", e);
+ op.addError(e.getMessage());
+ checkErr(op);
+ }
rsp.getValues().add("result", "success");
}
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/c1d28c3e/solr/core/src/java/org/apache/solr/servlet/ResponseUtils.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/servlet/ResponseUtils.java b/solr/core/src/java/org/apache/solr/servlet/ResponseUtils.java
index 90ca968..228bca8 100644
--- a/solr/core/src/java/org/apache/solr/servlet/ResponseUtils.java
+++ b/solr/core/src/java/org/apache/solr/servlet/ResponseUtils.java
@@ -15,15 +15,15 @@
* limitations under the License.
*/
package org.apache.solr.servlet;
+
+import java.io.PrintWriter;
+import java.io.StringWriter;
+
import org.apache.solr.api.ApiBag;
import org.apache.solr.common.SolrException;
import org.apache.solr.common.util.NamedList;
-import org.apache.solr.common.util.CommandOperation;
import org.slf4j.Logger;
-import java.io.PrintWriter;
-import java.io.StringWriter;
-
/**
* Response helper methods.
*/
@@ -52,7 +52,7 @@ public class ResponseUtils {
info.add("metadata", errorMetadata);
if (ex instanceof ApiBag.ExceptionWithErrObject) {
ApiBag.ExceptionWithErrObject exception = (ApiBag.ExceptionWithErrObject) ex;
- info.add(CommandOperation.ERR_MSGS, exception.getErrs() );
+ info.add("details", exception.getErrs() );
}
}
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/c1d28c3e/solr/core/src/test/org/apache/solr/cloud/autoscaling/AutoScalingHandlerTest.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/cloud/autoscaling/AutoScalingHandlerTest.java b/solr/core/src/test/org/apache/solr/cloud/autoscaling/AutoScalingHandlerTest.java
index 197801a..9250e8c 100644
--- a/solr/core/src/test/org/apache/solr/cloud/autoscaling/AutoScalingHandlerTest.java
+++ b/solr/core/src/test/org/apache/solr/cloud/autoscaling/AutoScalingHandlerTest.java
@@ -81,9 +81,11 @@ public class AutoScalingHandlerTest extends SolrCloudTestCase {
try {
solrClient.request(req);
fail("Adding a policy with 'cores' attribute should not have succeeded.");
- } catch (HttpSolrClient.RemoteSolrException e) {
+ } catch (HttpSolrClient.RemoteExecutionException e) {
+ String message = String.valueOf(Utils.getObjectByPath(e.getMetaData(), true, "error/details[0]/errorMessages[0]"));
+
// expected
- assertTrue(e.getMessage().contains("cores is only allowed in 'cluster-policy'"));
+ assertTrue(message.contains("cores is only allowed in 'cluster-policy'"));
}
setPolicyCommand = "{'set-policy': {" +
@@ -175,6 +177,25 @@ public class AutoScalingHandlerTest extends SolrCloudTestCase {
assertNotNull(clusterPolicy);
assertEquals(3, clusterPolicy.size());
}
+ public void testErrorHandling() throws Exception {
+ CloudSolrClient solrClient = cluster.getSolrClient();
+
+ String setClusterPolicyCommand = "{" +
+ " 'set-cluster-policy': [" +
+ " {'cores':'<10', 'node':'#ANY'}," +
+ " {'shard': '#EACH', 'node': '#ANY'}," +
+ " {'nodeRole':'overseer', 'replica':0}" +
+ " ]" +
+ "}";
+ try {
+ SolrRequest req = createAutoScalingRequest(SolrRequest.METHOD.POST, setClusterPolicyCommand);
+ solrClient.request(req);
+ fail("expect exception");
+ } catch (HttpSolrClient.RemoteExecutionException e) {
+ String message = String.valueOf(Utils.getObjectByPath(e.getMetaData(), true, "error/details[0]/errorMessages[0]"));
+ assertTrue(message.contains("replica is required in"));
+ }
+ }
@Test
public void testReadApi() throws Exception {
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/c1d28c3e/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java
----------------------------------------------------------------------
diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java
index 7566ba0..02d7c1a 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/HttpSolrClient.java
@@ -23,6 +23,7 @@ import java.lang.invoke.MethodHandles;
import java.net.ConnectException;
import java.net.SocketTimeoutException;
import java.nio.charset.StandardCharsets;
+import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.Iterator;
@@ -497,7 +498,9 @@ public class HttpSolrClient extends SolrClient {
throw new SolrServerException("Unsupported method: " + request.getMethod());
}
-
+
+ private static final List<String> errPath = Arrays.asList("metadata", "error-class");//Utils.getObjectByPath(err, false,"metadata/error-class")
+
protected NamedList<Object> executeMethod(HttpRequestBase method, final ResponseParser processor, final boolean isV2Api) throws SolrServerException {
method.addHeader("User-Agent", AGENT);
@@ -596,11 +599,9 @@ public class HttpSolrClient extends SolrClient {
} catch (Exception e) {
throw new RemoteSolrException(baseUrl, httpStatus, e.getMessage(), e);
}
- if (isV2Api) {
- Object err = rsp.get("error");
- if (err != null) {
+ Object error = rsp == null ? null : rsp.get("error");
+ if (error != null && (isV2Api || String.valueOf(getObjectByPath(error, true, errPath)).endsWith("ExceptionWithErrObject"))) {
throw RemoteExecutionException.create(baseUrl, rsp);
- }
}
if (httpStatus != HttpStatus.SC_OK && !isV2Api) {
NamedList<String> metadata = null;
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/c1d28c3e/solr/solrj/src/java/org/apache/solr/common/util/Utils.java
----------------------------------------------------------------------
diff --git a/solr/solrj/src/java/org/apache/solr/common/util/Utils.java b/solr/solrj/src/java/org/apache/solr/common/util/Utils.java
index 0605a35..8083886 100644
--- a/solr/solrj/src/java/org/apache/solr/common/util/Utils.java
+++ b/solr/solrj/src/java/org/apache/solr/common/util/Utils.java
@@ -285,7 +285,7 @@ public class Utils {
public static Object getObjectByPath(Object root, boolean onlyPrimitive, List<String> hierarchy) {
if(root == null) return null;
- if(!isMapLike(root)) throw new RuntimeException("must be a Map or NamedList");
+ if(!isMapLike(root)) return null;
Object obj = root;
for (int i = 0; i < hierarchy.size(); i++) {
int idx = -1;