Apache
/
lucene
mirror of https://github.com/apache/lucene.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

SOLR-11178: Change error handling in AutoScalingHandler to be consistent w/ other APIs

This commit is contained in:
Noble Paul 2017-08-04 17:18:10 +09:30
parent 1c47a85f23
commit d468313bac
3 changed files with 51 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
solr/core/src/java/org/apache/solr/cloud/autoscaling/AutoScalingHandler.java
View File

@ -250,7 +250,13 @@ public class AutoScalingHandler extends RequestHandlerBase implements Permission
op.addError("set-cluster-policy expects an array of objects");
return currentConfig;
}
List<Clause> cp = clusterPolicy.stream().map(Clause::new).collect(Collectors.toList());
List<Clause> cp = null;
try {
cp = clusterPolicy.stream().map(Clause::new).collect(Collectors.toList());
} catch (Exception e) {
op.addError(e.getMessage());
return currentConfig;
}
Policy p = currentConfig.getPolicy().withClusterPolicy(cp);
currentConfig = currentConfig.withPolicy(p);
return currentConfig;
@ -263,7 +269,13 @@ public class AutoScalingHandler extends RequestHandlerBase implements Permission
op.addError("A list of cluster preferences not found");
return currentConfig;
}
List<Preference> prefs = preferences.stream().map(Preference::new).collect(Collectors.toList());
List<Preference> prefs = null;
try {
prefs = preferences.stream().map(Preference::new).collect(Collectors.toList());
} catch (Exception e) {
op.addError(e.getMessage());
return currentConfig;
}
Policy p = currentConfig.getPolicy().withClusterPreferences(prefs);
currentConfig = currentConfig.withPolicy(p);
return currentConfig;
@ -305,8 +317,14 @@ public class AutoScalingHandler extends RequestHandlerBase implements Permission
}
List<String> params = new ArrayList<>(currentConfig.getPolicy().getParams());
Map<String, List<Clause>> mergedPolicies = new HashMap<>(currentConfig.getPolicy().getPolicies());
Map<String, List<Clause>> newPolicies = Policy.policiesFromMap((Map<String, List<Map<String, Object>>>)op.getCommandData(),
params);
Map<String, List<Clause>> newPolicies = null;
try {
newPolicies = Policy.policiesFromMap((Map<String, List<Map<String, Object>>>) op.getCommandData(),
params);
} catch (Exception e) {
op.addError(e.getMessage());
return currentConfig;
}
mergedPolicies.putAll(newPolicies);
Policy p = currentConfig.getPolicy().withPolicies(mergedPolicies).withParams(params);
currentConfig = currentConfig.withPolicy(p);

2
solr/core/src/java/org/apache/solr/servlet/ResponseUtils.java
View File

@ -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() );
}
}

34
solr/core/src/test/org/apache/solr/cloud/autoscaling/AutoScalingHandlerTest.java
View File

@ -344,10 +344,10 @@ public class AutoScalingHandlerTest extends SolrCloudTestCase {
try {
solrClient.request(req);
fail("expected exception");
} catch (HttpSolrClient.RemoteSolrException e) {
} catch (HttpSolrClient.RemoteExecutionException e) {
// expected
assertTrue(String.valueOf(getObjectByPath(((HttpSolrClient.RemoteExecutionException) e).getMetaData(),
false, "error/errorMessages[0]/errorMessages[0]")).contains("Cannot remove trigger: node_lost_trigger because it has active listeners: ["));
assertTrue(String.valueOf(getObjectByPath(e.getMetaData(),
false, "error/details[0]/errorMessages[0]")).contains("Cannot remove trigger: node_lost_trigger because it has active listeners: ["));
}
String removeListenerCommand = "{\n" +
@ -403,10 +403,31 @@ public class AutoScalingHandlerTest extends SolrCloudTestCase {
} catch (HttpSolrClient.RemoteSolrException e) {
// expected
assertTrue(String.valueOf(getObjectByPath(((HttpSolrClient.RemoteExecutionException) e).getMetaData(),
false, "error/errorMessages[0]/errorMessages[0]")).contains("A trigger with the name node_lost_trigger does not exist"));
false, "error/details[0]/errorMessages[0]")).contains("A trigger with the name node_lost_trigger does not exist"));
}
}
@Test
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 testPolicyAndPreferences() throws Exception {
CloudSolrClient solrClient = cluster.getSolrClient();
@ -428,7 +449,8 @@ public class AutoScalingHandlerTest extends SolrCloudTestCase {
fail("Adding a policy with 'cores' attribute should not have succeeded.");
} catch (HttpSolrClient.RemoteSolrException e) {
// expected
assertTrue(e.getMessage().contains("cores is only allowed in 'cluster-policy'"));
assertTrue(String.valueOf(getObjectByPath(((HttpSolrClient.RemoteExecutionException) e).getMetaData(),
false, "error/details[0]/errorMessages[0]")).contains("cores is only allowed in 'cluster-policy'"));
}
setPolicyCommand = "{'set-policy': {" +
@ -754,7 +776,7 @@ public class AutoScalingHandlerTest extends SolrCloudTestCase {
solrClient.request(createAutoScalingRequest(SolrRequest.METHOD.POST, removePolicyCommand));
fail("should have failed");
} catch (HttpSolrClient.RemoteExecutionException e) {
assertTrue(String.valueOf(getObjectByPath(e.getMetaData(), true, "error/errorMessages[0]/errorMessages[0]"))
assertTrue(String.valueOf(getObjectByPath(e.getMetaData(), true, "error/details[0]/errorMessages[0]"))
.contains("is being used by collection"));
} catch (Exception e) {
fail("Only RemoteExecutionException expected");