diff options
author | Jon Marius Venstad <jonmv@users.noreply.github.com> | 2024-01-22 12:52:49 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-01-22 12:52:49 +0100 |
commit | dffa08a20bc0a0b415ca91eea4a36c8270e73564 (patch) | |
tree | 375b7f9e1c13b8dc1b662e0007de90ffcafeb18a | |
parent | 758be11d0dca244c195fc7077c47709c8a6fbb81 (diff) | |
parent | 351ea579e6afe47f65875f2da76e89600153b105 (diff) |
Merge pull request #30010 from vespa-engine/jonmv/clean-up-dangling-remote-sessions-too
Clear dangling "active" _remote_ sessions too
3 files changed, 24 insertions, 29 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 c10220b4d95..32f4d2b653c 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 @@ -43,6 +43,7 @@ import com.yahoo.slime.Slime; import com.yahoo.transaction.NestedTransaction; import com.yahoo.transaction.Transaction; import com.yahoo.vespa.applicationmodel.InfrastructureApplication; +import com.yahoo.vespa.config.server.application.ActiveTokenFingerprints; import com.yahoo.vespa.config.server.application.ActiveTokenFingerprints.Token; import com.yahoo.vespa.config.server.application.ActiveTokenFingerprintsClient; import com.yahoo.vespa.config.server.application.Application; @@ -54,7 +55,6 @@ import com.yahoo.vespa.config.server.application.ClusterReindexing; import com.yahoo.vespa.config.server.application.ClusterReindexingStatusClient; import com.yahoo.vespa.config.server.application.CompressedApplicationInputStream; import com.yahoo.vespa.config.server.application.ConfigConvergenceChecker; -import com.yahoo.vespa.config.server.application.ActiveTokenFingerprints; import com.yahoo.vespa.config.server.application.DefaultClusterReindexingStatusClient; import com.yahoo.vespa.config.server.application.FileDistributionStatus; import com.yahoo.vespa.config.server.application.HttpProxy; @@ -111,7 +111,6 @@ import java.time.Duration; import java.time.Instant; import java.util.Collection; import java.util.Comparator; -import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -135,7 +134,6 @@ import static com.yahoo.vespa.config.server.tenant.TenantRepository.HOSTED_VESPA import static com.yahoo.vespa.curator.Curator.CompletionWaiter; import static com.yahoo.yolean.Exceptions.uncheck; import static java.nio.file.Files.readAttributes; -import static java.util.stream.Collectors.toMap; /** * The API for managing applications. @@ -957,30 +955,25 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye } public void deleteExpiredLocalSessions() { - Map<Tenant, Collection<LocalSession>> sessionsPerTenant = new HashMap<>(); - tenantRepository.getAllTenants() - .forEach(tenant -> sessionsPerTenant.put(tenant, tenant.getSessionRepository().getLocalSessions())); - - Set<ApplicationId> applicationIds = new HashSet<>(); - sessionsPerTenant.values() - .forEach(sessionList -> sessionList.stream() - .map(Session::getOptionalApplicationId) - .filter(Optional::isPresent) - .forEach(appId -> applicationIds.add(appId.get()))); - - Map<ApplicationId, Long> activeSessions = new HashMap<>(); - applicationIds.forEach(applicationId -> getActiveSession(applicationId).ifPresent(session -> activeSessions.put(applicationId, session.getSessionId()))); - sessionsPerTenant.keySet().forEach(tenant -> tenant.getSessionRepository().deleteExpiredSessions(activeSessions)); + for (Tenant tenant : tenantRepository.getAllTenants()) { + tenant.getSessionRepository().deleteExpiredSessions(session -> sessionIsActiveForItsApplication(tenant, session)); + } } public int deleteExpiredRemoteSessions(Clock clock) { return tenantRepository.getAllTenants() .stream() - .map(tenant -> tenant.getSessionRepository().deleteExpiredRemoteSessions(clock)) + .map(tenant -> tenant.getSessionRepository().deleteExpiredRemoteSessions(clock, session -> sessionIsActiveForItsApplication(tenant, session))) .mapToInt(i -> i) .sum(); } + private boolean sessionIsActiveForItsApplication(Tenant tenant, Session session) { + Optional<ApplicationId> owner = session.getOptionalApplicationId(); + if (owner.isEmpty()) return true; // Chicken out ~(˘▾˘)~ + return tenant.getApplicationRepo().activeSessionOf(owner.get()).equals(Optional.of(session.getSessionId())); + } + // ---------------- Tenant operations ---------------------------------------------------------------- diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ApplicationPackageMaintainer.java b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ApplicationPackageMaintainer.java index b85f9376f61..4f10b1215cf 100644 --- a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ApplicationPackageMaintainer.java +++ b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/ApplicationPackageMaintainer.java @@ -24,7 +24,6 @@ import java.util.ArrayList; import java.util.List; import java.util.Optional; import java.util.concurrent.Future; -import java.util.function.Supplier; import java.util.logging.Logger; import static com.yahoo.vespa.config.server.filedistribution.FileDistributionUtil.fileReferenceExistsOnDisk; @@ -58,14 +57,16 @@ public class ApplicationPackageMaintainer extends ConfigServerMaintainer { int[] failures = new int[1]; List<Runnable> futureDownloads = new ArrayList<>(); - for (TenantName tenantName : applicationRepository.tenantRepository().getAllTenantNames()) + for (TenantName tenantName : applicationRepository.tenantRepository().getAllTenantNames()) { for (Session session : applicationRepository.tenantRepository().getTenant(tenantName).getSessionRepository().getRemoteSessions()) { if (shuttingDown()) break; switch (session.getStatus()) { - case PREPARE, ACTIVATE: break; - default: continue; + case PREPARE, ACTIVATE: + break; + default: + continue; } var applicationId = session.getApplicationId(); @@ -102,10 +103,11 @@ public class ApplicationPackageMaintainer extends ConfigServerMaintainer { } } } + } futureDownloads.forEach(Runnable::run); - return asSuccessFactorDeviation(attempts, failures[0]); + return asSuccessFactorDeviation(attempts, failures[0]); } private static FileDownloader createFileDownloader(ApplicationRepository applicationRepository, 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 52c11ed0e93..2f0d8b4065d 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 @@ -79,6 +79,7 @@ import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; +import java.util.function.Predicate; import java.util.logging.Level; import java.util.logging.Logger; @@ -369,7 +370,7 @@ public class SessionRepository { return session; } - public int deleteExpiredRemoteSessions(Clock clock) { + public int deleteExpiredRemoteSessions(Clock clock, Predicate<Session> sessionIsActiveForApplication) { Duration expiryTime = Duration.ofSeconds(expiryTimeFlag.value()); List<Long> remoteSessionsFromZooKeeper = getRemoteSessionsFromZooKeeper(); log.log(Level.FINE, () -> "Remote sessions for tenant " + tenantName + ": " + remoteSessionsFromZooKeeper); @@ -377,11 +378,11 @@ public class SessionRepository { int deleted = 0; // Avoid deleting too many in one run int deleteMax = (int) Math.min(1000, Math.max(50, remoteSessionsFromZooKeeper.size() * 0.05)); - for (long sessionId : remoteSessionsFromZooKeeper) { + for (Long sessionId : remoteSessionsFromZooKeeper) { Session session = remoteSessionCache.get(sessionId); if (session == null) session = new RemoteSession(tenantName, sessionId, createSessionZooKeeperClient(sessionId)); - if (session.getStatus() == Session.Status.ACTIVATE) continue; + if (session.getStatus() == Session.Status.ACTIVATE && sessionIsActiveForApplication.test(session)) continue; if (sessionHasExpired(session.getCreateTime(), expiryTime, clock)) { log.log(Level.FINE, () -> "Remote session " + sessionId + " for " + tenantName + " has expired, deleting it"); deleteRemoteSessionFromZooKeeper(session); @@ -616,7 +617,7 @@ public class SessionRepository { // ---------------- Common stuff ---------------------------------------------------------------- - public void deleteExpiredSessions(Map<ApplicationId, Long> activeSessions) { + public void deleteExpiredSessions(Predicate<Session> sessionIsActiveForApplication) { log.log(Level.FINE, () -> "Deleting expired local sessions for tenant '" + tenantName + "'"); Set<Long> sessionIdsToDelete = new HashSet<>(); Set<Long> newSessions = findNewSessionsInFileSystem(); @@ -650,8 +651,7 @@ public class SessionRepository { Optional<ApplicationId> applicationId = session.getOptionalApplicationId(); if (applicationId.isEmpty()) continue; - Long activeSession = activeSessions.get(applicationId.get()); - if (activeSession == null || activeSession != sessionId) { + if ( ! sessionIsActiveForApplication.test(session)) { sessionIdsToDelete.add(sessionId); log.log(Level.FINE, () -> "Will delete inactive session " + sessionId + " created " + createTime + " for '" + applicationId + "'"); |