diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index 388d5dcdabe..8df583ad297 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -76,6 +76,8 @@ New Features * SOLR-11031: Implement SystemLogListener for autoscaling (ab) +* SOLR-11205: Any metrics value can be directly accessed in autoscaling policies (noble) + Bug Fixes ---------------------- diff --git a/solr/core/src/test/org/apache/solr/cloud/autoscaling/TestPolicyCloud.java b/solr/core/src/test/org/apache/solr/cloud/autoscaling/TestPolicyCloud.java index bce297ef6ba..51cfb1d82ec 100644 --- a/solr/core/src/test/org/apache/solr/cloud/autoscaling/TestPolicyCloud.java +++ b/solr/core/src/test/org/apache/solr/cloud/autoscaling/TestPolicyCloud.java @@ -30,6 +30,7 @@ import org.apache.solr.client.solrj.SolrRequest; import org.apache.solr.client.solrj.SolrServerException; import org.apache.solr.client.solrj.embedded.JettySolrRunner; import org.apache.solr.client.solrj.impl.CloudSolrClient; +import org.apache.solr.client.solrj.impl.HttpSolrClient; import org.apache.solr.client.solrj.impl.SolrClientDataProvider; import org.apache.solr.client.solrj.request.CollectionAdminRequest; import org.apache.solr.cloud.OverseerTaskProcessor; @@ -48,6 +49,7 @@ import org.slf4j.LoggerFactory; import static org.apache.solr.cloud.autoscaling.AutoScalingHandlerTest.createAutoScalingRequest; import static org.apache.solr.common.cloud.ZkStateReader.SOLR_AUTOSCALING_CONF_PATH; +import static org.apache.solr.common.util.Utils.getObjectByPath; @LuceneTestCase.Slow public class TestPolicyCloud extends SolrCloudTestCase { @@ -135,6 +137,49 @@ public class TestPolicyCloud extends SolrCloudTestCase { assertEquals("Expected exactly three replica of collection on node with port: " + secondNodePort, 3, replicasOnNode2); } + public void testMetricsTag() throws Exception { + CloudSolrClient solrClient = cluster.getSolrClient(); + String setClusterPolicyCommand = "{" + + " 'set-cluster-policy': [" + + " {'cores':'<10', 'node':'#ANY'}," + + " {'replica':'<2', 'shard': '#EACH', 'node': '#ANY'}," + + " {'metrics:abc':'overseer', 'replica':0}" + + " ]" + + "}"; + SolrRequest req = createAutoScalingRequest(SolrRequest.METHOD.POST, setClusterPolicyCommand); + try { + solrClient.request(req); + fail("expected exception"); + } catch (HttpSolrClient.RemoteExecutionException e) { + // expected + assertTrue(String.valueOf(getObjectByPath(e.getMetaData(), + false, "error/details[0]/errorMessages[0]")).contains("Invalid metrics: param in")); + } + setClusterPolicyCommand = "{" + + " 'set-cluster-policy': [" + + " {'cores':'<10', 'node':'#ANY'}," + + " {'replica':'<2', 'shard': '#EACH', 'node': '#ANY'}," + + " {'metrics:solr.node:ADMIN./admin/authorization.clientErrors:count':'>58768765', 'replica':0}" + + " ]" + + "}"; + req = createAutoScalingRequest(SolrRequest.METHOD.POST, setClusterPolicyCommand); + solrClient.request(req); + + //org.eclipse.jetty.server.handler.DefaultHandler.2xx-responses + CollectionAdminRequest.createCollection("metricsTest", "conf", 1, 1) + .process(cluster.getSolrClient()); + DocCollection collection = getCollectionState("metricsTest"); + SolrClientDataProvider provider = new SolrClientDataProvider(solrClient); + List tags = Arrays.asList("metrics:solr.node:ADMIN./admin/authorization.clientErrors:count", + "metrics:solr.jvm:buffers.direct.Count"); + Map val = provider.getNodeValues(collection .getReplicas().get(0).getNodeName(), tags); + for (String tag : tags) { + assertNotNull( "missing : "+ tag , val.get(tag)); + } + + + } + public void testCreateCollectionWithPolicyAndMaxShardsPerNode() throws Exception { CloudSolrClient solrClient = cluster.getSolrClient(); Map original = Utils.getJson(solrClient.getZkStateReader().getZkClient(), SOLR_AUTOSCALING_CONF_PATH, true); diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Clause.java b/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Clause.java index ab9b4beb476..ec6f73ccf47 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Clause.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Clause.java @@ -84,6 +84,12 @@ public class Clause implements MapWriter, Comparable { } if (tag == null) throw new RuntimeException("Invalid op, must have one and only one tag other than collection, shard,replica " + Utils.toJSONString(m)); + if (tag.name.startsWith(Clause.METRICS_PREFIX)) { + List ss = StrUtils.splitSmart(tag.name, ':'); + if (ss.size() < 3 || ss.size() > 4) { + throw new RuntimeException("Invalid metrics: param in " + Utils.toJSONString(m) + " must have at 2 or 3 segments after 'metrics:' separated by ':'"); + } + } } @@ -153,12 +159,18 @@ public class Clause implements MapWriter, Comparable { this.op = op; } - public boolean isPass(Object inputVal) { + boolean isPass(Object inputVal) { if (inputVal instanceof ReplicaCount) inputVal = ((ReplicaCount) inputVal).getVal(type); - return op.match(val, validate(name, inputVal, false)) == PASS; + ValidateInfo validator = getValidator(name); + if (validator == LazyValidator.INST) { // we don't know the type + return op.match(parseString(val), parseString(inputVal)) == PASS; + } else { + return op.match(val, validate(name, inputVal, false)) == PASS; + } } - public boolean isPass(Row row) { + + boolean isPass(Row row) { return op.match(val, row.getVal(name)) == PASS; } @@ -172,7 +184,13 @@ public class Clause implements MapWriter, Comparable { } public Long delta(Object val) { - return op.delta(this.val, val); + if (this.val instanceof String) { + if (op == LESS_THAN || op == GREATER_THAN) { + return op.delta(Clause.parseDouble(name, this.val), Clause.parseDouble(name, val)); + } else { + return 0l; + } + } else return op.delta(this.val, val); } public String getName() { @@ -376,8 +394,57 @@ public class Clause implements MapWriter, Comparable { if (max != null && !type.isInstance(max)) throw new RuntimeException("wrong max value type, expected: " + type.getName() + " actual: " + max.getClass().getName()); } + + public Object validate(String name, Object val, boolean isRuleVal) { + if (type == Double.class) { + Double num = parseDouble(name, val); + if (isRuleVal) { + if (min != null) + if (Double.compare(num, (Double) min) == -1) + throw new RuntimeException(name + ": " + val + " must be greater than " + min); + if (max != null) + if (Double.compare(num, (Double) max) == 1) + throw new RuntimeException(name + ": " + val + " must be less than " + max); + } + return num; + } else if (type == Long.class) { + Long num = parseLong(name, val); + if (isRuleVal) { + if (min != null) + if (num < min.longValue()) + throw new RuntimeException(name + ": " + val + " must be greater than " + min); + if (max != null) + if (num > max.longValue()) + throw new RuntimeException(name + ": " + val + " must be less than " + max); + } + return num; + } else if (type == String.class) { + if (isRuleVal && vals != null && !vals.contains(val)) + throw new RuntimeException(name + ": " + val + " must be one of " + StrUtils.join(vals, ',')); + return val; + } else { + throw new RuntimeException("Invalid type "); + } + + } } + static class LazyValidator extends ValidateInfo { + static final LazyValidator INST = new LazyValidator(); + + LazyValidator() { + super(null, null, null, null); + } + + @Override + public Object validate(String name, Object val, boolean isRuleVal) { + return parseString(val); + } + } + + public static String parseString(Object val) { + return val == null ? null : String.valueOf(val); + } /** * @param name name of the condition @@ -385,40 +452,18 @@ public class Clause implements MapWriter, Comparable { * @param isRuleVal is this provided in the rule * @return actual validated value */ - public static Object validate(String name, Object val, boolean isRuleVal) { + public static Object validate(String name, Object val, boolean isRuleVal) { if (val == null) return null; + ValidateInfo info = getValidator(name); + if (info == null) throw new RuntimeException("Unknown type :" + name); + return info.validate(name, val, isRuleVal); + } + + private static ValidateInfo getValidator(String name) { ValidateInfo info = validatetypes.get(name); if (info == null && name.startsWith(ImplicitSnitch.SYSPROP)) info = validatetypes.get("STRING"); - if (info == null) throw new RuntimeException("Unknown type :" + name); - if (info.type == Double.class) { - Double num = parseDouble(name, val); - if (isRuleVal) { - if (info.min != null) - if (Double.compare(num, (Double) info.min) == -1) - throw new RuntimeException(name + ": " + val + " must be greater than " + info.min); - if (info.max != null) - if (Double.compare(num, (Double) info.max) == 1) - throw new RuntimeException(name + ": " + val + " must be less than " + info.max); - } - return num; - } else if (info.type == Long.class) { - Long num = parseLong(name, val); - if (isRuleVal) { - if (info.min != null) - if (num < info.min.longValue()) - throw new RuntimeException(name + ": " + val + " must be greater than " + info.min); - if (info.max != null) - if (num > info.max.longValue()) - throw new RuntimeException(name + ": " + val + " must be less than " + info.max); - } - return num; - } else if (info.type == String.class) { - if (isRuleVal && info.vals != null && !info.vals.contains(val)) - throw new RuntimeException(name + ": " + val + " must be one of " + StrUtils.join(info.vals, ',')); - return val; - } else { - throw new RuntimeException("Invalid type "); - } + if (info == null && name.startsWith(METRICS_PREFIX)) info = LazyValidator.INST; + return info; } public static Long parseLong(String name, Object val) { @@ -467,6 +512,8 @@ public class Clause implements MapWriter, Comparable { throw new RuntimeException(name + ": " + val + "not a valid number"); } + public static final String METRICS_PREFIX = "metrics:"; + private static final Map validatetypes = new HashMap<>(); static { @@ -482,6 +529,7 @@ public class Clause implements MapWriter, Comparable { validatetypes.put("NUMBER", new ValidateInfo(Long.class, null, 0L, Long.MAX_VALUE));//generic number validation validatetypes.put("STRING", new ValidateInfo(String.class, null, null, null));//generic string validation validatetypes.put("node", new ValidateInfo(String.class, null, null, null)); + validatetypes.put("LAZY", LazyValidator.INST); for (String ip : ImplicitSnitch.IP_SNITCHES) validatetypes.put(ip, new ValidateInfo(Long.class, null, 0L, 255L)); } } diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Operand.java b/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Operand.java index 6522cb20b69..13738c636f6 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Operand.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Operand.java @@ -58,6 +58,7 @@ public enum Operand { @Override public TestStatus match(Object ruleVal, Object testVal) { if (testVal == null) return NOT_APPLICABLE; + if (ruleVal instanceof String) ruleVal = Clause.parseDouble("", ruleVal); if (ruleVal instanceof Double) { return Double.compare(Clause.parseDouble("", testVal), (Double) ruleVal) == 1 ? PASS : FAIL; } @@ -73,6 +74,7 @@ public enum Operand { @Override public TestStatus match(Object ruleVal, Object testVal) { if (testVal == null) return NOT_APPLICABLE; + if (ruleVal instanceof String) ruleVal = Clause.parseDouble("", ruleVal); if (ruleVal instanceof Double) { return Double.compare(Clause.parseDouble("", testVal), (Double) ruleVal) == -1 ? PASS : FAIL; } @@ -93,10 +95,6 @@ public enum Operand { this.priority = priority; } - public String toStr(Object expectedVal) { - return operand + expectedVal.toString(); - } - public TestStatus match(Object ruleVal, Object testVal) { return Objects.equals(ruleVal, testVal) ? PASS : FAIL; } diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/SolrClientDataProvider.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/SolrClientDataProvider.java index 627de765f34..b3053b611aa 100644 --- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/SolrClientDataProvider.java +++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/SolrClientDataProvider.java @@ -32,6 +32,7 @@ import java.util.Set; import org.apache.solr.client.solrj.SolrRequest; import org.apache.solr.client.solrj.SolrServerException; +import org.apache.solr.client.solrj.cloud.autoscaling.Clause; import org.apache.solr.client.solrj.cloud.autoscaling.ClusterDataProvider; import org.apache.solr.client.solrj.cloud.autoscaling.ReplicaInfo; import org.apache.solr.client.solrj.request.GenericSolrRequest; @@ -54,6 +55,8 @@ import org.apache.zookeeper.KeeperException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import static org.apache.solr.client.solrj.cloud.autoscaling.Clause.METRICS_PREFIX; + /** * Class that implements {@link ClusterStateProvider} accepting a SolrClient */ @@ -165,6 +168,7 @@ public class SolrClientDataProvider implements ClusterDataProvider, MapWriter { protected void getRemoteInfo(String solrNode, Set requestedTags, SnitchContext ctx) { ClientSnitchCtx snitchContext = (ClientSnitchCtx) ctx; readSysProps(solrNode, requestedTags, snitchContext); + readMetrics(solrNode, requestedTags, snitchContext); Set groups = new HashSet<>(); List prefixes = new ArrayList<>(); if (requestedTags.contains(DISK)) { @@ -217,6 +221,34 @@ public class SolrClientDataProvider implements ClusterDataProvider, MapWriter { } } + private void readMetrics(String solrNode, Set requestedTags, ClientSnitchCtx snitchContext) { + List metricsNames = null; + for (String tag : requestedTags) { + if (tag.startsWith(METRICS_PREFIX)) { + if (metricsNames == null) metricsNames = new ArrayList<>(); + metricsNames.add(tag); + } + } + if (metricsNames == null) return; + for (String metricsName : metricsNames) { + List ss = StrUtils.splitSmart(metricsName, ':'); + if (ss.size() < 3 || ss.size() > 4) { + throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, "Invalid metrics name" + metricsName); + } + try { + ModifiableSolrParams params = new ModifiableSolrParams(); + params.add("group", ss.get(1)); + params.add("prefix", ss.get(2)); + if (ss.size() == 4) params.add("property", ss.get(3)); + SimpleSolrResponse rsp = snitchContext.invoke(solrNode, CommonParams.METRICS_PATH, params); + Object v = Utils.getObjectByPath(rsp.nl, true, ss); + if (v != null) snitchContext.getTags().put(metricsName, v); + } catch (Exception e) { + throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "", e); + } + } + } + private void readSysProps(String solrNode, Set requestedTags, ClientSnitchCtx snitchContext) { List prefixes = null; ModifiableSolrParams params; @@ -237,9 +269,8 @@ public class SolrClientDataProvider implements ClusterDataProvider, MapWriter { for (String s : sysProp) params.add("property", s); try { SimpleSolrResponse rsp = snitchContext.invoke(solrNode, CommonParams.METRICS_PATH, params); - Map m = rsp.nl.asMap(6); for (String s : sysProp) { - Object v = Utils.getObjectByPath(m, true, + Object v = Utils.getObjectByPath(rsp.nl, true, Arrays.asList("metrics", "solr.jvm", "system.properties", s)); if (v != null) snitchContext.getTags().put("sysprop." + s, v); } diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/cloud/autoscaling/TestPolicy.java b/solr/solrj/src/test/org/apache/solr/client/solrj/cloud/autoscaling/TestPolicy.java index 60239ad4feb..483489ac94d 100644 --- a/solr/solrj/src/test/org/apache/solr/client/solrj/cloud/autoscaling/TestPolicy.java +++ b/solr/solrj/src/test/org/apache/solr/client/solrj/cloud/autoscaling/TestPolicy.java @@ -31,14 +31,12 @@ import java.util.Map; import com.google.common.collect.ImmutableList; import org.apache.solr.SolrTestCaseJ4; import org.apache.solr.client.solrj.SolrRequest; -import org.apache.solr.client.solrj.impl.SolrClientDataProvider; -import org.apache.solr.client.solrj.request.CollectionAdminRequest; import org.apache.solr.client.solrj.cloud.autoscaling.Clause.Violation; import org.apache.solr.client.solrj.cloud.autoscaling.Policy.Suggester.Hint; +import org.apache.solr.client.solrj.request.CollectionAdminRequest; import org.apache.solr.common.cloud.Replica; import org.apache.solr.common.cloud.ReplicaPosition; import org.apache.solr.common.cloud.ZkStateReader; -import org.apache.solr.common.params.CollectionParams; import org.apache.solr.common.params.SolrParams; import org.apache.solr.common.util.Utils; import org.apache.solr.common.util.ValidatingJsonMap; @@ -189,6 +187,14 @@ public class TestPolicy extends SolrTestCaseJ4 { assertTrue(c.tag.isPass(12.8d)); assertFalse(c.tag.isPass("12.6")); assertFalse(c.tag.isPass(12.6d)); + + c = new Clause((Map) Utils.fromJSONString("{replica:0, 'metrics:x:y:z':'>12.7'}")); + assertTrue(c.tag.val instanceof String); + assertTrue(c.tag.isPass("12.8")); + assertTrue(c.tag.isPass(12.8d)); + assertFalse(c.tag.isPass("12.6")); + assertFalse(c.tag.isPass(12.6d)); + } public void testNodeLost() { @@ -1201,8 +1207,6 @@ public class TestPolicy extends SolrTestCaseJ4 { suggester = suggester.getSession().getSuggester(MOVEREPLICA).hint(Hint.TARGET_NODE, "10.0.0.6:7574_solr"); op = suggester.getOperation(); assertNull(op); - - }