Use last access time to find file references in use

File references being used by active applications will never be deleted.
Use last access time instead of last modified time to find which ones to
delete or keep for the rest.

4 files changed, 29 insertions, 30 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 e0629a2e5db..38a31613592 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
@@ -587,11 +587,11 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye
         return orchestrator.getAllSuspendedApplications().contains(application);
     }
 
-    public HttpResponse filedistributionStatus(ApplicationId applicationId, Duration timeout) {
+    public HttpResponse fileDistributionStatus(ApplicationId applicationId, Duration timeout) {
         return fileDistributionStatus.status(getApplication(applicationId), timeout);
     }
 
-    public List<String> deleteUnusedFiledistributionReferences(File fileReferencesPath,
+    public List<String> deleteUnusedFileDistributionReferences(File fileReferencesPath,
                                                                Duration keepFileReferencesDuration,
                                                                int numberToAlwaysKeep) {
         log.log(Level.FINE, () -> "Keep unused file references for " + keepFileReferencesDuration);
@@ -632,8 +632,8 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye
         return fileReferencesOnDisk
                 .stream()
                 .filter(fileReference -> ! fileReferencesInUse.contains(fileReference))
-                .filter(fileReference -> isFileLastModifiedBefore(new File(fileReferencesPath, fileReference), instant))
-                .sorted((a, b) -> lastModified(new File(fileReferencesPath, a)).isBefore(lastModified(new File(fileReferencesPath, b))) ? -1 : 1)
+                .filter(fileReference -> isFileAccessedBefore(new File(fileReferencesPath, fileReference), instant))
+                .sorted((a, b) -> lastAccessed(new File(fileReferencesPath, a)).isBefore(lastAccessed(new File(fileReferencesPath, b))) ? -1 : 1)
                 .collect(Collectors.toList());
     }
 
@@ -688,15 +688,15 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye
                 .collect(Collectors.toList());
     }
 
-    private boolean isFileLastModifiedBefore(File fileReference, Instant instant) {
-        return lastModified(fileReference).isBefore(instant);
+    private boolean isFileAccessedBefore(File fileReference, Instant instant) {
+        return lastAccessed(fileReference).isBefore(instant);
     }
 
-    private Instant lastModified(File fileReference) {
+    private Instant lastAccessed(File fileReference) {
         BasicFileAttributes fileAttributes;
         try {
             fileAttributes = readAttributes(fileReference.toPath(), BasicFileAttributes.class);
-            return fileAttributes.lastModifiedTime().toInstant();
+            return fileAttributes.lastAccessTime().toInstant();
         } catch (IOException e) {
             throw new UncheckedIOException(e);
         }
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandler.java b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandler.java
index bdb48cc0160..e581a1edc21 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandler.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/http/v2/ApplicationHandler.java
@@ -136,7 +136,7 @@ public class ApplicationHandler extends HttpHandler {
     }
 
     private HttpResponse filedistributionStatus(ApplicationId applicationId, HttpRequest request) {
-        return applicationRepository.filedistributionStatus(applicationId, getTimeoutFromRequest(request));
+        return applicationRepository.fileDistributionStatus(applicationId, getTimeoutFromRequest(request));
     }
 
     private HttpResponse logs(ApplicationId applicationId, HttpRequest request) {
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 277e6acd6e6..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,7 +20,7 @@ import java.time.Duration;
  */
 public class FileDistributionMaintainer extends ConfigServerMaintainer {
 
-    private static final int numberToAlwaysKeep = 10;
+    private static final int numberToAlwaysKeep = 20;
 
     private final ApplicationRepository applicationRepository;
     private final File fileReferencesDir;
@@ -39,7 +39,7 @@ public class FileDistributionMaintainer extends ConfigServerMaintainer {
 
     @Override
     protected double maintain() {
-        applicationRepository.deleteUnusedFiledistributionReferences(fileReferencesDir, maxUnusedFileReferenceAge, numberToAlwaysKeep);
+        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 133570cc109..ce926016bd4 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
@@ -269,23 +269,23 @@ public class ApplicationRepositoryTest {
     }
 
     @Test
-    public void deleteUnusedFileReferences() throws IOException {
+    public void deleteUnusedFileReferences() throws IOException, InterruptedException {
         File fileReferencesDir = temporaryFolder.newFolder();
-        Duration keepFileReferences = Duration.ofHours(48);
+        Duration keepFileReferencesDuration = Duration.ofSeconds(4);
 
-        // Add file reference that is not in use and should be deleted (older than 'keepFileReferences')
+        // Add file reference that is not in use and should be deleted (older than 'keepFileReferencesDuration')
+        File filereferenceDirOldest = createFilereferenceOnDisk(new File(fileReferencesDir, "foo"));
+        //Thread.sleep(Duration.ofSeconds(1).toMillis());
 
-        Instant now = Instant.now();
-        File filereferenceDirOldest = createFilereferenceOnDisk(new File(fileReferencesDir, "foo"),
-                                                                now.minus(keepFileReferences.plus(Duration.ofHours(2))));
+        // Add file references that are not in use and could be deleted
+        IntStream.range(0, 3).forEach(i -> {
+            createFilereferenceOnDisk(new File(fileReferencesDir, "bar" + i));
+            try { Thread.sleep(Duration.ofSeconds(1).toMillis()); } catch (InterruptedException e) { /* ignore */ }
+        });
+        Thread.sleep(keepFileReferencesDuration.toMillis());
 
-        // Add file references that are not in use and some of them should be deleted (all are older than 'keepFileReferences')
-        IntStream.range(0, 6)
-                 .forEach(i -> createFilereferenceOnDisk(new File(fileReferencesDir, "bar" + i),
-                                                         now.minus(keepFileReferences.plus(Duration.ofHours(1).minus(Duration.ofMinutes(i))))));
-
-        // Add file reference that is not in use, but should not be deleted (newer than 'keepFileReferences')
-        File filereferenceDirNewest = createFilereferenceOnDisk(new File(fileReferencesDir, "baz"), now);
+        // Add file reference that is not in use, but should not be deleted (newer than 'keepFileReferencesDuration')
+        File filereferenceDirNewest = createFilereferenceOnDisk(new File(fileReferencesDir, "baz"));
 
         applicationRepository = new ApplicationRepository.Builder()
                 .withTenantRepository(tenantRepository)
@@ -298,22 +298,21 @@ public class ApplicationRepositoryTest {
         PrepareParams prepareParams = new PrepareParams.Builder().applicationId(applicationId()).ignoreValidationErrors(true).build();
         deployApp(new File("src/test/apps/app"), prepareParams);
 
-        List<String> toBeDeleted = applicationRepository.deleteUnusedFiledistributionReferences(fileReferencesDir,
-                                                                                                keepFileReferences,
-                                                                                                5);
+        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 5 newest no matter how old they are)
+        // bar0 and foo are the only ones that will be deleted (keeps 2 newest no matter how old they are)
         assertFalse(filereferenceDirOldest.exists());
         assertFalse(new File(fileReferencesDir, "bar0").exists());
         assertTrue(filereferenceDirNewest.exists());
     }
 
-    private File createFilereferenceOnDisk(File filereferenceDir, Instant lastModifiedTime) {
+    private File createFilereferenceOnDisk(File filereferenceDir) {
         assertTrue(filereferenceDir.mkdir());
         File bar = new File(filereferenceDir, "file");
         IOUtils.writeFile(bar, Utf8.toBytes("test"));
-        assertTrue(filereferenceDir.setLastModified(lastModifiedTime.toEpochMilli()));
         return filereferenceDir;
     }