Merge pull request #15641 from vespa-engine/jonmv/"clear"-reindexing-metrics0on-state-change

Report reindexing progress as -1 for non-current states

2 files changed, 28 insertions, 7 deletions

diff --git a/clustercontroller-reindexer/src/main/java/ai/vespa/reindexing/ReindexingMetrics.java b/clustercontroller-reindexer/src/main/java/ai/vespa/reindexing/ReindexingMetrics.java
index 5e536d1f2ee..cd3d964b4a5 100644
--- a/clustercontroller-reindexer/src/main/java/ai/vespa/reindexing/ReindexingMetrics.java
+++ b/clustercontroller-reindexer/src/main/java/ai/vespa/reindexing/ReindexingMetrics.java
@@ -5,6 +5,7 @@ import com.yahoo.documentapi.ProgressToken;
 import com.yahoo.jdisc.Metric;
 
 import java.time.Clock;
+import java.util.EnumSet;
 import java.util.Map;
 
 import static ai.vespa.reindexing.Reindexing.State.SUCCESSFUL;
@@ -26,12 +27,20 @@ class ReindexingMetrics {
 
     void dump(Reindexing reindexing) {
         reindexing.status().forEach((type, status) -> {
+            Reindexing.State state = status.state();
             metric.set("reindexing.progress",
                        status.progress().map(ProgressToken::percentFinished).map(percentage -> percentage * 1e-2)
                              .orElse(status.state() == SUCCESSFUL ? 1.0 : 0.0),
                        metric.createContext(Map.of("clusterid", cluster,
                                                    "documenttype", type.getName(),
-                                                   "state", toString(status.state()))));
+                                                   "state", toString(state))));
+            // Set metric value to -1 for all states not currently active, so we only have one value >= 0 at any given time.
+            for (Reindexing.State unset : EnumSet.complementOf(EnumSet.of(state)))
+                metric.set("reindexing.progress",
+                           -1,
+                           metric.createContext(Map.of("clusterid", cluster,
+                                                       "documenttype", type.getName(),
+                                                       "state", toString(unset))));
         });
     }
 
diff --git a/clustercontroller-reindexer/src/test/java/ai/vespa/reindexing/ReindexerTest.java b/clustercontroller-reindexer/src/test/java/ai/vespa/reindexing/ReindexerTest.java
index b0ffdf8ae60..3ba4083121c 100644
--- a/clustercontroller-reindexer/src/test/java/ai/vespa/reindexing/ReindexerTest.java
+++ b/clustercontroller-reindexer/src/test/java/ai/vespa/reindexing/ReindexerTest.java
@@ -106,7 +106,19 @@ class ReindexerTest {
         assertEquals(Map.of("reindexing.progress", Map.of(Map.of("documenttype", "music",
                                                                  "clusterid", "cluster",
                                                                  "state", "successful"),
-                                                          1.0)),
+                                                          1.0,
+                                                          Map.of("documenttype", "music",
+                                                                 "clusterid", "cluster",
+                                                                 "state", "pending"),
+                                                          -1.0,
+                                                          Map.of("documenttype", "music",
+                                                                 "clusterid", "cluster",
+                                                                 "state", "failed"),
+                                                          -1.0,
+                                                          Map.of("documenttype", "music",
+                                                                 "clusterid", "cluster",
+                                                                 "state", "running"),
+                                                          -1.0)),
                      metric.metrics());
 
         // New config tells reindexer to reindex "music" documents no earlier than at 10 millis after EPOCH, which isn't yet.
@@ -146,11 +158,11 @@ class ReindexerTest {
         reindexing = reindexing.with(music, Status.ready(clock.instant()).running().progressed(new ProgressToken()).halted());
         assertEquals(reindexing, database.readReindexing());
         assertTrue(shutDown.get(), "Session was shut down");
-        assertEquals(Map.of("reindexing.progress", Map.of(Map.of("documenttype", "music",
-                                                                 "clusterid", "cluster",
-                                                                 "state", "pending"),
-                                                          1.0)), // new ProgressToken() is 100% done.
-                     metric.metrics());
+        assertEquals(1.0, // new ProgressToken() is 100% done.
+                     metric.metrics().get("reindexing.progress")
+                           .get(Map.of("documenttype", "music",
+                                       "clusterid", "cluster",
+                                       "state", "pending")));
 
         // Last reindexing fails.
         clock.advance(Duration.ofMillis(10));