Revert "Refactor cleanup of file references"

4 files changed, 76 insertions, 14 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 f832c504526..2a15f724b29 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
@@ -4,9 +4,9 @@ package com.yahoo.vespa.config.server;
 import ai.vespa.http.DomainName;
 import ai.vespa.http.HttpURL;
 import ai.vespa.http.HttpURL.Query;
+import com.yahoo.component.annotation.Inject;
 import com.yahoo.cloud.config.ConfigserverConfig;
 import com.yahoo.component.Version;
-import com.yahoo.component.annotation.Inject;
 import com.yahoo.config.FileReference;
 import com.yahoo.config.application.api.ApplicationFile;
 import com.yahoo.config.application.api.ApplicationMetaData;
@@ -83,18 +83,20 @@ import com.yahoo.vespa.curator.Curator;
 import com.yahoo.vespa.curator.stats.LockStats;
 import com.yahoo.vespa.curator.stats.ThreadLockStats;
 import com.yahoo.vespa.defaults.Defaults;
-import com.yahoo.vespa.filedistribution.maintenance.FileDistributionCleanup;
 import com.yahoo.vespa.flags.FlagSource;
 import com.yahoo.vespa.flags.InMemoryFlagSource;
 import com.yahoo.vespa.orchestrator.Orchestrator;
 import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
+import java.io.UncheckedIOException;
 import java.nio.file.Files;
+import java.nio.file.attribute.BasicFileAttributes;
 import java.time.Clock;
 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;
@@ -114,9 +116,11 @@ import static com.yahoo.config.model.api.container.ContainerServiceType.LOGSERVE
 import static com.yahoo.vespa.config.server.application.ConfigConvergenceChecker.ServiceListResponse;
 import static com.yahoo.vespa.config.server.application.ConfigConvergenceChecker.ServiceResponse;
 import static com.yahoo.vespa.config.server.filedistribution.FileDistributionUtil.fileReferenceExistsOnDisk;
+import static com.yahoo.vespa.config.server.filedistribution.FileDistributionUtil.getFileReferencesOnDisk;
 import static com.yahoo.vespa.config.server.tenant.TenantRepository.HOSTED_VESPA_TENANT;
 import static com.yahoo.vespa.curator.Curator.CompletionWaiter;
 import static com.yahoo.yolean.Exceptions.uncheck;
