From 9f56b9f995e2297f60b02fc06af5a8f0897612bf Mon Sep 17 00:00:00 2001
From: Harald Musum <musum@yahooinc.com>
Date: Fri, 14 Jul 2023 12:29:53 +0200
Subject: Split out method for finding min replication per distributor

---
 .../core/NodeStateChangeChecker.java               | 35 ++++++++++++----------
 1 file changed, 20 insertions(+), 15 deletions(-)

(limited to 'clustercontroller-core/src/main')

diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/NodeStateChangeChecker.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/NodeStateChangeChecker.java
index 50cdf260105..a36bd187a47 100644
--- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/NodeStateChangeChecker.java
+++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/NodeStateChangeChecker.java
@@ -16,6 +16,7 @@ import com.yahoo.vespa.clustercontroller.utils.staterestapi.requests.SetUnitStat
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
@@ -391,26 +392,30 @@ public class NodeStateChangeChecker {
     }
 
     private Result checkRedundancy(DistributorNodeInfo distributorNodeInfo, Node node) {
-        List<StorageNode> storageNodes = distributorNodeInfo.getHostInfo().getDistributor().getStorageNodes();
-        for (StorageNode storageNode : storageNodes) {
-            if (storageNode.getIndex() == node.getIndex()) {
-                Integer minReplication = storageNode.getMinCurrentReplicationFactorOrNull();
-                // Why test on != null? Missing min-replication is OK (indicate empty/few buckets on system).
-                if (minReplication != null && minReplication < requiredRedundancy) {
-                    return disallow("Distributor " + distributorNodeInfo.getNodeIndex()
-                            + " says storage node " + node.getIndex()
-                            + " has buckets with redundancy as low as "
-                            + storageNode.getMinCurrentReplicationFactorOrNull()
-                            + ", but we require at least " + requiredRedundancy);
-                } else {
-                    return allow();
-                }
-            }
+        Integer minReplication = minReplication(distributorNodeInfo).get(node.getIndex());
+        // Why test on != null? Missing min-replication is OK (indicate empty/few buckets on system).
+        if (minReplication != null && minReplication < requiredRedundancy) {
+            return disallow("Distributor " + distributorNodeInfo.getNodeIndex()
+                                    + " says storage node " + node.getIndex()
+                                    + " has buckets with redundancy as low as "
+                                    + minReplication + ", but we require at least " + requiredRedundancy);
         }
 
         return allow();
     }
 
+    private Map<Integer, Integer> minReplication(DistributorNodeInfo distributorNodeInfo) {
+        Map<Integer, Integer> replicationPerNodeIndex = new HashMap<>();
+        for (StorageNode storageNode : distributorNodeInfo.getHostInfo().getDistributor().getStorageNodes()) {
+            var currentValue = replicationPerNodeIndex.get(storageNode.getIndex());
+            Integer minReplicationFactor = storageNode.getMinCurrentReplicationFactorOrNull();
+            if (currentValue == null || (minReplicationFactor != null && minReplicationFactor < currentValue))
+                replicationPerNodeIndex.put(storageNode.getIndex(), minReplicationFactor);
+        }
+
+        return replicationPerNodeIndex;
+    }
+
     /**
      * We want to check with the distributors to verify that it is safe to take down the storage node.
      * @param node the node to be checked
-- 
cgit v1.2.3