[GitHub] [solr] tflobbe commented on a diff in pull request #1577: SOLR-16719: Let AffinityPlacementFactory have an anti-affinity label

["tflobbe (via GitHub)" <gi...@apache.org>]

tflobbe commented on code in PR #1577:
URL: https://github.com/apache/solr/pull/1577#discussion_r1172983143


##########
solr/core/src/java/org/apache/solr/cluster/placement/plugins/AffinityPlacementFactory.java:
##########
@@ -503,15 +534,199 @@ private String getNodeAZ(Node n, final AttributeValues attrValues) {
      */
     private static class AzWithNodes {
       final String azName;
-      List<Node> availableNodesForPlacement;
-      boolean hasBeenSorted;
-
-      AzWithNodes(String azName, List<Node> availableNodesForPlacement) {
+      private final boolean useAntiAffinity;
+      private boolean listIsSorted = false;
+      private final Comparator<Node> nodeComparator;
+      private final Random random;
+      private final List<Node> availableNodesForPlacement;
+      private final AttributeValues attributeValues;
+      private TreeSet<AffinityGroupWithNodes> sortedAntiAffinityGroups;
+      private final Map<String, Integer> currentAntiAffinityUsage;
+      private int numNodesForPlacement;
+
+      AzWithNodes(
+          String azName,
+          List<Node> availableNodesForPlacement,
+          boolean useAntiAffinity,
+          Comparator<Node> nodeComparator,
+          Random random,
+          AttributeValues attributeValues,
+          Map<String, Integer> currentAntiAffinityUsage) {
         this.azName = azName;
         this.availableNodesForPlacement = availableNodesForPlacement;
-        // Once the list is sorted to an order we're happy with, this flag is set to true to avoid
-        // sorting multiple times unnecessarily.
-        this.hasBeenSorted = false;
+        this.useAntiAffinity = useAntiAffinity;
+        this.nodeComparator = nodeComparator;
+        this.random = random;
+        this.attributeValues = attributeValues;
+        this.currentAntiAffinityUsage = currentAntiAffinityUsage;
+        this.numNodesForPlacement = availableNodesForPlacement.size();
+      }
+
+      private boolean hasBeenSorted() {
+        return (useAntiAffinity && sortedAntiAffinityGroups != null)
+            || (!useAntiAffinity && listIsSorted);
+      }
+
+      void ensureSorted() {
+        if (!hasBeenSorted()) {
+          sort();
+        }
+      }
+
+      private void sort() {
+        assert !listIsSorted && sortedAntiAffinityGroups == null

Review Comment:
   Given these are internal private classes, I'd rather make sure they are being used as designed. Returning here would feel like hiding misuse IMO



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org