Merge pull request #24608 from vespa-engine/hmusum/remove-downloaded-app-package-faster

Remove downloaded app package earlier

4 files changed, 19 insertions, 24 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 cd61b44be1b..29590beed19 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
@@ -594,19 +594,15 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye
         return fileDistributionStatus.status(getApplication(applicationId), timeout);
     }
 
-    public List<String> deleteUnusedFileDistributionReferences(File fileReferencesPath,
-                                                               Duration keepFileReferencesDuration,
-                                                               int numberToAlwaysKeep) {
+    public List<String> deleteUnusedFileDistributionReferences(File fileReferencesPath, Duration keepFileReferencesDuration) {
         if (!fileReferencesPath.isDirectory()) throw new RuntimeException(fileReferencesPath + " is not a directory");
 
         Set<String> fileReferencesInUse = getFileReferencesInUse();
         log.log(Level.FINE, () -> "File references in use : " + fileReferencesInUse);
         Instant instant = clock.instant().minus(keepFileReferencesDuration);
-        log.log(Level.FINE, () -> "Remove unused file references last modified before " + instant +
-                " (but keep " + numberToAlwaysKeep + " of those)");
+        log.log(Level.FINE, () -> "Remove unused file references last modified before " + instant);
 
-        List<String> candidates = sortedUnusedFileReferences(fileReferencesPath, fileReferencesInUse, instant);
-        List<String> fileReferencesToDelete = candidates.subList(0, Math.max(0, candidates.size() - numberToAlwaysKeep));
+        List<String> fileReferencesToDelete = sortedUnusedFileReferences(fileReferencesPath, fileReferencesInUse, instant);
         if (fileReferencesToDelete.size() > 0) {
             log.log(Level.FINE, () -> "Will delete file references not in use: " + fileReferencesToDelete);
             fileReferencesToDelete.forEach(fileReference -> {
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/FileDistributionMaintainer.java b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/FileDistributionMaintainer.java
index 0588a126b68..5542d24253b 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/FileDistributionMaintainer.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/maintenance/FileDistributionMaintainer.java
@@ -20,8 +20,6 @@ import java.time.Duration;
  */
 public class FileDistributionMaintainer extends ConfigServerMaintainer {
 
-    private static final int numberToAlwaysKeep = 10; // TODO: Reduce to 0 / remove
-
     private final ApplicationRepository applicationRepository;
     private final File fileReferencesDir;
     private final Duration maxUnusedFileReferenceAge;
@@ -39,7 +37,7 @@ public class FileDistributionMaintainer extends ConfigServerMaintainer {
 
     @Override
     protected double maintain() {
-        applicationRepository.deleteUnusedFileDistributionReferences(fileReferencesDir, maxUnusedFileReferenceAge, numberToAlwaysKeep);
+        applicationRepository.deleteUnusedFileDistributionReferences(fileReferencesDir, maxUnusedFileReferenceAge);
         return 1.0;
     }
 
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 15be909c069..89ca0265008 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
@@ -898,7 +898,13 @@ public class SessionRepository {
             ApplicationId applicationId = sessionZKClient.readApplicationId()
                     .orElseThrow(() -> new RuntimeException("Could not find application id for session " + sessionId));
             log.log(Level.FINE, () -> "Creating local session for tenant '" + tenantName + "' with session id " + sessionId);
-            createLocalSession(sessionDir, applicationId, sessionZKClient.readTags(), sessionId);
+            try {
+                createLocalSession(sessionDir, applicationId, sessionZKClient.readTags(), sessionId);
+            } finally {
+                // Delete downloaded file reference, not needed anymore
+                log.log(Level.FINE, "Deleting file distribution reference for app package with session id " + sessionDir);
+                IOUtils.recursiveDeleteDir(sessionDir);
+            }
         }
     }
 
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 e8d969119f9..4b28814112e 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
@@ -268,13 +268,9 @@ public class ApplicationRepositoryTest {
         File filereferenceDirOldest = createFileReferenceOnDisk(new File(fileReferencesDir, "bar"));
         clock.advance(Duration.ofSeconds(1));
 
-        // Add file references that are not in use and could be deleted
-        IntStream.range(0, 3).forEach(i -> {
-            try {
-                createFileReferenceOnDisk(new File(fileReferencesDir, "baz" + i));
-            } catch (IOException e) {
-                fail(e.getMessage());
-            }
+        // Add file references that are not in use should be deleted (baz0 and baz1)
+        IntStream.range(0, 2).forEach(i -> {
+            createFileReferenceOnDisk(new File(fileReferencesDir, "baz" + i));
             clock.advance(Duration.ofSeconds(1));
         });
         clock.advance(keepFileReferencesDuration);
@@ -293,20 +289,19 @@ public class ApplicationRepositoryTest {
         PrepareParams prepareParams = new PrepareParams.Builder().applicationId(applicationId()).ignoreValidationErrors(true).build();
         deployApp(new File("src/test/apps/app"), prepareParams);
 
-        List<String> deleted = applicationRepository.deleteUnusedFileDistributionReferences(fileReferencesDir,
-                                                                                                keepFileReferencesDuration,
-                                                                                                2);
+        List<String> deleted = applicationRepository.deleteUnusedFileDistributionReferences(fileReferencesDir, keepFileReferencesDuration);
         Collections.sort(deleted);
-        List<String> expected = new ArrayList<>(List.of("bar", "baz0"));
+        List<String> expected = new ArrayList<>(List.of("bar", "baz0", "baz1"));
         Collections.sort(expected);
         assertEquals(expected, deleted);
-        // bar and baz0 will be deleted, 2 of the old ones (baz1 and baz2) will be kept and foo is not old enough to be considered
+        // bar, baz0 and baz1 will be deleted and foo is not old enough to be considered
         assertFalse(filereferenceDirOldest.exists());
         assertFalse(new File(fileReferencesDir, "baz0").exists());
+        assertFalse(new File(fileReferencesDir, "baz1").exists());
         assertTrue(filereferenceDirNewest.exists());
     }
 
-    private File createFileReferenceOnDisk(File filereference) throws IOException {
+    private File createFileReferenceOnDisk(File filereference) {
         File fileReferenceDir = filereference.getParentFile();
         fileReferenceDir.mkdir();
         IOUtils.writeFile(filereference, Utf8.toBytes("test"));