Merge pull request #7624 from vespa-engine/jvenstad/package-gc-fix

Jvenstad/package gc fix

2 files changed, 15 insertions, 10 deletions

diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java
index bed1df3a8a7..ac317794725 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/ApplicationController.java
@@ -567,6 +567,8 @@ public class ApplicationController {
                                                    new com.yahoo.vespa.hosted.controller.api.identifiers.ApplicationId(id.application().value()), token.get());
             }
             curator.removeApplication(id);
+            applicationStore.removeAll(id);
+            applicationStore.removeAll(TesterId.of(id));
 
             log.info("Deleted " + application);
         }));
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java
index da98dd0e165..480e7b3b0fe 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/deployment/JobController.java
@@ -248,14 +248,7 @@ public class JobController {
                                                              version.get(),
                                                              testPackageBytes);
 
-            application.get().deployments().values().stream()
-                       .map(Deployment::applicationVersion)
-                       .min(Comparator.comparingLong(applicationVersion -> applicationVersion.buildNumber().getAsLong()))
-                       .ifPresent(oldestDeployed -> {
-                           controller.applications().applicationStore().prune(id, oldestDeployed);
-                           controller.applications().applicationStore().prune(TesterId.of(id), oldestDeployed);
-                       });
-
+            prunePackages(id);
             controller.applications().storeWithUpdatedConfig(application.withBuiltInternally(true), new ApplicationPackage(packageBytes));
 
             notifyOfNewSubmission(id, projectId, revision, run);
@@ -304,8 +297,6 @@ public class JobController {
                                    logs.delete(id);
                                }
                            });
-                       controller.applications().applicationStore().removeAll(id);
-                       controller.applications().applicationStore().removeAll(tester);
                    }
                    catch (TimeoutException e) {
                        return; // Don't remove the data if we couldn't clean up all resources.
@@ -355,6 +346,18 @@ public class JobController {
         controller.applications().deploymentTrigger().notifyOfCompletion(report);
     }
 
+    private void prunePackages(ApplicationId id) {
+        controller.applications().lockIfPresent(id, application -> {
+            application.get().deployments().values().stream()
+                       .map(Deployment::applicationVersion)
+                       .min(Comparator.comparingLong(applicationVersion -> applicationVersion.buildNumber().getAsLong()))
+                       .ifPresent(oldestDeployed -> {
+                           controller.applications().applicationStore().prune(id, oldestDeployed);
+                           controller.applications().applicationStore().prune(TesterId.of(id), oldestDeployed);
+                       });
+        });
+    }
+
     /** Locks and modifies the list of historic runs for the given application and job type. */
     private void locked(ApplicationId id, JobType type, Consumer<SortedMap<RunId, Run>> modifications) {
         try (Lock __ = curator.lock(id, type)) {