Use a SortedMap to communicate order of map entries

3 files changed, 11 insertions, 9 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java
index 72279be921e..13c7860ebe3 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java
@@ -75,7 +75,7 @@ public class JobController {
     public void updateStorage() {
         for (ApplicationId id : applications())
             for (JobType type : jobs(id)) {
-                locked(id, type, runs -> { // runs is unmodified, and written back as such.
+                locked(id, type, runs -> { // runs is not modified here, and is written as it was.
                     curator.readLastRun(id, type).ifPresent(curator::writeLastRun);
                 });
             }
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java
index 0d8ea8d2537..9ee477e5ab2 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/CuratorDb.java
@@ -33,11 +33,12 @@ import java.time.Duration;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashSet;
-import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
+import java.util.SortedMap;
+import java.util.TreeMap;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.TimeoutException;
 import java.util.function.Function;
@@ -47,6 +48,7 @@ import java.util.logging.Logger;
 import java.util.stream.Collectors;
 import java.util.stream.LongStream;
 
+import static java.util.Comparator.comparing;
 import static java.util.stream.Collectors.collectingAndThen;
 
 /**
@@ -365,9 +367,8 @@ public class CuratorDb {
         return readSlime(lastRunPath(id, type)).map(runSerializer::runFromSlime);
     }
 
-    public Map<RunId, Run> readHistoricRuns(ApplicationId id, JobType type) {
-        // TODO jvenstad: Add, somewhere, a retention filter based on age or count.
-        return readSlime(runsPath(id, type)).map(runSerializer::runsFromSlime).orElse(new LinkedHashMap<>());
+    public SortedMap<RunId, Run> readHistoricRuns(ApplicationId id, JobType type) {
+        return readSlime(runsPath(id, type)).map(runSerializer::runsFromSlime).orElse(new TreeMap<>(comparing(RunId::number)));
     }
 
     public void deleteRunData(ApplicationId id, JobType type) {
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/RunSerializer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/RunSerializer.java
index b94a3b16898..5103dc244ed 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/RunSerializer.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/persistence/RunSerializer.java
@@ -20,9 +20,9 @@ import com.yahoo.vespa.hosted.controller.deployment.Versions;
 
 import java.time.Instant;
 import java.util.EnumMap;
-import java.util.LinkedHashMap;
-import java.util.Map;
 import java.util.Optional;
+import java.util.SortedMap;
+import java.util.TreeMap;
 
 import static com.yahoo.vespa.hosted.controller.deployment.RunStatus.aborted;
 import static com.yahoo.vespa.hosted.controller.deployment.RunStatus.deploymentFailed;
@@ -46,6 +46,7 @@ import static com.yahoo.vespa.hosted.controller.deployment.Step.installTester;
 import static com.yahoo.vespa.hosted.controller.deployment.Step.report;
 import static com.yahoo.vespa.hosted.controller.deployment.Step.startTests;
 import static com.yahoo.vespa.hosted.controller.deployment.Step.endTests;
+import static java.util.Comparator.comparing;
 
 /**
  * Serialises and deserialises RunStatus objects for persistent storage.
@@ -74,8 +75,8 @@ class RunSerializer {
         return runFromSlime(slime.get());
     }
 
-    Map<RunId, Run> runsFromSlime(Slime slime) {
-        Map<RunId, Run> runs = new LinkedHashMap<>();
+    SortedMap<RunId, Run> runsFromSlime(Slime slime) {
+        SortedMap<RunId, Run> runs = new TreeMap<>(comparing(RunId::number));
         Inspector runArray = slime.get();
         runArray.traverse((ArrayTraverser) (__, runObject) -> {
             Run run = runFromSlime(runObject);