Let BucketSpaceStats track invalid count to allow transition from invalid -> valid.

4 files changed, 52 insertions, 19 deletions

diff --git a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ContentNodeStats.java b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ContentNodeStats.java
index b579b406890..72a7b2e4bcf 100644
--- a/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ContentNodeStats.java
+++ b/clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/ContentNodeStats.java
@@ -14,18 +14,18 @@ public class ContentNodeStats {
     private Map<String, BucketSpaceStats> bucketSpaces = new HashMap<>();
 
     public static class BucketSpaceStats {
-        private boolean invalid;
+        private int invalidCount;
         private long bucketsTotal;
         private long bucketsPending;
 
         private BucketSpaceStats() {
-            this.invalid = true;
+            this.invalidCount = 1;
             this.bucketsTotal = 0;
             this.bucketsPending = 0;
         }
 
-        private BucketSpaceStats(long bucketsTotal, long bucketsPending) {
-            this.invalid = false;
+        private BucketSpaceStats(long bucketsTotal, long bucketsPending, boolean invalid) {
+            this.invalidCount = (invalid ? 1 : 0);
             this.bucketsTotal = bucketsTotal;
             this.bucketsPending = bucketsPending;
         }
@@ -34,8 +34,16 @@ public class ContentNodeStats {
             return new BucketSpaceStats();
         }
 
+        public static BucketSpaceStats invalid(long bucketsTotal, long bucketsPending) {
+            return new BucketSpaceStats(bucketsTotal, bucketsPending, true);
+        }
+
         public static BucketSpaceStats of(long bucketsTotal, long bucketsPending) {
-            return new BucketSpaceStats(bucketsTotal, bucketsPending);
+            return new BucketSpaceStats(bucketsTotal, bucketsPending, false);
+        }
+
+        public static BucketSpaceStats empty() {
+            return new BucketSpaceStats(0, 0, false);
         }
 
         public long getBucketsTotal() {
@@ -47,15 +55,15 @@ public class ContentNodeStats {
         }
 
         public boolean mayHaveBucketsPending() {
-            return (bucketsPending > 0) || invalid;
+            return (bucketsPending > 0) || (invalidCount > 0);
         }
 
         public boolean valid() {
-            return !invalid;
+            return invalidCount == 0;
         }
 
         public void merge(BucketSpaceStats rhs, int factor) {
-            this.invalid |= rhs.invalid;
+            this.invalidCount += (factor * rhs.invalidCount);
             this.bucketsTotal += (factor * rhs.bucketsTotal);
             this.bucketsPending += (factor * rhs.bucketsPending);
         }
@@ -65,18 +73,19 @@ public class ContentNodeStats {
             if (this == o) return true;
             if (o == null || getClass() != o.getClass()) return false;
             BucketSpaceStats that = (BucketSpaceStats) o;
-            return bucketsTotal == that.bucketsTotal &&
+            return invalidCount == that.invalidCount &&
+                    bucketsTotal == that.bucketsTotal &&
                     bucketsPending == that.bucketsPending;
         }
 
         @Override
         public int hashCode() {
-            return Objects.hash(bucketsTotal, bucketsPending);
+            return Objects.hash(invalidCount, bucketsTotal, bucketsPending);
         }
 
         @Override
         public String toString() {
-            return "{bucketsTotal=" + bucketsTotal + ", bucketsPending=" + bucketsPending + "}";
+            return "{bucketsTotal=" + bucketsTotal + ", bucketsPending=" + bucketsPending + ", invalidCount=" + invalidCount + "}";
         }
     }
 
@@ -116,7 +125,7 @@ public class ContentNodeStats {
         for (Map.Entry<String, BucketSpaceStats> entry : stats.bucketSpaces.entrySet()) {
             BucketSpaceStats statsToUpdate = bucketSpaces.get(entry.getKey());
             if (statsToUpdate == null && factor == 1) {
-                statsToUpdate = new BucketSpaceStats();
+                statsToUpdate = BucketSpaceStats.empty();
                 bucketSpaces.put(entry.getKey(), statsToUpdate);
             }
             if (statsToUpdate != null) {
diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ClusterStatsAggregatorTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ClusterStatsAggregatorTest.java
index 3ad18a0c64d..35391fc16f7 100644
--- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ClusterStatsAggregatorTest.java
+++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ClusterStatsAggregatorTest.java
@@ -75,7 +75,7 @@ public class ClusterStatsAggregatorTest {
         f.verify(new ContentClusterStatsBuilder().add(3, "default"));
 
         f.update(2, new ContentClusterStatsBuilder().add(3, "default", 10, 1));
-        f.verify(new ContentClusterStatsBuilder().add(3, "default", 10, 1));
+        f.verify(new ContentClusterStatsBuilder().addInvalid(3, "default", 10, 1));
 
         f.update(1, new ContentClusterStatsBuilder().add(3, "default", 11, 2));
         f.verify(new ContentClusterStatsBuilder().add(3, "default", 10 + 11, 1 + 2));
diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ContentClusterStatsBuilder.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ContentClusterStatsBuilder.java
index ec0d5903aeb..16767cafa8f 100644
--- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ContentClusterStatsBuilder.java
+++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ContentClusterStatsBuilder.java
@@ -15,6 +15,10 @@ public class ContentClusterStatsBuilder {
         return add(nodeIndex, bucketSpace, ContentNodeStats.BucketSpaceStats.of(bucketsTotal, bucketsPending));
     }
 
+    public ContentClusterStatsBuilder addInvalid(int nodeIndex, String bucketSpace, long bucketsTotal, long bucketsPending) {
+        return add(nodeIndex, bucketSpace, ContentNodeStats.BucketSpaceStats.invalid(bucketsTotal, bucketsPending));
+    }
+
     public ContentClusterStatsBuilder add(int nodeIndex, String bucketSpace) {
         return add(nodeIndex, bucketSpace, ContentNodeStats.BucketSpaceStats.invalid());
     }
diff --git a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ContentNodeStatsTest.java b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ContentNodeStatsTest.java
index 7bd8be73e66..c142913a061 100644
--- a/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ContentNodeStatsTest.java
+++ b/clustercontroller-core/src/test/java/com/yahoo/vespa/clustercontroller/core/ContentNodeStatsTest.java
@@ -11,25 +11,44 @@ import static org.junit.Assert.assertTrue;
 public class ContentNodeStatsTest {
 
     @Test
-    public void valid_bucket_space_stats_can_be_invalid_after_merge() {
+    public void bucket_space_stats_can_transition_from_valid_to_invalid() {
         BucketSpaceStats stats = BucketSpaceStats.of(5,1);
         assertTrue(stats.valid());
 
         stats.merge(BucketSpaceStats.invalid(), 1);
         assertFalse(stats.valid());
-        assertEquals(5, stats.getBucketsTotal());
-        assertEquals(1, stats.getBucketsPending());
+        assertEquals(BucketSpaceStats.invalid(5, 1), stats);
     }
 
     @Test
-    public void invalid_bucket_space_stats_is_still_invalid_after_merge() {
+    public void bucket_space_stats_can_transition_from_invalid_to_valid() {
         BucketSpaceStats stats = BucketSpaceStats.invalid();
         assertFalse(stats.valid());
 
         stats.merge(BucketSpaceStats.of(5, 1), 1);
         assertFalse(stats.valid());
-        assertEquals(5, stats.getBucketsTotal());
-        assertEquals(1, stats.getBucketsPending());
+        stats.merge(BucketSpaceStats.invalid(), -1);
+        assertTrue(stats.valid());
+        assertEquals(BucketSpaceStats.of(5, 1), stats);
+    }
+
+    @Test
+    public void bucket_space_stats_tracks_multiple_layers_of_invalid() {
+        BucketSpaceStats stats = BucketSpaceStats.invalid();
+        stats.merge(BucketSpaceStats.invalid(), 1);
+        assertFalse(stats.valid());
+        stats.merge(BucketSpaceStats.invalid(), 1);
+        assertFalse(stats.valid());
+        stats.merge(BucketSpaceStats.of(5, 1), 1);
+        assertFalse(stats.valid());
+
+        stats.merge(BucketSpaceStats.invalid(), -1);
+        assertFalse(stats.valid());
+        stats.merge(BucketSpaceStats.invalid(), -1);
+        assertFalse(stats.valid());
+        stats.merge(BucketSpaceStats.invalid(), -1);
+        assertTrue(stats.valid());
+        assertEquals(BucketSpaceStats.of(5, 1), stats);
     }
 
     @Test
@@ -46,4 +65,5 @@ public class ContentNodeStatsTest {
     public void valid_bucket_space_stats_may_have_no_pending_buckets() {
         assertFalse(BucketSpaceStats.of(5, 0).mayHaveBucketsPending());
     }
+
 }