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


##########
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
+            : "We shouldn't be sorting this list again";
+
+        // Make sure we do not tend to use always the same nodes (within an 
AZ) if all

Review Comment:
   I just carried this from the existing implementation. In fact, when using 
this spread across groups there is less randomization, since the groups are 
chosen in order (defined by the hashmap iteration, which is likely to be the 
same within the same instance, same data and same Java version).
   I'm still shuffling the same way within the group though.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to