From 8fdf427104ce67b32bd81ac725b02d1a1f3821a8 Mon Sep 17 00:00:00 2001 From: Harald Musum Date: Tue, 9 Apr 2019 08:11:19 +0200 Subject: Revert "Register applications in TenantApplications before activating their s…" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../server/application/TenantApplications.java | 15 ++++-------- .../config/server/session/SessionFactoryImpl.java | 1 - .../server/application/TenantApplicationsTest.java | 17 ++++++------- .../http/v2/ApplicationContentHandlerTest.java | 2 -- .../config/server/http/v2/HostHandlerTest.java | 1 - .../http/v2/ListApplicationsHandlerTest.java | 28 +++++++++++----------- .../server/http/v2/SessionActiveHandlerTest.java | 3 --- .../server/http/v2/SessionCreateHandlerTest.java | 2 -- .../config/server/session/LocalSessionTest.java | 4 +--- 9 files changed, 29 insertions(+), 44 deletions(-) (limited to 'configserver') diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/application/TenantApplications.java b/configserver/src/main/java/com/yahoo/vespa/config/server/application/TenantApplications.java index 92add4d053b..656030bb8d2 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/application/TenantApplications.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/application/TenantApplications.java @@ -32,7 +32,6 @@ import java.util.stream.Collectors; * of whatever session may be activated next, if any, and /lock is used for synchronizing writes to all these paths. * * @author Ulf Lilleengen - * @author jonmv */ public class TenantApplications { @@ -74,7 +73,6 @@ public class TenantApplications { public List activeApplications() { return curator.getChildren(applicationsPath).stream() .filter(this::isValid) - .sorted() .map(ApplicationId::fromSerializedForm) .filter(id -> activeSessionOf(id).isPresent()) .collect(Collectors.toUnmodifiableList()); @@ -111,14 +109,11 @@ public class TenantApplications { * @param sessionId Id of the session containing the application package for this id. */ public Transaction createPutTransaction(ApplicationId applicationId, long sessionId) { - return new CuratorTransaction(curator).add(CuratorOperations.setData(applicationPath(applicationId).getAbsolute(), Utf8.toAsciiBytes(sessionId))); - } - - /** - * Creates a node for the given application, marking its existence. - */ - public void createApplication(ApplicationId id) { - curator.create(applicationPath(id)); + if (curator.exists(applicationPath(applicationId))) { + return new CuratorTransaction(curator).add(CuratorOperations.setData(applicationPath(applicationId).getAbsolute(), Utf8.toAsciiBytes(sessionId))); + } else { + return new CuratorTransaction(curator).add(CuratorOperations.create(applicationPath(applicationId).getAbsolute(), Utf8.toAsciiBytes(sessionId))); + } } /** diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionFactoryImpl.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionFactoryImpl.java index cee18580094..d8ba5890545 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionFactoryImpl.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionFactoryImpl.java @@ -139,7 +139,6 @@ public class SessionFactoryImpl implements SessionFactory, LocalSessionLoader { private LocalSession create(File applicationFile, ApplicationId applicationId, long currentlyActiveSessionId, boolean internalRedeploy, TimeoutBudget timeoutBudget) { - applicationRepo.createApplication(applicationId); long sessionId = sessionCounter.nextSessionId(); Path sessionIdPath = sessionsPath.append(String.valueOf(sessionId)); log.log(LogLevel.DEBUG, TenantRepository.logPre(tenant) + "Next session id is " + sessionId + " , sessionIdPath=" + sessionIdPath.getAbsolute()); diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/application/TenantApplicationsTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/application/TenantApplicationsTest.java index 69c88dc0275..01a7d5e0239 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/application/TenantApplicationsTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/application/TenantApplicationsTest.java @@ -41,10 +41,10 @@ public class TenantApplicationsTest { TenantApplications repo = createZKAppRepo(); List applications = repo.activeApplications(); assertThat(applications.size(), is(2)); - assertThat(applications.get(0).application().value(), is("bar")); - assertThat(applications.get(1).application().value(), is("foo")); - assertThat(repo.requireActiveSessionOf(applications.get(0)), is(4L)); - assertThat(repo.requireActiveSessionOf(applications.get(1)), is(3L)); + assertThat(applications.get(0).application().value(), is("foo")); + assertThat(applications.get(1).application().value(), is("bar")); + assertThat(repo.requireActiveSessionOf(applications.get(0)), is(3L)); + assertThat(repo.requireActiveSessionOf(applications.get(1)), is(4L)); } @Test @@ -77,7 +77,6 @@ public class TenantApplicationsTest { public void require_that_application_ids_can_be_written() throws Exception { TenantApplications repo = createZKAppRepo(); ApplicationId myapp = createApplicationId("myapp"); - repo.createApplication(myapp); repo.createPutTransaction(myapp, 3l).commit(); String path = TenantRepository.getApplicationsPath(tenantName).append(myapp.serializedForm()).getAbsolute(); assertTrue(curatorFramework.checkExists().forPath(path) != null); @@ -92,8 +91,6 @@ public class TenantApplicationsTest { TenantApplications repo = createZKAppRepo(); ApplicationId id1 = createApplicationId("myapp"); ApplicationId id2 = createApplicationId("myapp2"); - repo.createApplication(id1); - repo.createApplication(id2); repo.createPutTransaction(id1, 1).commit(); repo.createPutTransaction(id2, 1).commit(); assertThat(repo.activeApplications().size(), is(2)); @@ -129,7 +126,11 @@ public class TenantApplicationsTest { } private static ApplicationId createApplicationId(String name) { - return ApplicationId.from(tenantName.value(), name, "myinst"); + return new ApplicationId.Builder() + .tenant(tenantName.value()) + .applicationName(name) + .instanceName("myinst") + .build(); } private void writeApplicationData(ApplicationId applicationId, long sessionId) throws Exception { diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/ApplicationContentHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/ApplicationContentHandlerTest.java index c6a8e1f2f9d..3d34a4eeaf5 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/ApplicationContentHandlerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/ApplicationContentHandlerTest.java @@ -56,13 +56,11 @@ public class ApplicationContentHandlerTest extends ContentHandlerTestBase { session2 = new MockSession(2l, FilesApplicationPackage.fromFile(new File("src/test/apps/content"))); Tenant tenant1 = tenantRepository.getTenant(tenantName1); tenant1.getLocalSessionRepo().addSession(session2); - tenant1.getApplicationRepo().createApplication(idTenant1); tenant1.getApplicationRepo().createPutTransaction(idTenant1, 2l).commit(); MockSession session3 = new MockSession(3l, FilesApplicationPackage.fromFile(new File("src/test/apps/content2"))); Tenant tenant2 = tenantRepository.getTenant(tenantName2); tenant2.getLocalSessionRepo().addSession(session3); - tenant2.getApplicationRepo().createApplication(idTenant2); tenant2.getApplicationRepo().createPutTransaction(idTenant2, 3l).commit(); handler = new ApplicationHandler(ApplicationHandler.testOnlyContext(), diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/HostHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/HostHandlerTest.java index 1db70956407..fb75e91dfd6 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/HostHandlerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/HostHandlerTest.java @@ -46,7 +46,6 @@ public class HostHandlerTest { private HostHandler hostHandler; static void addMockApplication(Tenant tenant, ApplicationId applicationId, long sessionId) { - tenant.getApplicationRepo().createApplication(applicationId); tenant.getApplicationRepo().createPutTransaction(applicationId, sessionId).commit(); ApplicationPackage app = FilesApplicationPackage.fromFile(testApp); tenant.getLocalSessionRepo().addSession(new SessionHandlerTest.MockSession(sessionId, app, applicationId)); diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/ListApplicationsHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/ListApplicationsHandlerTest.java index f97bc443a38..b34f7a0c487 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/ListApplicationsHandlerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/ListApplicationsHandlerTest.java @@ -51,17 +51,17 @@ public class ListApplicationsHandlerTest { final String url = "http://myhost:14000/application/v2/tenant/mytenant/application/"; assertResponse(url, Response.Status.OK, "[]"); - ApplicationId id1 = ApplicationId.from("mytenant", "foo", "quux"); - applicationRepo.createApplication(id1); - applicationRepo.createPutTransaction(id1, 1).commit(); + applicationRepo.createPutTransaction( + new ApplicationId.Builder().tenant("tenant").applicationName("foo").instanceName("quux").build(), + 1).commit(); assertResponse(url, Response.Status.OK, "[\"" + url + "foo/environment/dev/region/us-east/instance/quux\"]"); - ApplicationId id2 = ApplicationId.from("mytenant", "bali", "quux"); - applicationRepo.createApplication(id2); - applicationRepo.createPutTransaction(id2, 1).commit(); + applicationRepo.createPutTransaction( + new ApplicationId.Builder().tenant("tenant").applicationName("bali").instanceName("quux").build(), + 1).commit(); assertResponse(url, Response.Status.OK, - "[\"" + url + "bali/environment/dev/region/us-east/instance/quux\"," + - "\"" + url + "foo/environment/dev/region/us-east/instance/quux\"]" + "[\"" + url + "foo/environment/dev/region/us-east/instance/quux\"," + + "\"" + url + "bali/environment/dev/region/us-east/instance/quux\"]" ); } @@ -82,12 +82,12 @@ public class ListApplicationsHandlerTest { @Test public void require_that_listing_works_with_multiple_tenants() throws Exception { - ApplicationId id1 = ApplicationId.from("mytenant", "foo", "quux"); - applicationRepo.createApplication(id1); - applicationRepo.createPutTransaction(id1, 1).commit(); - ApplicationId id2 = ApplicationId.from("foobar", "quux", "foo"); - applicationRepo2.createApplication(id2); - applicationRepo2.createPutTransaction(id2, 1).commit(); + applicationRepo.createPutTransaction(new ApplicationId.Builder() + .tenant("tenant") + .applicationName("foo").instanceName("quux").build(), 1).commit(); + applicationRepo2.createPutTransaction(new ApplicationId.Builder() + .tenant("tenant") + .applicationName("quux").instanceName("foo").build(), 1).commit(); String url = "http://myhost:14000/application/v2/tenant/mytenant/application/"; assertResponse(url, Response.Status.OK, "[\"" + url + "foo/environment/dev/region/us-east/instance/quux\"]"); diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionActiveHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionActiveHandlerTest.java index 858d1e0eaa7..380b76c30af 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionActiveHandlerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionActiveHandlerTest.java @@ -323,9 +323,6 @@ public class SessionActiveHandlerTest extends SessionHandlerTest { Optional.of(AllocatedHosts.withHosts(Collections.singleton(new HostSpec("bar", Collections.emptyList()))))); session = createRemoteSession(sessionId, initialStatus, zkClient); addLocalSession(sessionId, deployData, zkClient); - tenantRepository.getTenant(tenantName).getApplicationRepo().createApplication(ApplicationId.from(tenantName.value(), - deployData.getApplicationName(), - InstanceName.defaultName().value())); metaData = localRepo.getSession(sessionId).getMetaData(); actResponse = handler.handle(SessionHandlerTest.createTestRequest(pathPrefix, HttpRequest.Method.PUT, Cmd.ACTIVE, sessionId, subPath)); return this; diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionCreateHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionCreateHandlerTest.java index 0946ef3992c..bc509fcd802 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionCreateHandlerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/v2/SessionCreateHandlerTest.java @@ -194,7 +194,6 @@ public class SessionCreateHandlerTest extends SessionHandlerTest { .applicationName("foo") .instanceName("quux") .build(); - applicationRepo.createApplication(fooId); applicationRepo.createPutTransaction(fooId, 2).commit(); assertFromParameter("3", "http://myhost:40555/application/v2/tenant/" + tenant + "/application/foo/environment/test/region/baz/instance/quux"); localSessionRepo.addSession(new SessionHandlerTest.MockSession(5l, FilesApplicationPackage.fromFile(testApp))); @@ -203,7 +202,6 @@ public class SessionCreateHandlerTest extends SessionHandlerTest { .applicationName("foobio") .instanceName("quux") .build(); - applicationRepo.createApplication(bioId); applicationRepo.createPutTransaction(bioId, 5).commit(); assertFromParameter("6", "http://myhost:40555/application/v2/tenant/" + tenant + "/application/foobio/environment/staging/region/baz/instance/quux"); } diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/session/LocalSessionTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/session/LocalSessionTest.java index a4432dcbfcd..e7db4dcf58f 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/session/LocalSessionTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/session/LocalSessionTest.java @@ -196,14 +196,12 @@ public class LocalSessionTest { zkClient.write(Collections.singletonMap(new Version(0, 0, 0), new MockFileRegistry())); File sessionDir = new File(tenantFileSystemDirs.sessionsPath(), String.valueOf(sessionId)); sessionDir.createNewFile(); - TenantApplications applications = TenantApplications.create(curator, new MockReloadHandler(), tenant); - applications.createApplication(zkc.readApplicationId()); return new LocalSession(tenant, sessionId, preparer, new SessionContext( FilesApplicationPackage.fromFile(testApp), zkc, sessionDir, - applications, + TenantApplications.create(curator, new MockReloadHandler(), tenant), new HostRegistry<>(), superModelGenerationCounter, flagSource)); -- cgit v1.2.3