From 3e43ffd02d19a0af114f9d341b34ab6a621607dc Mon Sep 17 00:00:00 2001 From: Jon Marius Venstad Date: Fri, 8 Nov 2019 09:07:40 +0100 Subject: Use not() for negated conditions in ApplicationList --- .../controller/application/ApplicationList.java | 60 ++++++---------------- .../hosted/controller/maintenance/Upgrader.java | 18 +++---- .../hosted/controller/versions/VespaVersion.java | 8 ++- 3 files changed, 28 insertions(+), 58 deletions(-) (limited to 'controller-server') diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationList.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationList.java index 8978af78574..3fe8e9f52c3 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationList.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/application/ApplicationList.java @@ -12,7 +12,6 @@ import com.yahoo.vespa.hosted.controller.Instance; import java.time.Instant; import java.util.Collection; -import java.util.Collections; import java.util.Comparator; import java.util.List; import java.util.Optional; @@ -59,7 +58,12 @@ public class ApplicationList extends AbstractFilteringList isUpgradingTo(version, application)); + return upgradingTo(List.of(version)); + } + + /** Returns the subset of applications which are currently upgrading to the given version */ + public ApplicationList upgradingTo(Collection versions) { + return matching(application -> versions.stream().anyMatch(version -> isUpgradingTo(version, application))); } /** Returns the subset of applications which are not pinned to a certain Vespa version. */ @@ -67,47 +71,22 @@ public class ApplicationList extends AbstractFilteringList ! application.change().isPinned()); } - /** Returns the subset of applications which are currently not upgrading to the given version */ - public ApplicationList notUpgradingTo(Version version) { - return notUpgradingTo(Collections.singletonList(version)); - } - - public ApplicationList notFailingUpgrade() { - return matching(application -> application.instances().values().stream() + public ApplicationList failingUpgrade() { + return matching(application -> ! application.instances().values().stream() .allMatch(instance -> JobList.from(instance) .failing() .not().failingApplicationChange() .isEmpty())); } - /** Returns the subset of applications which are currently not upgrading to any of the given versions */ - public ApplicationList notUpgradingTo(Collection versions) { - return matching(application -> versions.stream().noneMatch(version -> isUpgradingTo(version, application))); - } - - /** - * Returns the subset of applications which are currently not upgrading to the given version, - * or returns all if no version is specified - */ - public ApplicationList notUpgradingTo(Optional version) { - if (version.isEmpty()) return this; - return notUpgradingTo(version.get()); - } - /** Returns the subset of applications which have changes left to deploy; blocked, or deploying */ public ApplicationList withChanges() { return matching(application -> application.change().hasTargets() || application.outstandingChange().hasTargets()); } - /** Returns the subset of applications which are currently not deploying a change */ - public ApplicationList notDeploying() { - return matching(application -> ! application.change().hasTargets()); - } - - /** Returns the subset of applications which currently does not have any failing jobs */ - public ApplicationList notFailing() { - return matching(application -> application.instances().values().stream() - .noneMatch(instance -> instance.deploymentJobs().hasFailures())); + /** Returns the subset of applications which are currently deploying a change */ + public ApplicationList deploying() { + return matching(application -> application.change().hasTargets()); } /** Returns the subset of applications which currently have failing jobs */ @@ -119,19 +98,19 @@ public class ApplicationList extends AbstractFilteringList application.instances().values().stream() - .anyMatch(instance -> failingUpgradeToVersionSince(instance, version, threshold))); + .anyMatch(instance -> failingUpgradeToVersionSince(instance, version, threshold))); } /** Returns the subset of applications which have been failing an application change since the given instant */ public ApplicationList failingApplicationChangeSince(Instant threshold) { return matching(application -> application.instances().values().stream() - .anyMatch(instance -> failingApplicationChangeSince(instance, threshold))); + .anyMatch(instance -> failingApplicationChangeSince(instance, threshold))); } - /** Returns the subset of applications which currently does not have any failing jobs on the given version */ - public ApplicationList notFailingOn(Version version) { + /** Returns the subset of applications which currently have failing jobs on the given version */ + public ApplicationList failingOn(Version version) { return matching(application -> application.instances().values().stream() - .noneMatch(instance -> failingOn(version, instance))); + .anyMatch(instance -> failingOn(version, instance))); } /** Returns the subset of applications which have at least one production deployment */ @@ -153,13 +132,6 @@ public class ApplicationList extends AbstractFilteringList instance.upgradePolicy() == policy)); } - /** Returns the subset of applications which does not have the given upgrade policy */ - // TODO jonmv: Make this instance based when instances are orchestrated, and deployments reported per instance. - public ApplicationList without(UpgradePolicy policy) { - return matching(application -> application.deploymentSpec().instances().stream() - .allMatch(instance -> instance.upgradePolicy() != policy)); - } - /** Returns the subset of applications which have at least one deployment on a lower version than the given one */ public ApplicationList onLowerVersionThan(Version version) { return matching(application -> application.instances().values().stream() diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Upgrader.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Upgrader.java index 08fa3abcd9f..28e276c1497 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Upgrader.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/maintenance/Upgrader.java @@ -49,29 +49,29 @@ public class Upgrader extends Maintainer { @Override public void maintain() { // Determine target versions for each upgrade policy - Optional canaryTarget = controller().versionStatus().systemVersion().map(VespaVersion::versionNumber); + Version canaryTarget = controller().systemVersion(); Collection defaultTargets = targetVersions(Confidence.normal); Collection conservativeTargets = targetVersions(Confidence.high); // Cancel upgrades to broken targets (let other ongoing upgrades complete to avoid starvation) for (VespaVersion version : controller().versionStatus().versions()) { if (version.confidence() == Confidence.broken) - cancelUpgradesOf(applications().without(UpgradePolicy.canary).upgradingTo(version.versionNumber()), + cancelUpgradesOf(applications().not().with(UpgradePolicy.canary).upgradingTo(version.versionNumber()), version.versionNumber() + " is broken"); } // Canaries should always try the canary target - cancelUpgradesOf(applications().with(UpgradePolicy.canary).upgrading().notUpgradingTo(canaryTarget), + cancelUpgradesOf(applications().with(UpgradePolicy.canary).upgrading().not().upgradingTo(canaryTarget), "Outdated target version for Canaries"); // Cancel *failed* upgrades to earlier versions, as the new version may fix it String reason = "Failing on outdated version"; - cancelUpgradesOf(applications().with(UpgradePolicy.defaultPolicy).upgrading().failing().notUpgradingTo(defaultTargets), reason); - cancelUpgradesOf(applications().with(UpgradePolicy.conservative).upgrading().failing().notUpgradingTo(conservativeTargets), reason); + cancelUpgradesOf(applications().with(UpgradePolicy.defaultPolicy).upgrading().failing().not().upgradingTo(defaultTargets), reason); + cancelUpgradesOf(applications().with(UpgradePolicy.conservative).upgrading().failing().not().upgradingTo(conservativeTargets), reason); // Schedule the right upgrades ApplicationList applications = applications(); - canaryTarget.ifPresent(target -> upgrade(applications.with(UpgradePolicy.canary), target)); + upgrade(applications.with(UpgradePolicy.canary), canaryTarget); defaultTargets.forEach(target -> upgrade(applications.with(UpgradePolicy.defaultPolicy), target)); conservativeTargets.forEach(target -> upgrade(applications.with(UpgradePolicy.conservative), target)); } @@ -98,13 +98,13 @@ public class Upgrader extends Maintainer { applications = applications.withProductionDeployment(); applications = applications.onLowerVersionThan(version); applications = applications.allowMajorVersion(version.getMajor(), targetMajorVersion().orElse(version.getMajor())); - applications = applications.notDeploying(); // wait with applications deploying an application change or already upgrading - applications = applications.notFailingOn(version); // try to upgrade only if it hasn't failed on this version + applications = applications.not().deploying(); // wait with applications deploying an application change or already upgrading + applications = applications.not().failingOn(version); // try to upgrade only if it hasn't failed on this version applications = applications.canUpgradeAt(controller().clock().instant()); // wait with applications that are currently blocking upgrades applications = applications.byIncreasingDeployedVersion(); // start with lowest versions for (Application application : applications.with(UpgradePolicy.canary).asList()) controller().applications().deploymentTrigger().triggerChange(application.id(), Change.of(version)); - for (Application application : applications.without(UpgradePolicy.canary).first(numberOfApplicationsToUpgrade()).asList()) + for (Application application : applications.not().with(UpgradePolicy.canary).first(numberOfApplicationsToUpgrade()).asList()) controller().applications().deploymentTrigger().triggerChange(application.id(), Change.of(version)); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VespaVersion.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VespaVersion.java index d0cf5aac2d9..296639245a3 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VespaVersion.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/versions/VespaVersion.java @@ -45,9 +45,7 @@ public class VespaVersion implements Comparable { public static Confidence confidenceFrom(DeploymentStatistics statistics, Controller controller) { // 'production on this': All deployment jobs upgrading to this version have completed without failure - ApplicationList productionOnThis = ApplicationList.from(statistics.production(), controller.applications()) - .notUpgradingTo(statistics.version()) - .notFailingUpgrade(); + ApplicationList productionOnThis = ApplicationList.from(statistics.production(), controller.applications()).not().upgradingTo(statistics.version()).not().failingUpgrade(); ApplicationList failingOnThis = ApplicationList.from(statistics.failing(), controller.applications()); ApplicationList all = ApplicationList.from(controller.applications().asList()) .withProductionDeployment(); @@ -162,8 +160,8 @@ public class VespaVersion implements Comparable { private static boolean nonCanaryApplicationsBroken(Version version, ApplicationList failingOnThis, ApplicationList productionOnThis) { - ApplicationList failingNonCanaries = failingOnThis.without(UpgradePolicy.canary).startedFailingOn(version); - ApplicationList productionNonCanaries = productionOnThis.without(UpgradePolicy.canary); + ApplicationList failingNonCanaries = failingOnThis.not().with(UpgradePolicy.canary).startedFailingOn(version); + ApplicationList productionNonCanaries = productionOnThis.not().with(UpgradePolicy.canary); if (productionNonCanaries.size() + failingNonCanaries.size() == 0) return false; -- cgit v1.2.3