Skip to content

Commit

Permalink
Tie-break by path, so that AC entries are garbage collected before CA…
Browse files Browse the repository at this point in the history
…S entries with the same age.

PiperOrigin-RevId: 679138266
Change-Id: I6eadf1b872b4adaece69e56fd3977088634a348f
  • Loading branch information
tjgq authored and copybara-github committed Sep 26, 2024
1 parent 0b86e78 commit 6096b5e
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@
package com.google.devtools.build.lib.remote.disk;

import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static java.util.Comparator.comparing;

import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ComparisonChain;
import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.concurrent.AbstractQueueVisitor;
import com.google.devtools.build.lib.concurrent.ErrorClassifier;
Expand All @@ -29,6 +29,7 @@
import java.time.Duration;
import java.time.Instant;
import java.util.ArrayList;
import java.util.Comparator;
import java.util.List;
import java.util.Optional;
import java.util.concurrent.ExecutorService;
Expand Down Expand Up @@ -62,13 +63,23 @@ private record Entry(String path, long size, long mtime) {}
*/
public record CollectionPolicy(Optional<Long> maxSizeBytes, Optional<Duration> maxAge) {

// Sort older entries before newer ones, tie breaking by path. This causes AC entries to be
// sorted before CAS entries with the same age, making it less likely for garbage collection
// to break referential integrity in the event that mtime resolution is insufficient.
private static final Comparator<Entry> COMPARATOR =
(x, y) ->
ComparisonChain.start()
.compare(x.mtime(), y.mtime())
.compare(x.path(), y.path())
.result();

/**
* Returns the entries to be deleted.
*
* @param entries the full list of entries
*/
List<Entry> getEntriesToDelete(List<Entry> entries) {
entries.sort(comparing(Entry::mtime));
entries.sort(COMPARATOR);

long excessSizeBytes = getExcessSizeBytes(entries);
long timeCutoff = getTimeCutoff();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,21 @@ public void sizePolicy_collectsOldest() throws Exception {
assertFilesDoNotExist("ac/abc", "cas/def");
}

@Test
public void sizePolicy_tieBreakByPath() throws Exception {
writeFiles(
Entry.of("ac/123", kbytes(1), daysAgo(1)),
Entry.of("cas/456", kbytes(1), daysAgo(1)),
Entry.of("ac/abc", kbytes(1), daysAgo(1)),
Entry.of("cas/def", kbytes(1), daysAgo(1)));

CollectionStats stats = runGarbageCollector(Optional.of(kbytes(2)), Optional.empty());

assertThat(stats).isEqualTo(new CollectionStats(4, kbytes(4), 2, kbytes(2)));
assertFilesExist("cas/456", "cas/def");
assertFilesDoNotExist("ac/123", "ac/abc");
}

@Test
public void agePolicy_noCollection() throws Exception {
writeFiles(
Expand Down

0 comments on commit 6096b5e

Please sign in to comment.