Report reindexing progress as -1 for non-current states

5 files changed, 34 insertions, 13 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..f9eff86b200 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 >= 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));
diff --git a/simplemetrics/src/main/java/com/yahoo/metrics/simple/MetricUpdater.java b/simplemetrics/src/main/java/com/yahoo/metrics/simple/MetricUpdater.java
index 06d048c5211..848132c9bea 100644
--- a/simplemetrics/src/main/java/com/yahoo/metrics/simple/MetricUpdater.java
+++ b/simplemetrics/src/main/java/com/yahoo/metrics/simple/MetricUpdater.java
@@ -6,7 +6,7 @@ import com.yahoo.concurrent.ThreadLocalDirectory.Updater;
 /**
  * The link between each single thread and the central data store.
  *
- * @author <a href="mailto:steinar@yahoo-inc.com">Steinar Knutsen</a>
+ * @author Steinar Knutsen
  */
 class MetricUpdater implements Updater<Bucket, Sample> {
 
diff --git a/vespajlib/src/main/java/com/yahoo/concurrent/LocalInstance.java b/vespajlib/src/main/java/com/yahoo/concurrent/LocalInstance.java
index 1e929f3ee36..1f886c6f34a 100644
--- a/vespajlib/src/main/java/com/yahoo/concurrent/LocalInstance.java
+++ b/vespajlib/src/main/java/com/yahoo/concurrent/LocalInstance.java
@@ -10,12 +10,12 @@ import com.yahoo.concurrent.ThreadLocalDirectory.Updater;
  * {@link ThreadLocal} in ThreadLocalDirectory if possible, but has no user
  * available methods.
  *
- * @param AGGREGATOR
+ * @param <AGGREGATOR>
  *            the structure to insert produced data into
- * @param SAMPLE
+ * @param <SAMPLE>
  *            type of produced data to insert from each participating thread
  *
- * @author <a href="mailto:steinar@yahoo-inc.com">Steinar Knutsen</a>
+ * @author Steinar Knutsen
  */
 public final class LocalInstance<AGGREGATOR, SAMPLE> {
     /**
diff --git a/vespajlib/src/main/java/com/yahoo/concurrent/ThreadLocalDirectory.java b/vespajlib/src/main/java/com/yahoo/concurrent/ThreadLocalDirectory.java
index 5ab1c88775a..016ad167e98 100644
--- a/vespajlib/src/main/java/com/yahoo/concurrent/ThreadLocalDirectory.java
+++ b/vespajlib/src/main/java/com/yahoo/concurrent/ThreadLocalDirectory.java
@@ -95,7 +95,7 @@ public final class ThreadLocalDirectory<AGGREGATOR, SAMPLE> {
          * This might be an empty list, creating a new counter set to zero, or
          * even copying the current state of LocalInstance.current.
          * LocalInstance.current will be set to the value received from this
-         * factory after invokation this method.
+         * factory after invocation this method.
          *
          * <p>
          * The first time this method is invoked for a thread, previous will be
@@ -223,7 +223,7 @@ public final class ThreadLocalDirectory<AGGREGATOR, SAMPLE> {
         // Has to set registered before adding to the list. Otherwise, the
         // instance might be removed from the list, set as unregistered, and
         // then the local thread might happily remove that information. The Java
-        // memory model is a guarantuee for the minimum amount of visibility,
+        // memory model is a guarantee for the minimum amount of visibility,
         // not a definition of the actual amount.
         q.setRegistered(true);
         synchronized (directoryLock) {