SOLR-11538: Implement suggestions for port,ip_*, nodeRole,sysprop.*, metrics:*

solr/CHANGES.txt

@@ -67,6 +67,8 @@ New Features
 * SOLR-11003: Support bi-directional syncing of cdcr clusters. We still only support active indexing in one cluster,
   but have the ability to switch indexing clusters and cdcr will replicate correctly. (Amrit Sarkar, Varun Thacker)
 
+* SOLR-11538: Implement suggestions for port,ip_*, nodeRole,sysprop.*, metrics:* (noble)
+
 Bug Fixes
 ----------------------
 

solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/PolicyHelper.java

@@ -146,10 +146,10 @@ public class PolicyHelper {
     suggestionCtx.session = policy.createSession(cloudManager);
     List<Violation> violations = suggestionCtx.session.getViolations();
     for (Violation violation : violations) {
-      Suggestion.getTagType(violation.getClause().isPerCollectiontag() ?
+      Suggestion.ConditionType tagType = Suggestion.getTagType(violation.getClause().isPerCollectiontag() ?
-          violation.getClause().tag.name:
+          violation.getClause().tag.name :
-          violation.getClause().globalTag.name)
+          violation.getClause().globalTag.name);
-          .getSuggestions(suggestionCtx.setViolation(violation));
+      tagType.getSuggestions(suggestionCtx.setViolation(violation));
       suggestionCtx.violation = null;
     }
     return suggestionCtx.getSuggestions();

solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Suggestion.java

@@ -29,6 +29,7 @@ import java.util.stream.Stream;
 
 import org.apache.solr.client.solrj.SolrRequest;
 import org.apache.solr.client.solrj.V2RequestSupport;
+import org.apache.solr.client.solrj.cloud.autoscaling.Violation.ReplicaInfoAndErr;
 import org.apache.solr.common.cloud.rule.ImplicitSnitch;
 import org.apache.solr.common.util.Pair;
 import org.apache.solr.common.util.StrUtils;
@@ -42,7 +43,7 @@ public class Suggestion {
 
   public static ConditionType getTagType(String name) {
     ConditionType info = validatetypes.get(name);
-    if (info == null && name.startsWith(ImplicitSnitch.SYSPROP)) info = validatetypes.get("STRING");
+    if (info == null && name.startsWith(ImplicitSnitch.SYSPROP)) info = ConditionType.LAZY;
     if (info == null && name.startsWith(Clause.METRICS_PREFIX)) info = ConditionType.LAZY;
     return info;
   }
@@ -132,7 +133,7 @@ public class Suggestion {
           List<Row> matchingNodes = ctx.session.matrix.stream().filter(
               row -> ctx.violation.getViolatingReplicas()
                   .stream()
-                  .anyMatch(p -> row.node.equals(p.first().getNode())))
+                  .anyMatch(p -> row.node.equals(p.replicaInfo.getNode())))
               .sorted(rowComparator)
               .collect(Collectors.toList());
 
@@ -173,8 +174,8 @@ public class Suggestion {
       public void addViolatingReplicas(ViolationCtx ctx) {
         for (Row r : ctx.allRows) {
           if (!ctx.clause.tag.isPass(r)) {
-            r.forEachReplica(replicaInfo -> ctx.currentViolation.addReplica(replicaInfo,
+            r.forEachReplica(replicaInfo -> ctx.currentViolation
-                new Violation.ViolationMeta().withDelta(ctx.clause.tag.delta(r.getVal(ImplicitSnitch.CORES)))));
+                .addReplica(new ReplicaInfoAndErr(replicaInfo).withDelta(ctx.clause.tag.delta(r.getVal(ImplicitSnitch.CORES)))));
           }
         }
 
@@ -218,6 +219,17 @@ public class Suggestion {
       public Object validate(String name, Object val, boolean isRuleVal) {
         return Clause.parseString(val);
       }
+
+      @Override
+      public void getSuggestions(SuggestionCtx ctx) {
+        if(ctx.violation == null)return;
+        for (ReplicaInfoAndErr e : ctx.violation.getViolatingReplicas()) {
+          Suggester suggester = ctx.session.getSuggester(MOVEREPLICA)
+              .hint(Suggester.Hint.COLL_SHARD, new Pair<>( e.replicaInfo.getCollection(), e.replicaInfo.getShard()))
+              .hint(Suggester.Hint.SRC_NODE, e.replicaInfo.getNode());
+          if (ctx.addSuggestion(suggester) == null) break;
+        }
+      }
     },;
 
     final Class type;
@@ -252,7 +264,7 @@ public class Suggestion {
           if (!ctx.clause.replica.isPass(0) && ctx.clause.tag.isPass(row)) return;
           if (!ctx.clause.collection.isPass(ctx.currentViolation.coll) || !ctx.clause.shard.isPass(ctx.currentViolation.shard))
             return;
-          ctx.currentViolation.addReplica(replica, new Violation.ViolationMeta().withDelta(ctx.clause.tag.delta(row.getVal(ctx.clause.tag.name))));
+          ctx.currentViolation.addReplica(new ReplicaInfoAndErr(replica).withDelta(ctx.clause.tag.delta(row.getVal(ctx.clause.tag.name))));
         });
       }
     }

solr/solrj/src/java/org/apache/solr/client/solrj/cloud/autoscaling/Violation.java

@@ -25,7 +25,6 @@ import java.util.List;
 import java.util.Objects;
 
 import org.apache.solr.common.MapWriter;
-import org.apache.solr.common.util.Pair;
 import org.apache.solr.common.util.Utils;
 
 public class Violation implements MapWriter {
@@ -35,7 +34,7 @@ public class Violation implements MapWriter {
   final Object tagKey;
   private final int hash;
   private final Clause clause;
-  private List<Pair<ReplicaInfo, ViolationMeta>> violationVsMetaData = new ArrayList<>();
+  private List<ReplicaInfoAndErr> violationVsMetaData = new ArrayList<>();
 
   Violation(Clause clause, String coll, String shard, String node, Object actualVal, Long replicaCountDelta, Object tagKey) {
     this.clause = clause;
@@ -48,18 +47,37 @@ public class Violation implements MapWriter {
     hash = ("" + coll + " " + shard + " " + node + " " + String.valueOf(tagKey) + " " + Utils.toJSONString(getClause().toMap(new HashMap<>()))).hashCode();
   }
 
-  public Violation addReplica(ReplicaInfo r, ViolationMeta meta) {
+  public Violation addReplica(ReplicaInfoAndErr r) {
-    violationVsMetaData.add(new Pair<>(r, meta));
+    violationVsMetaData.add(r);
     return this;
   }
 
-  public List<Pair<ReplicaInfo, ViolationMeta>> getViolatingReplicas() {
+  public List<ReplicaInfoAndErr> getViolatingReplicas() {
     return violationVsMetaData;
   }
 
   public Clause getClause() {
     return clause;
   }
+  static class ReplicaInfoAndErr implements MapWriter{
+    final ReplicaInfo replicaInfo;
+
+    ReplicaInfoAndErr(ReplicaInfo replicaInfo) {
+      this.replicaInfo = replicaInfo;
+    }
+    Long delta;
+
+    public ReplicaInfoAndErr withDelta(Long delta) {
+      this.delta = delta;
+      return this;
+    }
+
+    @Override
+    public void writeMap(EntryWriter ew) throws IOException {
+      ew.put("replica", replicaInfo);
+      ew.putIfNotNull("delta",delta );
+    }
+  }
 
   @Override
   public int hashCode() {
@@ -102,23 +120,4 @@ public class Violation implements MapWriter {
     });
     ew.put("clause", getClause());
   }
-
-  static class ViolationMeta implements MapWriter {
-    Long delta;
-
-    public ViolationMeta withDelta(Long delta) {
-      this.delta = delta;
-      return this;
-    }
-
-    @Override
-    public void writeMap(EntryWriter ew) throws IOException {
-      ew.putIfNotNull("delta", delta);
-    }
-
-    @Override
-    public String toString() {
-      return Utils.toJSONString(this);
-    }
-  }
 }

solr/solrj/src/java/org/apache/solr/common/util/Utils.java

@@ -326,11 +326,27 @@ public class Utils {
   }
 
   private static boolean isMapLike(Object o) {
-    return o instanceof Map || o instanceof NamedList;
+    return o instanceof Map || o instanceof NamedList || o instanceof MapWriter;
   }
 
   private static Object getVal(Object obj, String key) {
-    if(obj instanceof NamedList) return ((NamedList) obj).get(key);
+    if (obj instanceof MapWriter) {
+      Object[] result = new Object[1];
+      try {
+        ((MapWriter) obj).writeMap(new MapWriter.EntryWriter() {
+          @Override
+          public MapWriter.EntryWriter put(String k, Object v) throws IOException {
+            if (key.equals(k)) result[0] = v;
+            return this;
+          }
+        });
+      } catch (IOException e) {
+        throw new RuntimeException(e);
+      }
+      return result[0];
+    }
+
+    if (obj instanceof NamedList) return ((NamedList) obj).get(key);
     else if (obj instanceof Map) return ((Map) obj).get(key);
     else throw new RuntimeException("must be a NamedList or Map");
   }

solr/solrj/src/test/org/apache/solr/client/solrj/cloud/autoscaling/TestPolicy.java

@@ -451,12 +451,14 @@ public class TestPolicy extends SolrTestCaseJ4 {
         return new DelegatingNodeStateProvider(null) {
           @Override
           public Map<String, Object> getNodeValues(String node, Collection<String> tags) {
-            return (Map<String, Object>) Utils.getObjectByPath(m,false, Arrays.asList("nodeValues", node));
+            Map<String, Object> result = (Map<String, Object>) Utils.getObjectByPath(m, false, Arrays.asList("nodeValues", node));
+            return result == null ? Collections.emptyMap() : result;
           }
 
           @Override
           public Map<String, Map<String, List<ReplicaInfo>>> getReplicaInfo(String node, Collection<String> keys) {
-            return (Map<String, Map<String, List<ReplicaInfo>>>) Utils.getObjectByPath(m,false, Arrays.asList("replicaInfo", node));
+            Map<String, Map<String, List<ReplicaInfo>>> result = (Map<String, Map<String, List<ReplicaInfo>>>) Utils.getObjectByPath(m, false, Arrays.asList("replicaInfo", node));
+            return result == null ? Collections.emptyMap() : result;
           }
         };
       }
@@ -1472,4 +1474,44 @@ public class TestPolicy extends SolrTestCaseJ4 {
 
   }
 
+  public void testSyspropSuggestions() {
+    String autoScalingjson = "{" +
+        "  'cluster-preferences': [" +
+        "    { 'maximize': 'freedisk', 'precision': 50}," +
+        "    { 'minimize': 'cores', 'precision': 3}" +
+        "  ]," +
+        "  'cluster-policy': [" +
+        "    { 'replica': '1', shard:'#EACH', sysprop.fs : 'ssd'}" +
+        "  ]" +
+        "}";
+
+
+    String dataproviderdata = "{" +
+        "  'liveNodes': [" +
+        "    'node1'," +
+        "    'node2'," +
+        "    'node3'" +
+        "  ]," +
+        "  'replicaInfo': {" +
+        "    'node1': {" +
+        "      'c1': {'s1': [{'r1': {'type': 'NRT'}, 'r2': {'type': 'NRT'}}]," +
+        "             's2': [{'r1': {'type': 'NRT'}, 'r2': {'type': 'NRT'}}]}," +
+        "    }" +
+        "  }," +
+        "    'nodeValues': {" +
+        "      'node1': {'cores': 2, 'freedisk': 334, 'sysprop.fs': 'slowdisk'}," +
+        "      'node2': {'cores': 2, 'freedisk': 749, 'sysprop.fs': 'slowdisk'}," +
+        "      'node3': {'cores': 0, 'freedisk': 262, 'sysprop.fs': 'ssd'}" +
+        "    }" +
+        "}";
+    AutoScalingConfig cfg = new AutoScalingConfig((Map<String, Object>) Utils.fromJSONString(autoScalingjson));
+    List<Violation> violations = cfg.getPolicy().createSession(cloudManagerWithData(dataproviderdata)).getViolations();
+    assertEquals(2, violations.size());
+    List<Suggester.SuggestionInfo> suggestions = PolicyHelper.getSuggestions(cfg, cloudManagerWithData(dataproviderdata));
+    assertEquals(2, suggestions.size());
+    for (Suggester.SuggestionInfo suggestion : suggestions) {
+      Utils.getObjectByPath(suggestion ,true, "operation/move-replica/targetNode");
+    }
+  }
+
 }