diff options
author | Martin Polden <mpolden@mpolden.no> | 2019-01-08 13:10:03 +0100 |
---|---|---|
committer | Martin Polden <mpolden@mpolden.no> | 2019-01-08 13:12:59 +0100 |
commit | 0bb150d3684dd84b5f78292a86091e235dc88180 (patch) | |
tree | 0d15518fdd5e742143ba52342a879757d71e502b /node-repository | |
parent | 080e5c3cea2393708c8cdfea16f43add68ba95d4 (diff) |
Fix unstable comparable in PeriodicApplicationMaintainer
Diffstat (limited to 'node-repository')
-rw-r--r-- | node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/PeriodicApplicationMaintainer.java | 22 |
1 files changed, 15 insertions, 7 deletions
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/PeriodicApplicationMaintainer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/PeriodicApplicationMaintainer.java index 8b2d0a55cd8..ff0773fde71 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/PeriodicApplicationMaintainer.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/PeriodicApplicationMaintainer.java @@ -10,10 +10,11 @@ import java.time.Clock; import java.time.Duration; import java.time.Instant; import java.util.Collections; -import java.util.Comparator; import java.util.LinkedHashSet; import java.util.List; +import java.util.Map; import java.util.Set; +import java.util.function.Function; import java.util.stream.Collectors; /** @@ -48,12 +49,19 @@ public class PeriodicApplicationMaintainer extends ApplicationMaintainer { protected Set<ApplicationId> applicationsNeedingMaintenance() { if (waitInitially()) return Collections.emptySet(); - return nodesNeedingMaintenance().stream() - .map(node -> node.allocation().get().owner()) - .filter(this::shouldBeDeployedOnThisServer) - .filter(this::canDeployNow) - .sorted(Comparator.comparing(this::getLastDeployTime)) - .collect(Collectors.toCollection(LinkedHashSet::new)); + // Collect all deployment times before sorting as deployments may happen while we build the set, breaking + // the comparable contract. Stale times are fine as the time is rechecked in ApplicationMaintainer#deployWithLock + Map<ApplicationId, Instant> deploymentTimes = nodesNeedingMaintenance().stream() + .map(node -> node.allocation().get().owner()) + .distinct() + .filter(this::shouldBeDeployedOnThisServer) + .filter(this::canDeployNow) + .collect(Collectors.toMap(Function.identity(), this::getLastDeployTime)); + + return deploymentTimes.entrySet().stream() + .sorted(Map.Entry.comparingByValue()) + .map(Map.Entry::getKey) + .collect(Collectors.toCollection(LinkedHashSet::new)); } // We only know last deploy time for applications that were deployed on this config server, |