+import static java.nio.file.Files.readAttributes;
 
 /**
  * The API for managing applications.
@@ -586,11 +590,30 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye
         return fileDistributionStatus.status(getApplication(applicationId), timeout);
     }
 
-    public void deleteUnusedFileDistributionReferences(File fileReferencesPath, Duration keepFileReferencesDuration) {
-        new FileDistributionCleanup(clock).deleteUnusedFileReferences(fileReferencesPath, keepFileReferencesDuration, getFileReferencesInUse());
+    public List<String> deleteUnusedFileDistributionReferences(File fileReferencesPath,
+                                                               Duration keepFileReferencesDuration,
+                                                               int numberToAlwaysKeep) {
+        log.log(Level.FINE, () -> "Keep unused file references for " + 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);
+
+        List<String> candidates = sortedUnusedFileReferences(fileReferencesPath, fileReferencesInUse, keepFileReferencesDuration);
+        // Do not delete the newest ones
+        List<String> fileReferencesToDelete = candidates.subList(0, Math.max(0, candidates.size() - numberToAlwaysKeep));
+        if (fileReferencesToDelete.size() > 0) {
+            log.log(Level.FINE, () -> "Will delete file references not in use: " + fileReferencesToDelete);
+            fileReferencesToDelete.forEach(fileReference -> {
+                File file = new File(fileReferencesPath, fileReference);
+                if ( ! IOUtils.recursiveDeleteDir(file))
+                    log.log(Level.WARNING, "Could not delete " + file.getAbsolutePath());
+            });
+        }
+        return fileReferencesToDelete;
     }
 
-    Set<String> getFileReferencesInUse() {
+    private Set<String> getFileReferencesInUse() {
         Set<String> fileReferencesInUse = new HashSet<>();
         for (var applicationId : listApplications()) {
             Application app = getApplication(applicationId);
@@ -601,6 +624,18 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye
         return fileReferencesInUse;
     }
 
+    private List<String> sortedUnusedFileReferences(File fileReferencesPath, Set<String> fileReferencesInUse, Duration keepFileReferences) {
+        Set<String> fileReferencesOnDisk = getFileReferencesOnDisk(fileReferencesPath);
+        log.log(Level.FINE, () -> "File references on disk (in " + fileReferencesPath + "): " + fileReferencesOnDisk);
+        Instant instant = clock.instant().minus(keepFileReferences);
+        return fileReferencesOnDisk
+                .stream()
+                .filter(fileReference -> ! fileReferencesInUse.contains(fileReference))
+                .filter(fileReference -> isLastFileAccessBefore(new File(fileReferencesPath, fileReference), instant))
+                .sorted(Comparator.comparing(a -> lastAccessed(new File(fileReferencesPath, a))))
+                .collect(Collectors.toList());
+    }
+
     public Set<FileReference> getFileReferences(ApplicationId applicationId) {
         return getOptionalApplication(applicationId).map(app -> app.getModel().fileReferences()).orElse(Set.of());
     }
@@ -651,6 +686,20 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye
                 .collect(Collectors.toList());
     }
 
+    private boolean isLastFileAccessBefore(File fileReference, Instant instant) {
+        return lastAccessed(fileReference).isBefore(instant);
+    }
+
+    private Instant lastAccessed(File fileReference) {
+        BasicFileAttributes fileAttributes;
+        try {
+            fileAttributes = readAttributes(fileReference.toPath(), BasicFileAttributes.class);
+            return fileAttributes.lastAccessTime().toInstant();
+        } catch (IOException e) {
+            throw new UncheckedIOException(e);
+        }
+    }
+
     public Optional<String> getApplicationPackageReference(ApplicationId applicationId) {
         Optional<String> applicationPackage = Optional.empty();
         Optional<Session> session = getActiveSession(applicationId);
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileDistributionUtil.java b/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileDistributionUtil.java
index c1de9b4e5f6..a1ddad7bfd4 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileDistributionUtil.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/filedistribution/FileDistributionUtil.java
@@ -5,9 +5,12 @@ import com.yahoo.cloud.config.ConfigserverConfig;
 import com.yahoo.config.FileReference;
 import com.yahoo.net.HostName;
 import com.yahoo.vespa.config.server.ConfigServerSpec;
-import com.yahoo.vespa.filedistribution.maintenance.FileDistributionCleanup;
+
 import java.io.File;
+import java.util.Arrays;
+import java.util.HashSet;
 import java.util.List;
+import java.util.Set;
 import java.util.stream.Collectors;
 
 /**
@@ -18,6 +21,17 @@ import java.util.stream.Collectors;
  */
 public class FileDistributionUtil {
 
+    /**
+     * Returns all files in the given directory, non-recursive.
+     */
+    public static Set<String> getFileReferencesOnDisk(File directory) {
+        Set<String> fileReferencesOnDisk = new HashSet<>();
+        File[] filesOnDisk = directory.listFiles();
+        if (filesOnDisk != null)
+            fileReferencesOnDisk.addAll(Arrays.stream(filesOnDisk).map(File::getName).collect(Collectors.toSet()));
+        return fileReferencesOnDisk;
+    }
+
     public static List<String> getOtherConfigServersInCluster(ConfigserverConfig configserverConfig) {
         return ConfigServerSpec.fromConfig(configserverConfig)
                                .stream()
@@ -27,8 +41,7 @@ public class FileDistributionUtil {
     }
 
     public static boolean fileReferenceExistsOnDisk(File downloadDirectory, FileReference applicationPackageReference) {
-        return FileDistributionCleanup.getFileReferencesOnDisk(downloadDirectory.toPath())
-                                      .anyMatch(fileReference -> fileReference.equals(applicationPackageReference.value()));
+        return getFileReferencesOnDisk(downloadDirectory).contains(applicationPackageReference.value());
     }
 
 }
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 5542d24253b..f6aee416c9c 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,6 +20,8 @@ import java.time.Duration;
  */
 public class FileDistributionMaintainer extends ConfigServerMaintainer {
 
+    private static final int numberToAlwaysKeep = 20;
+
     private final ApplicationRepository applicationRepository;
     private final File fileReferencesDir;
     private final Duration maxUnusedFileReferenceAge;
@@ -37,7 +39,7 @@ public class FileDistributionMaintainer extends ConfigServerMaintainer {
 
     @Override
     protected double maintain() {
-        applicationRepository.deleteUnusedFileDistributionReferences(fileReferencesDir, maxUnusedFileReferenceAge);
+        applicationRepository.deleteUnusedFileDistributionReferences(fileReferencesDir, maxUnusedFileReferenceAge, numberToAlwaysKeep);
         return 1.0;
     }
 
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 4f7be104b9c..99487230c5d 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
@@ -49,7 +49,6 @@ import com.yahoo.vespa.config.server.tenant.TestTenantRepository;
 import com.yahoo.vespa.config.util.ConfigUtils;
 import com.yahoo.vespa.curator.Curator;
 import com.yahoo.vespa.curator.mock.MockCurator;
-import com.yahoo.vespa.filedistribution.maintenance.FileDistributionCleanup;
 import com.yahoo.vespa.flags.InMemoryFlagSource;
 import com.yahoo.vespa.model.VespaModelFactory;
 import org.junit.Before;
@@ -294,10 +293,9 @@ public class ApplicationRepositoryTest {
         PrepareParams prepareParams = new PrepareParams.Builder().applicationId(applicationId()).ignoreValidationErrors(true).build();
         deployApp(new File("src/test/apps/app"), prepareParams);
 
-        List<String> toBeDeleted = new FileDistributionCleanup(clock).deleteUnusedFileReferences(fileReferencesDir,
-                                                                                                 keepFileReferencesDuration,
-                                                                                                 2,
-                                                                                                 applicationRepository.getFileReferencesInUse());
+        List<String> toBeDeleted = applicationRepository.deleteUnusedFileDistributionReferences(fileReferencesDir,
+                                                                                                keepFileReferencesDuration,
+                                                                                                2);
         Collections.sort(toBeDeleted);
         assertEquals(List.of("bar0", "foo"), toBeDeleted);
         // bar0 and foo are the only ones that will be deleted (keeps 2 newest no matter how old they are)