diff options
author | Harald Musum <musum@yahoo-inc.com> | 2017-10-17 17:01:32 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2017-10-17 17:01:32 +0200 |
commit | cb970bf88a8d35c1970c1baad103be868e4a8f2e (patch) | |
tree | 634c45486847ea6d35d31152a3dd2e8c2fba9c85 /controller-server/src | |
parent | 8d475f58081a43f27dc349135a53e54b2c479921 (diff) | |
parent | 1d0f737067778f70d7908b8b390f06865eb19762 (diff) |
Merge pull request #3789 from vespa-engine/hmusum/add-persisted-setting-for-ignoring-confidence-calculations
Add persistent setting for ignoring confidence calculations
Diffstat (limited to 'controller-server/src')
8 files changed, 129 insertions, 23 deletions
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java index e338bc17788..c293e00ae48 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/Controller.java @@ -241,6 +241,8 @@ public class Controller extends AbstractComponent { return chefClient; } + public CuratorDb curator() { return curator; } + private String printableVersion(Optional<VespaVersion> vespaVersion) { return vespaVersion.map(v -> v.versionNumber().toFullString()).orElse("Unknown"); } 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 0722a58e18d..44b15053a8c 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 @@ -7,13 +7,11 @@ import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.Controller; import com.yahoo.vespa.hosted.controller.application.ApplicationList; import com.yahoo.vespa.hosted.controller.application.Change; -import com.yahoo.vespa.hosted.controller.deployment.BuildSystem; import com.yahoo.vespa.hosted.controller.persistence.CuratorDb; import com.yahoo.vespa.hosted.controller.versions.VespaVersion; import com.yahoo.yolean.Exceptions; import java.time.Duration; -import java.time.Instant; import java.util.logging.Level; import java.util.logging.Logger; @@ -111,4 +109,18 @@ public class Upgrader extends Maintainer { curator.writeUpgradesPerMinute(n); } + /** + * Returns whether to ignore confidence calculations when upgrading + */ + public boolean ignoreConfidence() { + return curator.readIgnoreConfidence(); + } + + /** + * Controls whether to ignore confidence calculations or not + */ + public void ignoreConfidence(boolean value) { + curator.writeIgnoreConfidence(value); + } + } 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 03b4dd6efe3..0a288955055 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 @@ -212,6 +212,20 @@ public class CuratorDb { transaction.commit(); } + public boolean readIgnoreConfidence() { + Optional<byte[]> value = curator.getData(ignoreConfidencePath()); + if (! value.isPresent() || value.get().length == 0) { + return false; // Default if value has never been written + } + return ByteBuffer.wrap(value.get()).getInt() == 1; + } + + public void writeIgnoreConfidence(boolean value) { + NestedTransaction transaction = new NestedTransaction(); + curator.set(ignoreConfidencePath(), ByteBuffer.allocate(Integer.BYTES).putInt(value ? 1 : 0).array()); + transaction.commit(); + } + public void writeVersionStatus(VersionStatus status) { VersionStatusSerializer serializer = new VersionStatusSerializer(); NestedTransaction transaction = new NestedTransaction(); @@ -305,6 +319,10 @@ public class CuratorDb { return root.append("upgrader").append("upgradesPerMinute"); } + private Path ignoreConfidencePath() { + return root.append("upgrader").append("ignoreConfidence"); + } + private Path versionStatusPath() { return root.append("versionStatus"); } private Path provisionStatePath() { diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/controller/ControllerApiHandler.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/controller/ControllerApiHandler.java index 03ac073a34a..162827cdb99 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/controller/ControllerApiHandler.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/controller/ControllerApiHandler.java @@ -7,9 +7,10 @@ import com.yahoo.container.jdisc.LoggingRequestHandler; import com.yahoo.container.logging.AccessLog; import com.yahoo.io.IOUtils; import com.yahoo.slime.Inspector; -import com.yahoo.slime.Slime; +import com.yahoo.text.Utf8; import com.yahoo.vespa.config.SlimeUtils; import com.yahoo.vespa.hosted.controller.maintenance.ControllerMaintenance; +import com.yahoo.vespa.hosted.controller.maintenance.Upgrader; import com.yahoo.vespa.hosted.controller.restapi.ErrorResponse; import com.yahoo.vespa.hosted.controller.restapi.MessageResponse; import com.yahoo.vespa.hosted.controller.restapi.Path; @@ -62,7 +63,7 @@ public class ControllerApiHandler extends LoggingRequestHandler { Path path = new Path(request.getUri().getPath()); if (path.matches("/controller/v1/")) return root(request); if (path.matches("/controller/v1/maintenance/")) return new JobsResponse(maintenance.jobControl()); - if (path.matches("/controller/v1/jobs/upgrader")) return new UpgraderResponse(maintenance.upgrader().upgradesPerMinute()); + if (path.matches("/controller/v1/jobs/upgrader")) return new UpgraderResponse(maintenance.upgrader()); return notFound(path); } @@ -101,18 +102,26 @@ public class ControllerApiHandler extends LoggingRequestHandler { private HttpResponse configureUpgrader(HttpRequest request) { String upgradesPerMinuteField = "upgradesPerMinute"; - Slime slime = toSlime(request.getData()); - Inspector inspect = slime.get(); + String ignoreConfidenceField = "ignoreConfidence"; + + byte[] jsonBytes = toJsonBytes(request.getData()); + Inspector inspect = SlimeUtils.jsonToSlime(jsonBytes).get(); + Upgrader upgrader = maintenance.upgrader(); if (inspect.field(upgradesPerMinuteField).valid()) { - maintenance.upgrader().setUpgradesPerMinute(inspect.field(upgradesPerMinuteField).asDouble()); + upgrader.setUpgradesPerMinute(inspect.field(upgradesPerMinuteField).asDouble()); + } else if (inspect.field(ignoreConfidenceField).valid()) { + upgrader.ignoreConfidence(inspect.field(ignoreConfidenceField).asBool()); + } else { + return ErrorResponse.badRequest("Unable to configure upgrader with data in request: '" + + Utf8.toString(jsonBytes) + "'"); } - return new UpgraderResponse(maintenance.upgrader().upgradesPerMinute()); + + return new UpgraderResponse(maintenance.upgrader()); } - private Slime toSlime(InputStream jsonStream) { + private byte[] toJsonBytes(InputStream jsonStream) { try { - byte[] jsonBytes = IOUtils.readBytes(jsonStream, 1000 * 1000); - return SlimeUtils.jsonToSlime(jsonBytes); + return IOUtils.readBytes(jsonStream, 1000 * 1000); } catch (IOException e) { throw new UncheckedIOException(e); } diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/controller/UpgraderResponse.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/controller/UpgraderResponse.java index fe88a0f1f22..3444f710d4c 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/controller/UpgraderResponse.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/controller/UpgraderResponse.java @@ -4,6 +4,7 @@ import com.yahoo.container.jdisc.HttpResponse; import com.yahoo.slime.Cursor; import com.yahoo.slime.JsonFormat; import com.yahoo.slime.Slime; +import com.yahoo.vespa.hosted.controller.maintenance.Upgrader; import java.io.IOException; import java.io.OutputStream; @@ -13,18 +14,19 @@ import java.io.OutputStream; */ public class UpgraderResponse extends HttpResponse { - private final double upgradesPerMinute; + private final Upgrader upgrader; - public UpgraderResponse(double upgradesPerMinute) { + public UpgraderResponse(Upgrader upgrader) { super(200); - this.upgradesPerMinute = upgradesPerMinute; + this.upgrader = upgrader; } @Override public void render(OutputStream outputStream) throws IOException { Slime slime = new Slime(); Cursor root = slime.setObject(); - root.setDouble("upgradesPerMinute", upgradesPerMinute); + root.setDouble("upgradesPerMinute", upgrader.upgradesPerMinute()); + root.setBool("ignoreConfidence", upgrader.ignoreConfidence()); new JsonFormat(true).encode(outputStream, slime); } 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 c1b9c045fbe..1541e4d35f8 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 @@ -6,6 +6,7 @@ import com.yahoo.component.Version; import com.yahoo.component.Vtag; import com.yahoo.vespa.hosted.controller.Controller; import com.yahoo.vespa.hosted.controller.application.ApplicationList; +import com.yahoo.vespa.hosted.controller.persistence.CuratorDb; import java.time.Instant; import java.util.Collection; @@ -57,8 +58,7 @@ public class VespaVersion implements Comparable<VespaVersion> { return Confidence.broken; // 'broken' if 4 non-canary was broken by this, and that is at least 10% of all - int brokenByThisVersion = failingOnThis.without(UpgradePolicy.canary).startedFailingAfter(releasedAt).size(); - if (brokenByThisVersion >= 4 && brokenByThisVersion >= productionOnThis.size() * 0.1) + if (nonCanaryApplicationsBroken(failingOnThis, productionOnThis, releasedAt, controller.curator())) return Confidence.broken; // 'low' unless all canary applications are upgraded @@ -136,4 +136,17 @@ public class VespaVersion implements Comparable<VespaVersion> { } + private static boolean nonCanaryApplicationsBroken(ApplicationList failingOnThis, + ApplicationList productionOnThis, + Instant releasedAt, + CuratorDb curator) { + ApplicationList failingNonCanaries = failingOnThis.without(UpgradePolicy.canary).startedFailingAfter(releasedAt); + ApplicationList productionNonCanaries = productionOnThis.without(UpgradePolicy.canary); + + if (productionNonCanaries.size() + failingNonCanaries.size() == 0 || curator.readIgnoreConfidence()) return false; + + // 'broken' if 4 non-canary was broken by this, and that is at least 10% of all + int brokenByThisVersion = failingNonCanaries.size(); + return brokenByThisVersion >= 4 && brokenByThisVersion >= productionOnThis.size() * 0.1; + } } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/controller/ControllerApiTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/controller/ControllerApiTest.java index e1c5cdb7742..8047b0d48c9 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/controller/ControllerApiTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/restapi/controller/ControllerApiTest.java @@ -7,6 +7,7 @@ import com.yahoo.vespa.hosted.controller.restapi.ControllerContainerTest; import org.junit.Test; import java.io.File; +import java.io.IOException; /** * @author bratseth @@ -42,7 +43,7 @@ public class ControllerApiTest extends ControllerContainerTest { // Get current configuration tester.assertResponse(new Request("http://localhost:8080/controller/v1/jobs/upgrader"), - "{\"upgradesPerMinute\":0.5}", + "{\"upgradesPerMinute\":0.5,\"ignoreConfidence\":false}", 200); // Set invalid configuration @@ -51,16 +52,22 @@ public class ControllerApiTest extends ControllerContainerTest { "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Upgrades per minute must be >= 0\"}", 400); - // Unrecognized fields are ignored + // Unrecognized field tester.assertResponse(new Request("http://localhost:8080/controller/v1/jobs/upgrader", "{\"foo\":bar}", Request.Method.PATCH), - "{\"upgradesPerMinute\":0.5}", + "{\"error-code\":\"BAD_REQUEST\",\"message\":\"Unable to configure upgrader with data in request: '{\\\"foo\\\":bar}'\"}", + + 400); + + // Patch configuration + tester.assertResponse(new Request("http://localhost:8080/controller/v1/jobs/upgrader", + "{\"upgradesPerMinute\":42.0}", Request.Method.PATCH), + "{\"upgradesPerMinute\":42.0,\"ignoreConfidence\":false}", 200); - // Set configuration + // Patch configuration tester.assertResponse(new Request("http://localhost:8080/controller/v1/jobs/upgrader", - "{\"upgradesPerMinute\":42}", Request.Method.PATCH), - "{\"upgradesPerMinute\":42.0}", + "{\"ignoreConfidence\":true}", Request.Method.PATCH), + "{\"upgradesPerMinute\":42.0,\"ignoreConfidence\":true}", 200); } diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/versions/VersionStatusTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/versions/VersionStatusTest.java index 17935906186..bc31079cfe0 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/versions/VersionStatusTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/versions/VersionStatusTest.java @@ -233,6 +233,49 @@ public class VersionStatusTest { VespaVersion.Confidence.broken, confidence(tester.controller(), version3)); } + + @Test + public void testIgnoreConfigdeince() { + DeploymentTester tester = new DeploymentTester(); + + Version version0 = new Version("5.0"); + tester.upgradeSystem(version0); + + // Setup applications - all running on version0 + Application canary0 = tester.createAndDeploy("canary0", 1, "canary"); + Application canary1 = tester.createAndDeploy("canary1", 2, "canary"); + Application default0 = tester.createAndDeploy("default0", 3, "default"); + Application default1 = tester.createAndDeploy("default1", 4, "default"); + Application default2 = tester.createAndDeploy("default2", 5, "default"); + Application default3 = tester.createAndDeploy("default3", 6, "default"); + Application default4 = tester.createAndDeploy("default4", 7, "default"); + + // New version is released + Version version1 = new Version("5.1"); + tester.upgradeSystem(version1); + + // All canaries upgrade successfully, 1 default apps ok, 3 default apps fail + tester.completeUpgrade(canary0, version1, "canary"); + tester.completeUpgrade(canary1, version1, "canary"); + tester.upgradeSystem(version1); + tester.completeUpgrade(default0, version1, "default"); + tester.completeUpgradeWithError(default1, version1, "default", stagingTest); + tester.completeUpgradeWithError(default2, version1, "default", stagingTest); + tester.completeUpgradeWithError(default3, version1, "default", stagingTest); + tester.completeUpgradeWithError(default4, version1, "default", stagingTest); + tester.updateVersionStatus(); + + assertEquals("Canaries have upgraded, 1 of 4 default apps failing: Broken", + Confidence.broken, confidence(tester.controller(), version1)); + + // Same as above, but ignore confidence calculations, will force normal confidence + tester.controllerTester().curator().writeIgnoreConfidence(true); + tester.updateVersionStatus(); + assertEquals("Canaries have upgraded, 1 of 4 default apps failing, but confidence ignored: Low", + Confidence.normal, confidence(tester.controller(), version1)); + tester.controllerTester().curator().writeIgnoreConfidence(false); + } + @Test public void testComputeIgnoresVersionWithUnknownGitMetadata() { ControllerTester tester = new ControllerTester(); |