From 8edf60cd8d701509a23a90712f1f97a71771d4a5 Mon Sep 17 00:00:00 2001 From: Harald Musum Date: Thu, 9 Jul 2020 11:35:14 +0200 Subject: Delete expired locks (older than 1 day) * There has never been any cleanup of locks, but until we started taking a lock per session this has not been seen as an issue, since there were few of them * Add code to maintainer that deletes locks older than 1 day --- .../vespa/config/server/ApplicationRepository.java | 8 ++++++++ .../maintenance/ConfigServerMaintenance.java | 2 +- .../server/maintenance/SessionsMaintainer.java | 4 ++++ .../config/server/session/SessionRepository.java | 24 +++++++++++++++++++++- 4 files changed, 36 insertions(+), 2 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 bd86052c0dd..0d020486641 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 @@ -688,6 +688,14 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye sessionsPerTenant.keySet().forEach(tenant -> tenant.getSessionRepository().deleteExpiredSessions(activeSessions)); } + public int deleteExpiredLocks(Duration expiryTime) { + return tenantRepository.getAllTenants() + .stream() + .map(tenant -> tenant.getSessionRepository().deleteExpiredLocks(clock, expiryTime)) + .mapToInt(i -> i) + .sum(); + } + public int deleteExpiredRemoteSessions(Duration expiryTime) { return deleteExpiredRemoteSessions(clock, expiryTime); } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ConfigServerMaintenance.java b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ConfigServerMaintenance.java index c9866ac6dc0..b948da05556 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ConfigServerMaintenance.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ConfigServerMaintenance.java @@ -36,7 +36,7 @@ public class ConfigServerMaintenance extends AbstractComponent { // TODO: Disabled until we have application metadata //tenantsMaintainer = new TenantsMaintainer(applicationRepository, curator, defaults.tenantsMaintainerInterval); fileDistributionMaintainer = new FileDistributionMaintainer(applicationRepository, curator, defaults.defaultInterval, configserverConfig); - sessionsMaintainer = new SessionsMaintainer(applicationRepository, curator, defaults.defaultInterval); + sessionsMaintainer = new SessionsMaintainer(applicationRepository, curator, Duration.ofMinutes(1)); applicationPackageMaintainer = new ApplicationPackageMaintainer(applicationRepository, curator, Duration.ofMinutes(1), configserverConfig, flagSource); } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/SessionsMaintainer.java b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/SessionsMaintainer.java index 4975b82a801..46b1f26a62b 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/SessionsMaintainer.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/SessionsMaintainer.java @@ -35,5 +35,9 @@ public class SessionsMaintainer extends ConfigServerMaintainer { int deleted = applicationRepository.deleteExpiredRemoteSessions(expiryTime); log.log(LogLevel.FINE, "Deleted " + deleted + " expired remote sessions, expiry time " + expiryTime); } + + Duration lockExpiryTime = Duration.ofDays(1); + int deleted = applicationRepository.deleteExpiredLocks(lockExpiryTime); + log.log(LogLevel.INFO, "Deleted " + deleted + " expired locks, expiry time " + lockExpiryTime); } } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionRepository.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionRepository.java index fb2bbf29033..312a574bb2e 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionRepository.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionRepository.java @@ -37,6 +37,7 @@ import com.yahoo.vespa.flags.Flags; import org.apache.curator.framework.CuratorFramework; import org.apache.curator.framework.recipes.cache.ChildData; import org.apache.curator.framework.recipes.cache.PathChildrenCacheEvent; +import org.apache.zookeeper.data.Stat; import java.io.File; import java.io.FilenameFilter; @@ -281,6 +282,27 @@ public class SessionRepository { return deleted; } + public int deleteExpiredLocks(Clock clock, Duration expiryTime) { + int deleted = 0; + for (var lock : curator.getChildren(locksPath)) { + Path path = locksPath.append(lock); + if (zooKeeperNodeCreated(path).isBefore(clock.instant().minus(expiryTime))) { + log.log(Level.INFO, "Lock " + path + " has expired, deleting it"); + curator.delete(path); + deleted++; + } + } + return deleted; + } + + private Instant zooKeeperNodeCreated(Path path) { + Optional stat = curator.getStat(path); + if (stat.isPresent()) + return Instant.ofEpochMilli(stat.get().getCtime()); + else + return componentRegistry.getClock().instant(); + } + private boolean sessionHasExpired(Instant created, Duration expiryTime, Clock clock) { return (created.plus(expiryTime).isBefore(clock.instant())); } @@ -665,7 +687,7 @@ public class SessionRepository { return curator.lock(lockPath(sessionId), Duration.ofMinutes(1)); // These locks shouldn't be held for very long. } - private Path lockPath(long sessionId) { + public Path lockPath(long sessionId) { return locksPath.append(String.valueOf(sessionId)); } -- cgit v1.2.3 From 4ffd74ae2eabd17e49f95427addb7ed82bf11ee7 Mon Sep 17 00:00:00 2001 From: Harald Musum Date: Thu, 9 Jul 2020 13:00:09 +0200 Subject: Minor changes after review feedback --- .../vespa/config/server/maintenance/SessionsMaintainer.java | 2 +- .../vespa/config/server/session/SessionRepository.java | 13 ++++--------- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/SessionsMaintainer.java b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/SessionsMaintainer.java index 46b1f26a62b..ec06336c80a 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/SessionsMaintainer.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/SessionsMaintainer.java @@ -38,6 +38,6 @@ public class SessionsMaintainer extends ConfigServerMaintainer { Duration lockExpiryTime = Duration.ofDays(1); int deleted = applicationRepository.deleteExpiredLocks(lockExpiryTime); - log.log(LogLevel.INFO, "Deleted " + deleted + " expired locks, expiry time " + lockExpiryTime); + log.log(LogLevel.INFO, "Deleted " + deleted + " locks older than " + lockExpiryTime); } } diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionRepository.java b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionRepository.java index 312a574bb2e..b3eb0342a2f 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionRepository.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/session/SessionRepository.java @@ -37,7 +37,6 @@ import com.yahoo.vespa.flags.Flags; import org.apache.curator.framework.CuratorFramework; import org.apache.curator.framework.recipes.cache.ChildData; import org.apache.curator.framework.recipes.cache.PathChildrenCacheEvent; -import org.apache.zookeeper.data.Stat; import java.io.File; import java.io.FilenameFilter; @@ -286,7 +285,7 @@ public class SessionRepository { int deleted = 0; for (var lock : curator.getChildren(locksPath)) { Path path = locksPath.append(lock); - if (zooKeeperNodeCreated(path).isBefore(clock.instant().minus(expiryTime))) { + if (zooKeeperNodeCreated(path).orElse(clock.instant()).isBefore(clock.instant().minus(expiryTime))) { log.log(Level.INFO, "Lock " + path + " has expired, deleting it"); curator.delete(path); deleted++; @@ -295,12 +294,8 @@ public class SessionRepository { return deleted; } - private Instant zooKeeperNodeCreated(Path path) { - Optional stat = curator.getStat(path); - if (stat.isPresent()) - return Instant.ofEpochMilli(stat.get().getCtime()); - else - return componentRegistry.getClock().instant(); + private Optional zooKeeperNodeCreated(Path path) { + return curator.getStat(path).map(s -> Instant.ofEpochMilli(s.getCtime())); } private boolean sessionHasExpired(Instant created, Duration expiryTime, Clock clock) { @@ -687,7 +682,7 @@ public class SessionRepository { return curator.lock(lockPath(sessionId), Duration.ofMinutes(1)); // These locks shouldn't be held for very long. } - public Path lockPath(long sessionId) { + private Path lockPath(long sessionId) { return locksPath.append(String.valueOf(sessionId)); } -- cgit v1.2.3