Revert "Try to reduce contention when bootstrapping tenants [run-systemtest]"

8 files changed, 26 insertions, 25 deletions

diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/TenantRepository.java b/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/TenantRepository.java
index 365f68b7aea..15f30dc6ece 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/TenantRepository.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/TenantRepository.java
@@ -216,10 +216,8 @@ public class TenantRepository {
     }
 
     public synchronized Tenant addTenant(TenantName tenantName) {
-        if (tenants.containsKey(tenantName)) throw new IllegalArgumentException("Tenant '" + tenantName + "' already exists");
-
         writeTenantPath(tenantName);
-        return getOrCreateTenant(tenantName, clock.instant());
+        return createTenant(tenantName, clock.instant());
     }
 
     public void createAndWriteTenantMetaData(Tenant tenant) {
@@ -288,7 +286,7 @@ public class TenantRepository {
     }
 
     // Use when bootstrapping an existing tenant based on ZooKeeper data
-    protected void bootstrapTenant(TenantName tenantName) {
+    protected synchronized void bootstrapTenant(TenantName tenantName) {
         createTenant(tenantName, readCreatedTimeFromZooKeeper(tenantName));
     }
 
@@ -300,18 +298,14 @@ public class TenantRepository {
             return clock.instant();
     }
 
-    private Tenant getOrCreateTenant(TenantName tenantName, Instant created) {
+    // Creates tenant and all its dependencies. This also includes loading active applications
+    private Tenant createTenant(TenantName tenantName, Instant created) {
         if (tenants.containsKey(tenantName)) {
             Tenant tenant = getTenant(tenantName);
             createAndWriteTenantMetaData(tenant);
             return tenant;
-        } else {
-            return createTenant(tenantName, created);
         }
-    }
 
-    // Creates tenant and all its dependencies. This also includes loading active applications
-    private Tenant createTenant(TenantName tenantName, Instant created) {
         TenantApplications applicationRepo =
                 new TenantApplications(tenantName,
                                        curator,
@@ -501,7 +495,7 @@ public class TenantRepository {
             case CHILD_ADDED:
                 TenantName t1 = getTenantNameFromEvent(event);
                 if ( ! tenants.containsKey(t1))
-                    zkWatcherExecutor.execute(t1, () -> getOrCreateTenant(t1, readCreatedTimeFromZooKeeper(t1)));
+                    zkWatcherExecutor.execute(t1, () -> bootstrapTenant(t1));
                 break;
             case CHILD_REMOVED:
                 TenantName t2 = getTenantNameFromEvent(event);
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/ApplicationRepositoryTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/ApplicationRepositoryTest.java
index fdb36da1990..dae79486775 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/ApplicationRepositoryTest.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/ApplicationRepositoryTest.java
@@ -130,6 +130,7 @@ public class ApplicationRepositoryTest {
                 .withFileDistributionFactory(new MockFileDistributionFactory(configserverConfig))
                 .withFlagSource(flagSource)
                 .build();
+        tenantRepository.addTenant(TenantRepository.HOSTED_VESPA_TENANT);
         tenantRepository.addTenant(tenant1);
         tenantRepository.addTenant(tenant2);
         tenantRepository.addTenant(tenant3);
@@ -264,8 +265,9 @@ public class ApplicationRepositoryTest {
 
     @Test
     public void getLogsForHostname() {
-        deployApp(testAppLogServerWithContainer, new PrepareParams.Builder().applicationId(applicationId()).build());
-        HttpResponse response = applicationRepository.getLogs(applicationId(), Optional.of("localhost"), "");
+        ApplicationId applicationId = ApplicationId.from("hosted-vespa", "tenant-host", "default");
+        deployApp(testAppLogServerWithContainer, new PrepareParams.Builder().applicationId(applicationId).build());
+        HttpResponse response = applicationRepository.getLogs(applicationId, Optional.of("localhost"), "");
         assertEquals(200, response.getStatus());
     }
 
@@ -278,6 +280,7 @@ public class ApplicationRepositoryTest {
         // Add file reference that is not in use, but should not be deleted (not older than 14 days)
         File filereferenceDir2 = createFilereferenceOnDisk(new File(fileReferencesDir, "baz"), Instant.now());
 
+        tenantRepository.addTenant(tenant1);
         applicationRepository = new ApplicationRepository.Builder()
                 .withTenantRepository(tenantRepository)
                 .withProvisioner(provisioner)
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/HttpGetConfigHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/HttpGetConfigHandlerTest.java
index b924d350c13..176686e5843 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/HttpGetConfigHandlerTest.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/HttpGetConfigHandlerTest.java
@@ -56,6 +56,7 @@ public class HttpGetConfigHandlerTest {
         TenantRepository tenantRepository = new TestTenantRepository.Builder()
                 .withConfigserverConfig(configserverConfig)
                 .build();
+        tenantRepository.addTenant(tenant);
         ApplicationRepository applicationRepository = new ApplicationRepository.Builder()
                 .withTenantRepository(tenantRepository)
                 .withProvisioner(new MockProvisioner())
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/http/HttpListConfigsHandlerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/http/HttpListConfigsHandlerTest.java
index b9d09996bc5..ff33eb70bca 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/http/HttpListConfigsHandlerTest.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/http/HttpListConfigsHandlerTest.java
@@ -3,6 +3,7 @@ package com.yahoo.vespa.config.server.http;
 
 import com.yahoo.cloud.config.ConfigserverConfig;
 import com.yahoo.config.provision.ApplicationId;
+import com.yahoo.config.provision.TenantName;
 import com.yahoo.container.jdisc.HttpRequest;
 import com.yahoo.container.jdisc.HttpResponse;
 import com.yahoo.vespa.config.ConfigKey;
@@ -39,6 +40,7 @@ import static org.junit.Assert.assertTrue;
  */
 public class HttpListConfigsHandlerTest {
 
+    private static final TenantName tenant = TenantName.from("default");
     private static final String baseUri = "http://foo.com:8080/config/v1/";
     private final static File testApp = new File("src/test/resources/deploy/validapp");
     private static final ApplicationId applicationId = ApplicationId.defaultId();
@@ -59,6 +61,7 @@ public class HttpListConfigsHandlerTest {
         TenantRepository tenantRepository = new TestTenantRepository.Builder()
                 .withConfigserverConfig(configserverConfig)
                 .build();
+        tenantRepository.addTenant(tenant);
         ApplicationRepository applicationRepository = new ApplicationRepository.Builder()
                 .withTenantRepository(tenantRepository)
                 .withProvisioner(new MockProvisioner())
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/maintenance/MaintainerTester.java b/configserver/src/test/java/com/yahoo/vespa/config/server/maintenance/MaintainerTester.java
index 97e581e1969..3afd2f185a1 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/maintenance/MaintainerTester.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/maintenance/MaintainerTester.java
@@ -43,16 +43,15 @@ class MaintainerTester {
                 .withClock(clock)
                 .withHostProvisionerProvider(HostProvisionerProvider.withProvisioner(provisioner, true))
                 .withConfigserverConfig(configserverConfig)
-                .withCurator(curator)
                 .withModelFactoryRegistry(new ModelFactoryRegistry(List.of(new DeployTester.CountingModelFactory(clock))))
                 .build();
         applicationRepository = new ApplicationRepository.Builder()
+                .withTenantRepository(tenantRepository)
+                .withProvisioner(provisioner)
+                .withOrchestrator(new OrchestratorMock())
+                .withLogRetriever(new MockLogRetriever())
                 .withClock(clock)
                 .withConfigserverConfig(configserverConfig)
-                .withLogRetriever(new MockLogRetriever())
-                .withOrchestrator(new OrchestratorMock())
-                .withProvisioner(provisioner)
-                .withTenantRepository(tenantRepository)
                 .build();
     }
 
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/maintenance/TenantsMaintainerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/maintenance/TenantsMaintainerTest.java
index 97821e9c0d6..a072bf62852 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/maintenance/TenantsMaintainerTest.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/maintenance/TenantsMaintainerTest.java
@@ -1,4 +1,4 @@
-// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.config.server.maintenance;
 
 import com.yahoo.config.provision.ApplicationId;
@@ -34,11 +34,12 @@ public class TenantsMaintainerTest {
         ApplicationRepository applicationRepository = tester.applicationRepository();
         File applicationPackage = new File("src/test/apps/hosted");
 
-        TenantName shouldBeDeleted = TenantName.from("should-be-deleted");
+        TenantName shouldBeDeleted = TenantName.from("to-be-deleted");
         TenantName shouldNotBeDeleted = TenantName.from("should-not-be-deleted");
 
         tenantRepository.addTenant(shouldBeDeleted);
         tenantRepository.addTenant(shouldNotBeDeleted);
+        tenantRepository.addTenant(TenantRepository.HOSTED_VESPA_TENANT);
 
         tester.deployApp(applicationPackage, prepareParams(shouldNotBeDeleted));
         assertNotNull(tenantRepository.getTenant(shouldBeDeleted));
@@ -47,15 +48,15 @@ public class TenantsMaintainerTest {
         clock.advance(TenantsMaintainer.defaultTtlForUnusedTenant.plus(Duration.ofDays(1)));
         new TenantsMaintainer(applicationRepository, tester.curator(), new InMemoryFlagSource(), Duration.ofDays(1), clock).run();
 
-        // One tenant should have been deleted
+        // One tenant should now have been deleted
         assertNull(tenantRepository.getTenant(shouldBeDeleted));
         assertNotNull(tenantRepository.getTenant(shouldNotBeDeleted));
 
-        // System tenants should not have been deleted
+        // System tenants should not be deleted
         assertNotNull(tenantRepository.getTenant(TenantName.defaultName()));
         assertNotNull(tenantRepository.getTenant(TenantRepository.HOSTED_VESPA_TENANT));
 
-        // Delete both apps, add tenant again and deploy
+        // Delete app, add tenant again and deploy
         tester.applicationRepository().delete(applicationId(shouldNotBeDeleted));
         tenantRepository.addTenant(shouldBeDeleted);
         tester.deployApp(applicationPackage, prepareParams(shouldBeDeleted));
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/RpcTester.java b/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/RpcTester.java
index 726d9e9935a..135603def69 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/RpcTester.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/RpcTester.java
@@ -89,8 +89,7 @@ public class RpcTester implements AutoCloseable {
                 .withHostRegistry(hostRegistry)
                 .withConfigserverConfig(configserverConfig)
                 .build();
-        if (tenantRepository.getTenant(tenantName) == null)
-            tenantRepository.addTenant(tenantName);
+        tenantRepository.addTenant(tenantName);
         startRpcServer();
         applicationRepository = new ApplicationRepository.Builder()
                 .withTenantRepository(tenantRepository)
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/session/SessionRepositoryTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/session/SessionRepositoryTest.java
index 2000c73a8fa..d747df7cb41 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/session/SessionRepositoryTest.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/session/SessionRepositoryTest.java
@@ -91,6 +91,7 @@ public class SessionRepositoryTest {
                 .withFileDistributionFactory(new MockFileDistributionFactory(configserverConfig))
                 .withModelFactoryRegistry(modelFactoryRegistry)
                 .build();
+        tenantRepository.addTenant(SessionRepositoryTest.tenantName);
         applicationRepository = new ApplicationRepository.Builder()
                 .withTenantRepository(tenantRepository)
                 .withProvisioner(new MockProvisioner())