diff options
5 files changed, 76 insertions, 16 deletions
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java b/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java index f8e1bcec0c3..a80ecffd94a 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java @@ -438,14 +438,14 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye // ---------------- Tenant operations ---------------------------------------------------------------- - public Set<TenantName> removeUnusedTenants() { - Set<TenantName> tenantsToBeDeleted = tenantRepository.getAllTenantNames().stream() + public Set<TenantName> deleteUnusedTenants(Duration ttlForUnusedTenant, Instant now) { + return tenantRepository.getAllTenantNames().stream() .filter(tenantName -> activeApplications(tenantName).isEmpty()) .filter(tenantName -> !tenantName.equals(TenantName.defaultName())) // Not allowed to remove 'default' tenant .filter(tenantName -> !tenantName.equals(TenantRepository.HOSTED_VESPA_TENANT)) // Not allowed to remove 'hosted-vespa' tenant + .filter(tenantName -> tenantRepository.getTenant(tenantName).getCreatedTime().isBefore(now.minus(ttlForUnusedTenant))) + .peek(tenantRepository::deleteTenant) .collect(Collectors.toSet()); - tenantsToBeDeleted.forEach(tenantRepository::deleteTenant); - return tenantsToBeDeleted; } public void deleteTenant(TenantName tenantName) { diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/TenantsMaintainer.java b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/TenantsMaintainer.java index 36306dbdde8..ad3b1f9b1ea 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/TenantsMaintainer.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/TenantsMaintainer.java @@ -5,15 +5,24 @@ import com.yahoo.vespa.config.server.ApplicationRepository; import com.yahoo.vespa.curator.Curator; import java.time.Duration; +import java.time.Instant; public class TenantsMaintainer extends Maintainer { + private final Duration ttlForUnusedTenant; + TenantsMaintainer(ApplicationRepository applicationRepository, Curator curator, Duration interval) { + this(applicationRepository, curator, interval, Duration.ofDays(7)); + } + + private TenantsMaintainer(ApplicationRepository applicationRepository, Curator curator, Duration interval, Duration ttlForUnusedTenant) { super(applicationRepository, curator, interval); + this.ttlForUnusedTenant = ttlForUnusedTenant; } @Override + // Delete unused tenants that were created more than ttlForUnusedTenant ago protected void maintain() { - applicationRepository.removeUnusedTenants(); + applicationRepository.deleteUnusedTenants(ttlForUnusedTenant, Instant.now()); } } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/Tenant.java b/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/Tenant.java index 3fe4c750922..3557d7bf9ab 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/Tenant.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/tenant/Tenant.java @@ -11,6 +11,11 @@ import com.yahoo.vespa.config.server.session.LocalSessionRepo; import com.yahoo.vespa.config.server.session.RemoteSessionRepo; import com.yahoo.vespa.config.server.session.SessionFactory; import com.yahoo.vespa.curator.Curator; +import org.apache.zookeeper.Op; +import org.apache.zookeeper.data.Stat; + +import java.time.Instant; +import java.util.Optional; /** * Contains all tenant-level components for a single tenant, dealing with editing sessions and @@ -124,6 +129,14 @@ public class Tenant implements TenantHandlerProvider { return curator; } + public Instant getCreatedTime() { + Optional<Stat> stat = curator.getStat(path); + if (stat.isPresent()) + return Instant.ofEpochMilli(stat.get().getCtime()); + else + return Instant.now(); + } + @Override public boolean equals(Object other) { if (!(other instanceof Tenant)) { 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 f7e76c2b3bb..12a4ed7a4c0 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 @@ -11,11 +11,11 @@ import com.yahoo.config.provision.InstanceName; import com.yahoo.config.provision.Provisioner; import com.yahoo.config.provision.TenantName; import com.yahoo.io.IOUtils; +import com.yahoo.test.ManualClock; import com.yahoo.text.Utf8; import com.yahoo.vespa.config.server.http.SessionHandlerTest; import com.yahoo.vespa.config.server.http.v2.PrepareResult; import com.yahoo.vespa.config.server.session.PrepareParams; -import com.yahoo.vespa.config.server.tenant.Tenant; import com.yahoo.vespa.config.server.tenant.TenantRepository; import com.yahoo.vespa.curator.Curator; import com.yahoo.vespa.curator.mock.MockCurator; @@ -45,10 +45,11 @@ public class ApplicationRepositoryTest { private final static File testAppJdiscOnly = new File("src/test/apps/app-jdisc-only"); private final static File testAppJdiscOnlyRestart = new File("src/test/apps/app-jdisc-only-restart"); - private final static TenantName tenantName = TenantName.from("test"); + private final static TenantName tenant1 = TenantName.from("test1"); + private final static TenantName tenant2 = TenantName.from("test2"); + private final static TenantName tenant3 = TenantName.from("test3"); private final static Clock clock = Clock.systemUTC(); - private Tenant tenant; private ApplicationRepository applicationRepository; private TenantRepository tenantRepository; private TimeoutBudget timeoutBudget; @@ -62,8 +63,9 @@ public class ApplicationRepositoryTest { tenantRepository = new TenantRepository(new TestComponentRegistry.Builder() .curator(curator) .build()); - tenantRepository.addTenant(tenantName); - tenant = tenantRepository.getTenant(tenantName); + tenantRepository.addTenant(tenant1); + tenantRepository.addTenant(tenant2); + tenantRepository.addTenant(tenant3); Provisioner provisioner = new SessionHandlerTest.MockProvisioner(); applicationRepository = new ApplicationRepository(tenantRepository, provisioner, clock); timeoutBudget = new TimeoutBudget(clock, Duration.ofSeconds(60)); @@ -93,10 +95,25 @@ public class ApplicationRepositoryTest { @Test public void deleteUnusedTenants() { + // Set clock to epoch plus hour, as mock curator will always return epoch as creation time + Instant now = ManualClock.at("1970-01-01T01:00:00"); + + // 3 tenants exist, tenant1 and tenant2 has applications deployed: deployApp(testApp); - assertTrue(applicationRepository.removeUnusedTenants().isEmpty()); + deployApp(testApp, new PrepareParams.Builder().applicationId(applicationId(tenant2)).build()); + + // Should not be deleted, not old enough + Duration ttlForUnusedTenant = Duration.ofHours(1); + assertTrue(applicationRepository.deleteUnusedTenants(ttlForUnusedTenant, now).isEmpty()); + // Should be deleted + ttlForUnusedTenant = Duration.ofMillis(1); + assertEquals(tenant3, applicationRepository.deleteUnusedTenants(ttlForUnusedTenant, now).iterator().next()); + + // Delete app used by tenant1, tenant2 still has an application applicationRepository.remove(applicationId()); - assertEquals(tenantName, applicationRepository.removeUnusedTenants().iterator().next()); + Set<TenantName> tenantsDeleted = applicationRepository.deleteUnusedTenants(Duration.ofMillis(1), now); + assertTrue(tenantsDeleted.contains(tenant1)); + assertFalse(tenantsDeleted.contains(tenant2)); } @Test @@ -125,8 +142,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(tenantName); - tenant = tenantRepository.getTenant(tenantName); + tenantRepository.addTenant(tenant1); Provisioner provisioner = new SessionHandlerTest.MockProvisioner(); applicationRepository = new ApplicationRepository(tenantRepository, provisioner, clock); timeoutBudget = new TimeoutBudget(clock, Duration.ofSeconds(60)); @@ -158,8 +174,10 @@ public class ApplicationRepositoryTest { private PrepareResult prepareAndActivateApp(File application) throws IOException { FilesApplicationPackage appDir = FilesApplicationPackage.fromFile(application); - long sessionId = applicationRepository.createSession(applicationId(), timeoutBudget, appDir.getAppDir()); - return applicationRepository.prepareAndActivate(tenant, sessionId, prepareParams(), false, false, Instant.now()); + ApplicationId applicationId = applicationId(); + long sessionId = applicationRepository.createSession(applicationId, timeoutBudget, appDir.getAppDir()); + return applicationRepository.prepareAndActivate(tenantRepository.getTenant(applicationId.tenant()), + sessionId, prepareParams(), false, false, Instant.now()); } private PrepareResult deployApp(File applicationPackage) { @@ -175,6 +193,10 @@ public class ApplicationRepositoryTest { } private ApplicationId applicationId() { + return ApplicationId.from(tenant1, ApplicationName.from("testapp"), InstanceName.defaultName()); + } + + private ApplicationId applicationId(TenantName tenantName) { return ApplicationId.from(tenantName, ApplicationName.from("testapp"), InstanceName.defaultName()); } diff --git a/zkfacade/src/main/java/com/yahoo/vespa/curator/Curator.java b/zkfacade/src/main/java/com/yahoo/vespa/curator/Curator.java index 0e0c7401e0e..6ba394c340d 100644 --- a/zkfacade/src/main/java/com/yahoo/vespa/curator/Curator.java +++ b/zkfacade/src/main/java/com/yahoo/vespa/curator/Curator.java @@ -21,6 +21,7 @@ import org.apache.curator.framework.recipes.locks.InterProcessMutex; import org.apache.curator.framework.state.ConnectionState; import org.apache.curator.framework.state.ConnectionStateListener; import org.apache.curator.retry.ExponentialBackoffRetry; +import org.apache.zookeeper.data.Stat; import java.time.Duration; import java.util.Arrays; @@ -298,6 +299,21 @@ public class Curator implements AutoCloseable { } } + /** + * Returns the stat data at the given path. + * Empty is returned if the path does not exist. + */ + public Optional<Stat> getStat(Path path) { + if ( ! exists(path)) return Optional.empty(); + + try { + return Optional.of(framework().checkExists().forPath(path.getAbsolute())); + } + catch (Exception e) { + throw new RuntimeException("Could not get data at " + path.getAbsolute(), e); + } + } + /** Returns the curator framework API */ public CuratorFramework framework() { return curatorFramework; |