Never modify cached runs

The cached runs can be read by multiple threads, which will cause a
ConcurrentModificationException if the same runs are being updated by another
thread

3 files changed, 8 insertions, 8 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 6e5218917bf..2480c81755e 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
@@ -41,6 +41,7 @@ import java.util.NavigableMap;
 import java.util.Optional;
 import java.util.Set;
 import java.util.SortedMap;
+import java.util.TreeMap;
 import java.util.concurrent.TimeoutException;
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.function.Consumer;
@@ -603,7 +604,7 @@ public class JobController {
     /** Locks all runs and modifies the list of historic runs for the given application and job type. */
     private void locked(ApplicationId id, JobType type, Consumer<SortedMap<RunId, Run>> modifications) {
         try (Lock __ = curator.lock(id, type)) {
-            SortedMap<RunId, Run> runs = curator.readHistoricRuns(id, type);
+            SortedMap<RunId, Run> runs = new TreeMap<>(curator.readHistoricRuns(id, type));
             modifications.accept(runs);
             curator.writeHistoricRuns(id, type, runs.values());
         }
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 c6dd8bab309..caad32d9a17 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
@@ -53,7 +53,6 @@ import java.util.Map;
 import java.util.NavigableMap;
 import java.util.Optional;
 import java.util.Set;
-import java.util.TreeMap;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.TimeoutException;
 import java.util.function.Function;
@@ -63,7 +62,6 @@ 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;
 import static java.util.stream.Collectors.toUnmodifiableList;
 
@@ -425,7 +423,7 @@ public class CuratorDb {
                               old != null && old.getFirst() == stat.getVersion()
                               ? old
                               : new Pair<>(stat.getVersion(), runSerializer.runsFromSlime(readSlime(path).get()))).getSecond())
-                      .orElse(new TreeMap<>(comparing(RunId::number)));
+                      .orElseGet(Collections::emptyNavigableMap);
     }
 
     public void deleteRunData(ApplicationId id, JobType type) {
@@ -546,7 +544,7 @@ public class CuratorDb {
 
     public ZoneRoutingPolicy readZoneRoutingPolicy(ZoneId zone) {
         return readSlime(zoneRoutingPolicyPath(zone)).map(data -> zoneRoutingPolicySerializer.fromSlime(zone, data))
-                                                     .orElse(new ZoneRoutingPolicy(zone, RoutingStatus.DEFAULT));
+                                                     .orElseGet(() -> new ZoneRoutingPolicy(zone, RoutingStatus.DEFAULT));
     }
 
     // -------------- Application endpoint certificates ----------------------------
@@ -590,7 +588,7 @@ public class CuratorDb {
 
     public Set<ArchiveBucket> readArchiveBuckets(ZoneId zoneId) {
         return curator.getData(archiveBucketsPath(zoneId)).map(String::new).map(ArchiveBucketsSerializer::fromJsonString)
-                .orElse(Set.of());
+                .orElseGet(Set::of);
     }
 
     public void writeArchiveBuckets(ZoneId zoneid, Set<ArchiveBucket> archiveBuckets) {
@@ -655,7 +653,7 @@ public class CuratorDb {
     // -------------- Job Retrigger entries -----------------------------------
 
     public List<RetriggerEntry> readRetriggerEntries() {
-        return readSlime(deploymentRetriggerPath()).map(RetriggerEntrySerializer::fromSlime).orElse(List.of());
+        return readSlime(deploymentRetriggerPath()).map(RetriggerEntrySerializer::fromSlime).orElseGet(List::of);
     }
 
     public void writeRetriggerEntries(List<RetriggerEntry> retriggerEntries) {
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 289d6c3a99d..c93b77e9af1 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
@@ -24,6 +24,7 @@ import com.yahoo.vespa.hosted.controller.deployment.Versions;
 
 import java.time.Instant;
 import java.time.temporal.ChronoUnit;
+import java.util.Collections;
 import java.util.EnumMap;
 import java.util.NavigableMap;
 import java.util.Optional;
@@ -113,7 +114,7 @@ class RunSerializer {
             Run run = runFromSlime(runObject);
             runs.put(run.id(), run);
         });
-        return runs;
+        return Collections.unmodifiableNavigableMap(runs);
     }
 
     private Run runFromSlime(Inspector runObject) {