diff options
author | Jon Marius Venstad <venstad@gmail.com> | 2019-09-29 08:44:59 +0200 |
---|---|---|
committer | Jon Marius Venstad <venstad@gmail.com> | 2019-09-29 08:44:59 +0200 |
commit | 54f5946704857dbf682fbc85237fbe1dfc297738 (patch) | |
tree | 293edf6bbfd0299bc563563aff4113ae07328071 | |
parent | 72ee0653a18ce4f6d472c431611edb3cc9d2b992 (diff) |
Read from new application paths, and change deletion of instances, with updated tests
4 files changed, 122 insertions, 65 deletions
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java index bb28839cb79..ba9f8e809c8 100644 --- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java +++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java @@ -742,7 +742,6 @@ public class ApplicationController { application.get().require(applicationId.instance()).deployments().keySet().stream().map(ZoneId::toString) .sorted().collect(Collectors.joining(", "))); - curator.removeApplication(applicationId); applicationStore.removeAll(applicationId); applicationStore.removeAll(TesterId.of(applicationId)); @@ -755,6 +754,7 @@ public class ApplicationController { controller.nameServiceForwarder().removeRecords(Record.Type.CNAME, RecordName.from(name), Priority.normal); }); }); + curator.storeWithoutInstance(application.without(applicationId.instance()).get(), applicationId); log.info("Deleted " + application); }); 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 a3de98a9582..76a146fb1e6 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 @@ -15,7 +15,6 @@ import com.yahoo.vespa.config.SlimeUtils; import com.yahoo.vespa.curator.Curator; import com.yahoo.vespa.curator.Lock; import com.yahoo.vespa.hosted.controller.Application; -import com.yahoo.vespa.hosted.controller.Instance; import com.yahoo.vespa.hosted.controller.api.integration.certificates.ApplicationCertificate; import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; import com.yahoo.vespa.hosted.controller.api.integration.deployment.RunId; @@ -345,8 +344,7 @@ public class CuratorDb { } public Optional<Application> readApplication(TenantAndApplicationId application) { - List<Instance> instances = readInstances(id -> TenantAndApplicationId.from(id).equals(application)); - return Application.aggregate(instances); + return readSlime(applicationPath(application)).map(applicationSerializer::fromSlime); } public List<Application> readApplications() { @@ -357,41 +355,28 @@ public class CuratorDb { return readApplications(application -> application.tenant().equals(name)); } - private Stream<TenantAndApplicationId> readTenantAndApplicationIds() { - return readInstanceIds().map(TenantAndApplicationId::from).distinct(); - } - private List<Application> readApplications(Predicate<TenantAndApplicationId> applicationFilter) { - return readTenantAndApplicationIds().filter(applicationFilter) - .sorted() - .map(this::readApplication) - .flatMap(Optional::stream) - .collect(Collectors.toUnmodifiableList()); - } - - private Optional<Instance> readInstance(ApplicationId application) { - return readSlime(oldApplicationPath(application)).map(instanceSerializer::fromSlime); + return readApplicationIds().filter(applicationFilter) + .sorted() + .map(this::readApplication) + .flatMap(Optional::stream) + .collect(Collectors.toUnmodifiableList()); } - private Stream<ApplicationId> readInstanceIds() { + // TODO jonmv: Clear out old instance data here + private Stream<TenantAndApplicationId> readApplicationIds() { return curator.getChildren(applicationRoot).stream() - .filter(id -> id.split(":").length == 3) - .map(ApplicationId::fromSerializedForm); + .filter(id -> id.split(":").length == 2) + .map(TenantAndApplicationId::fromSerialized); } - private List<Instance> readInstances(Predicate<ApplicationId> applicationFilter) { - return readInstanceIds().filter(applicationFilter) - .sorted() - .map(this::readInstance) - .flatMap(Optional::stream) - .collect(Collectors.toUnmodifiableList()); - } - - public void removeApplication(ApplicationId id) { - // WARNING: This is part of a multi-step data move operation, so don't touch!!! - curator.delete(oldApplicationPath(id)); - if (readApplication(TenantAndApplicationId.from(id)).isEmpty()) - curator.delete(applicationPath(TenantAndApplicationId.from(id))); + // TODO jonmv: Refactor when instance split operation is done + public void storeWithoutInstance(Application application, ApplicationId instanceId) { + curator.delete(oldApplicationPath(instanceId)); + if (application.instances().isEmpty()) + curator.delete(applicationPath(application.id())); + else + writeApplication(application); } /** @@ -415,7 +400,7 @@ public class CuratorDb { * Remove everything under instance root DONE * Stop locking applications on instance level DONE * - * Read Application with instances from application path (with filter) + * Read Application with instances from application path (with filter) DONE * * Stop writing Instance to old application path * Remove unused parts of Instance (Used only for legacy serialization) diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java index 5d82225c9d5..075451f60bf 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/ControllerTest.java @@ -853,18 +853,19 @@ public class ControllerTest { MockCuratorDb newDb = new MockCuratorDb(); OldMockCuratorDb oldDb = new OldMockCuratorDb(newDb.curator()); - oldDb.writeInstance(old1); - oldDb.writeInstance(old2); + oldDb.writeApplication(Application.aggregate(List.of(old1, old2)).orElseThrow()); - Application application = newDb.readApplication(TenantAndApplicationId.from("t1", "a1")).orElseThrow(); + Application application = oldDb.readApplication(TenantAndApplicationId.from("t1", "a1")).orElseThrow(); Instance new1 = application.legacy(InstanceName.defaultName()); String new1Serialized = new String(JsonFormat.toJsonBytes(instanceSerializer.toSlime(new1)), UTF_8); assertEquals(old1Serialized, new1Serialized); LockedApplication locked = new LockedApplication(application, newDb.lock(application.id())); - newDb.writeApplication(locked.with(new ApplicationMetrics(8, 9)).get()); - Instance mod1 = oldDb.readInstance(old1.id()).orElseThrow(); - Instance mod2 = oldDb.readInstance(old2.id()).orElseThrow(); + oldDb.writeApplication(locked.with(new ApplicationMetrics(8, 9)).get()); + + Application newApp = newDb.readApplication(application.id()).orElseThrow(); + Instance mod1 = newApp.legacy(old1.name()); + Instance mod2 = newApp.legacy(old2.name()); old1 = old1.with(new ApplicationMetrics(8, 9)); old1Serialized = new String(JsonFormat.toJsonBytes(instanceSerializer.toSlime(old1)), UTF_8); @@ -878,12 +879,14 @@ public class ControllerTest { assertEquals(old2.deployments(), mod2.deployments()); assertEquals(old2.deploymentJobs().jobStatus(), mod2.deploymentJobs().jobStatus()); - newDb.removeApplication(old1.id()); + application = new LockedApplication(application, newDb.lock(application.id())).without(old1.name()).get(); + newDb.storeWithoutInstance(application, old1.id()); assertEquals(1, newDb.readApplication(application.id()).orElseThrow().instances().size()); - newDb.removeApplication(old2.id()); + assertEquals(1, oldDb.readApplication(application.id()).orElseThrow().instances().size()); + application = new LockedApplication(application, newDb.lock(application.id())).without(old2.name()).get(); + newDb.storeWithoutInstance(application, old2.id()); assertTrue(newDb.readApplication(application.id()).isEmpty()); - assertTrue(oldDb.readInstance(old1.id()).isEmpty()); - assertTrue(oldDb.readInstance(old2.id()).isEmpty()); + assertTrue(oldDb.readApplication(application.id()).isEmpty()); } private void runUpgrade(DeploymentTester tester, ApplicationId application, ApplicationVersion version) { diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/OldCuratorDb.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/OldCuratorDb.java index 8aea72a9ac4..f7fdd91d74e 100644 --- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/OldCuratorDb.java +++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/persistence/OldCuratorDb.java @@ -6,6 +6,7 @@ import com.google.inject.Inject; import com.yahoo.component.Version; import com.yahoo.config.provision.ApplicationId; import com.yahoo.config.provision.HostName; +import com.yahoo.config.provision.InstanceName; import com.yahoo.config.provision.TenantName; import com.yahoo.config.provision.zone.ZoneId; import com.yahoo.path.Path; @@ -13,11 +14,13 @@ import com.yahoo.slime.Slime; import com.yahoo.vespa.config.SlimeUtils; import com.yahoo.vespa.curator.Curator; import com.yahoo.vespa.curator.Lock; +import com.yahoo.vespa.hosted.controller.Application; import com.yahoo.vespa.hosted.controller.Instance; import com.yahoo.vespa.hosted.controller.api.integration.certificates.ApplicationCertificate; import com.yahoo.vespa.hosted.controller.api.integration.deployment.JobType; import com.yahoo.vespa.hosted.controller.api.integration.deployment.RunId; import com.yahoo.vespa.hosted.controller.application.RoutingPolicy; +import com.yahoo.vespa.hosted.controller.application.TenantAndApplicationId; import com.yahoo.vespa.hosted.controller.auditlog.AuditLog; import com.yahoo.vespa.hosted.controller.deployment.Run; import com.yahoo.vespa.hosted.controller.deployment.Step; @@ -63,7 +66,7 @@ import static java.util.stream.Collectors.collectingAndThen; */ public class OldCuratorDb { - private static final Logger log = Logger.getLogger(OldCuratorDb.class.getName()); + private static final Logger log = Logger.getLogger(CuratorDb.class.getName()); private static final Duration deployLockTimeout = Duration.ofMinutes(30); private static final Duration defaultLockTimeout = Duration.ofMinutes(5); private static final Duration defaultTryLockTimeout = Duration.ofSeconds(1); @@ -83,6 +86,7 @@ public class OldCuratorDb { private final ControllerVersionSerializer controllerVersionSerializer = new ControllerVersionSerializer(); private final ConfidenceOverrideSerializer confidenceOverrideSerializer = new ConfidenceOverrideSerializer(); private final TenantSerializer tenantSerializer = new TenantSerializer(); + private final ApplicationSerializer applicationSerializer = new ApplicationSerializer(); private final InstanceSerializer instanceSerializer = new InstanceSerializer(); private final RunSerializer runSerializer = new RunSerializer(); private final OsVersionSerializer osVersionSerializer = new OsVersionSerializer(); @@ -133,6 +137,10 @@ public class OldCuratorDb { return lock(lockPath(name), defaultLockTimeout.multipliedBy(2)); } + public Lock lock(TenantAndApplicationId id) { + return lock(lockPath(id), defaultLockTimeout.multipliedBy(2)); + } + public Lock lock(ApplicationId id) { return lock(lockPath(id), defaultLockTimeout.multipliedBy(2)); } @@ -329,45 +337,97 @@ public class OldCuratorDb { curator.delete(tenantPath(name)); } - // -------------- Application --------------------------------------------- + // -------------- Applications --------------------------------------------- - public void writeInstance(Instance instance) { - curator.set(applicationPath(instance.id()), asJson(instanceSerializer.toSlime(instance))); - curator.set(instancePath(instance.id()), asJson(instanceSerializer.toSlime(instance))); + public void writeApplication(Application application) { + curator.set(applicationPath(application.id()), asJson(applicationSerializer.toSlime(application))); + for (InstanceName name : application.instances().keySet()) { + curator.set(oldApplicationPath(application.id().instance(name)), + asJson(instanceSerializer.toSlime(application.legacy(name)))); + } } - public Optional<Instance> readInstance(ApplicationId application) { - return readSlime(applicationPath(application)).map(instanceSerializer::fromSlime); + public Optional<Application> readApplication(TenantAndApplicationId application) { + List<Instance> instances = readInstances(id -> TenantAndApplicationId.from(id).equals(application)); + return Application.aggregate(instances); } - public List<Instance> readInstances() { - return readInstances(ignored -> true); + public List<Application> readApplications() { + return readApplications(ignored -> true); } - public List<Instance> readInstances(TenantName name) { - return readInstances(application -> application.tenant().equals(name)); + public List<Application> readApplications(TenantName name) { + return readApplications(application -> application.tenant().equals(name)); + } + + private Stream<TenantAndApplicationId> readTenantAndApplicationIds() { + return readInstanceIds().map(TenantAndApplicationId::from).distinct(); + } + + private List<Application> readApplications(Predicate<TenantAndApplicationId> applicationFilter) { + return readTenantAndApplicationIds().filter(applicationFilter) + .sorted() + .map(this::readApplication) + .flatMap(Optional::stream) + .collect(Collectors.toUnmodifiableList()); + } + + private Optional<Instance> readInstance(ApplicationId application) { + return readSlime(oldApplicationPath(application)).map(instanceSerializer::fromSlime); } private Stream<ApplicationId> readInstanceIds() { return curator.getChildren(applicationRoot).stream() .filter(id -> id.split(":").length == 3) - .distinct() .map(ApplicationId::fromSerializedForm); } - private List<Instance> readInstances(Predicate<ApplicationId> instanceFilter) { - return readInstanceIds().filter(instanceFilter) + private List<Instance> readInstances(Predicate<ApplicationId> applicationFilter) { + return readInstanceIds().filter(applicationFilter) + .sorted() .map(this::readInstance) .flatMap(Optional::stream) .collect(Collectors.toUnmodifiableList()); } - public void removeInstance(ApplicationId id) { + public void removeApplication(ApplicationId id) { // WARNING: This is part of a multi-step data move operation, so don't touch!!! - curator.delete(applicationPath(id)); - curator.delete(instancePath(id)); + curator.delete(oldApplicationPath(id)); + if (readApplication(TenantAndApplicationId.from(id)).isEmpty()) + curator.delete(applicationPath(TenantAndApplicationId.from(id))); } + public void clearInstanceRoot() { + curator.delete(instanceRoot); + } + + /** + * Migration plan: + * + * Add filter for reading only Instance from old application path RELEASED + * Write Instance to instance and old application path RELEASED + * + * Lock on application level for instance mutations MERGED + * + * Write Instance to instance and application and old application paths DONE TO CHANGE DONE + * Read Instance from instance path DONE TO REMOVE DONE + * Duplicate Application from Instance, with helper classes DONE + * Write Application to instance and application and old application paths DONE TO CHANGE DONE + * Read Application from instance path DONE TO REMOVE DONE + * Use Application where applicable DONE !!! + * Lock instances and application on same level: tenant + application DONE TO CHANGE DONE + * When reading an application, read all instances, and aggregate them DONE + * Write application with instances to application path DONE + * Write all instances of an application to old application path DONE + * Remove everything under instance root DONE + * + * Read Application with instances from application path (with filter) + * Stop locking applications on instance level + * + * Stop writing Instance to old application path + * Remove unused parts of Instance (Used only for legacy serialization) + */ + // -------------- Job Runs ------------------------------------------------ public void writeLastRun(Run run) { @@ -503,10 +563,14 @@ public class OldCuratorDb { .append(tenant.value()); } - private Path lockPath(ApplicationId application) { + private Path lockPath(TenantAndApplicationId application) { return lockPath(application.tenant()) - .append(application.application().value()) - .append(application.instance().value()); + .append(application.application().value()); + } + + private Path lockPath(ApplicationId instance) { + return lockPath(TenantAndApplicationId.from(instance)) + .append(instance.instance().value()); } private Path lockPath(ApplicationId application, ZoneId zone) { @@ -583,7 +647,11 @@ public class OldCuratorDb { return tenantRoot.append(name.value()); } - private static Path applicationPath(ApplicationId application) { + private static Path applicationPath(TenantAndApplicationId id) { + return applicationRoot.append(id.serialized()); + } + + private static Path oldApplicationPath(ApplicationId application) { return applicationRoot.append(application.serializedForm()); } @@ -616,3 +684,4 @@ public class OldCuratorDb { } } + |