Skip to content

Commit

Permalink
Merge pull request #24598 from vespa-engine/hmusum/use-last-modified-…
Browse files Browse the repository at this point in the history 
…time-for-deleting-unused-file-references

Remove unused file references based on last modified time
  • Loading branch information
Harald Musum authored Oct 26, 2022
2 parents 5c279f0 + 16b382e commit 6670558
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 27 deletions.
22 changes: 11 additions & 11 deletions configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -597,14 +597,15 @@ public HttpResponse fileDistributionStatus(ApplicationId applicationId, Duration
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);
Instant instant = clock.instant().minus(keepFileReferencesDuration);
log.log(Level.FINE, () -> "Remove unused file references last modified before " + instant +
" (but keep " + numberToAlwaysKeep + " of those)");

List<String> candidates = sortedUnusedFileReferences(fileReferencesPath, fileReferencesInUse, keepFileReferencesDuration);
// Do not delete the newest ones
List<String> candidates = sortedUnusedFileReferences(fileReferencesPath, fileReferencesInUse, instant);
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);
Expand All @@ -628,15 +629,14 @@ private Set<String> getFileReferencesInUse() {
return fileReferencesInUse;
}

private List<String> sortedUnusedFileReferences(File fileReferencesPath, Set<String> fileReferencesInUse, Duration keepFileReferences) {
private List<String> sortedUnusedFileReferences(File fileReferencesPath, Set<String> fileReferencesInUse, Instant instant) {
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))))
.filter(fileReference -> isLastModifiedBefore(new File(fileReferencesPath, fileReference), instant))
.sorted(Comparator.comparing(a -> lastModified(new File(fileReferencesPath, a))))
.collect(Collectors.toList());
}

Expand Down Expand Up @@ -690,15 +690,15 @@ public List<ApplicationId> listApplications() {
.collect(Collectors.toList());
}

private boolean isLastFileAccessBefore(File fileReference, Instant instant) {
return lastAccessed(fileReference).isBefore(instant);
private boolean isLastModifiedBefore(File fileReference, Instant instant) {
return lastModified(fileReference).isBefore(instant);
}

private Instant lastAccessed(File fileReference) {
private Instant lastModified(File fileReference) {
BasicFileAttributes fileAttributes;
try {
fileAttributes = readAttributes(fileReference.toPath(), BasicFileAttributes.class);
return fileAttributes.lastAccessTime().toInstant();
return fileAttributes.lastModifiedTime().toInstant();
} catch (IOException e) {
throw new UncheckedIOException(e);
}
Expand Down
2 changes: 1 addition & 1 deletion ...r/src/main/java/com/yahoo/vespa/config/server/maintenance/FileDistributionMaintainer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
*/
public class FileDistributionMaintainer extends ConfigServerMaintainer {

private static final int numberToAlwaysKeep = 20;
private static final int numberToAlwaysKeep = 10; // TODO: Reduce to 0 / remove

private final ApplicationRepository applicationRepository;
private final File fileReferencesDir;
Expand Down
31 changes: 16 additions & 15 deletions configserver/src/test/java/com/yahoo/vespa/config/server/ApplicationRepositoryTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@
import java.io.File;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.attribute.FileTime;
import java.time.Duration;
import java.time.Instant;
import java.util.ArrayList;
Expand Down Expand Up @@ -266,13 +265,13 @@ public void deleteUnusedFileReferences() throws IOException {
Duration keepFileReferencesDuration = Duration.ofSeconds(4);

// Add file reference that is not in use and should be deleted (older than 'keepFileReferencesDuration')
File filereferenceDirOldest = createFilereferenceOnDisk(new File(fileReferencesDir, "foo"));
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, "bar" + i));
createFileReferenceOnDisk(new File(fileReferencesDir, "baz" + i));
} catch (IOException e) {
fail(e.getMessage());
}
Expand All @@ -281,7 +280,7 @@ public void deleteUnusedFileReferences() throws IOException {
clock.advance(keepFileReferencesDuration);

// Add file reference that is not in use, but should not be deleted (newer than 'keepFileReferencesDuration')
File filereferenceDirNewest = createFilereferenceOnDisk(new File(fileReferencesDir, "baz"));
File filereferenceDirNewest = createFileReferenceOnDisk(new File(fileReferencesDir, "foo"));

applicationRepository = new ApplicationRepository.Builder()
.withTenantRepository(tenantRepository)
Expand All @@ -294,23 +293,25 @@ public void deleteUnusedFileReferences() throws IOException {
PrepareParams prepareParams = new PrepareParams.Builder().applicationId(applicationId()).ignoreValidationErrors(true).build();
deployApp(new File("src/test/apps/app"), prepareParams);

List<String> toBeDeleted = applicationRepository.deleteUnusedFileDistributionReferences(fileReferencesDir,
List<String> deleted = 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)
Collections.sort(deleted);
List<String> expected = new ArrayList<>(List.of("bar", "baz0"));
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
assertFalse(filereferenceDirOldest.exists());
assertFalse(new File(fileReferencesDir, "bar0").exists());
assertFalse(new File(fileReferencesDir, "baz0").exists());
assertTrue(filereferenceDirNewest.exists());
}

private File createFilereferenceOnDisk(File filereferenceDir) throws IOException {
assertTrue(filereferenceDir.mkdir());
File file = new File(filereferenceDir, "bar");
IOUtils.writeFile(file, Utf8.toBytes("test"));
Files.setAttribute(filereferenceDir.toPath(), "lastAccessTime", FileTime.from(clock.instant()));
return filereferenceDir;
private File createFileReferenceOnDisk(File filereference) throws IOException {
File fileReferenceDir = filereference.getParentFile();
fileReferenceDir.mkdir();
IOUtils.writeFile(filereference, Utf8.toBytes("test"));
filereference.setLastModified(clock.instant().toEpochMilli());
return filereference;
}

@Test
Expand Down

0 comments on commit 6670558

Please sign in to comment.