Skip to content

Commit

Permalink
Prevent excessive thread creation in disk cache garbage collection
Browse files Browse the repository at this point in the history
Fixes bazelbuild#24098

With this change the disk cache garbage collection works correctly:
```
241027 09:06:34.732:I 681 [com.google.devtools.build.lib.remote.disk.DiskCacheGarbageCollectorIdleTask.run] Disk cache garbage collection started
241027 09:07:06.123:I 681 [com.google.devtools.build.lib.remote.disk.DiskCacheGarbageCollectorIdleTask.run] Deleted 190243 of 446229 files, reclaimed 5.4 GiB of 15.4 GiB
```

Closes bazelbuild#24099.

PiperOrigin-RevId: 690652512
Change-Id: Ie8d1fa6b2afb0bd5bd85fdb6835871023a64ad24
  • Loading branch information
rsalvador authored and bazel-io committed Oct 28, 2024
1 parent b86ef42 commit 20a793a
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -121,16 +121,24 @@ public record CollectionStats(
long totalBytes,
long deletedEntries,
long deletedBytes,
boolean concurrentUpdate) {
boolean concurrentUpdate,
Duration elapsedTime) {

/** Returns a human-readable summary. */
public String displayString() {
return "Deleted %d of %d files, reclaimed %s of %s%s"
double elapsedSeconds = elapsedTime.toSecondsPart() + elapsedTime.toMillisPart() / 1000.0;
int filesPerSecond = (int) Math.round((double) deletedEntries / elapsedSeconds);
int mbPerSecond = (int) Math.round((deletedBytes / (1024.0 * 1024.0)) / elapsedSeconds);

return "Deleted %d of %d files, reclaimed %s of %s in %.2f seconds (%d files/s, %d MB/s)%s"
.formatted(
deletedEntries(),
totalEntries(),
bytesCountToDisplayString(deletedBytes()),
bytesCountToDisplayString(totalBytes()),
elapsedSeconds,
filesPerSecond,
mbPerSecond,
concurrentUpdate() ? " (concurrent update detected)" : "");
}
}
Expand Down Expand Up @@ -180,6 +188,7 @@ public CollectionStats run() throws IOException, InterruptedException {
}

private CollectionStats runUnderLock() throws IOException, InterruptedException {
Instant startTime = Instant.now();
EntryScanner scanner = new EntryScanner();
EntryDeleter deleter = new EntryDeleter();

Expand All @@ -191,13 +200,15 @@ private CollectionStats runUnderLock() throws IOException, InterruptedException
}

DeletionStats deletionStats = deleter.await();
Duration elapsedTime = Duration.between(startTime, Instant.now());

return new CollectionStats(
allEntries.size(),
allEntries.stream().mapToLong(Entry::size).sum(),
deletionStats.deletedEntries(),
deletionStats.deletedBytes(),
deletionStats.concurrentUpdate());
deletionStats.concurrentUpdate(),
elapsedTime);
}

/** Lists all disk cache entries, performing I/O in parallel. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,8 @@ public final class DiskCacheGarbageCollectorIdleTask implements IdleTask {
private final DiskCacheGarbageCollector gc;

private static final ExecutorService executorService =
Executors.newCachedThreadPool(
Executors.newFixedThreadPool(
Math.max(4, Runtime.getRuntime().availableProcessors()),
new ThreadFactoryBuilder().setNameFormat("disk-cache-gc-%d").build());

private DiskCacheGarbageCollectorIdleTask(Duration delay, DiskCacheGarbageCollector gc) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public void sizePolicy_noCollection() throws Exception {

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

assertThat(stats).isEqualTo(new CollectionStats(2, kbytes(2), 0, 0, false));
assertThat(stats).isEqualTo(new CollectionStats(2, kbytes(2), 0, 0, false, Duration.ZERO));
assertFilesExist("ac/123", "cas/456");
}

Expand All @@ -75,7 +75,8 @@ public void sizePolicy_collectsOldest() throws Exception {

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

assertThat(stats).isEqualTo(new CollectionStats(4, kbytes(4), 2, kbytes(2), false));
assertThat(stats)
.isEqualTo(new CollectionStats(4, kbytes(4), 2, kbytes(2), false, Duration.ZERO));
assertFilesExist("ac/123", "cas/456");
assertFilesDoNotExist("ac/abc", "cas/def");
}
Expand All @@ -90,7 +91,8 @@ public void sizePolicy_tieBreakByPath() throws Exception {

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

assertThat(stats).isEqualTo(new CollectionStats(4, kbytes(4), 2, kbytes(2), false));
assertThat(stats)
.isEqualTo(new CollectionStats(4, kbytes(4), 2, kbytes(2), false, Duration.ZERO));
assertFilesExist("cas/456", "cas/def");
assertFilesDoNotExist("ac/123", "ac/abc");
}
Expand All @@ -103,7 +105,7 @@ public void agePolicy_noCollection() throws Exception {

CollectionStats stats = runGarbageCollector(Optional.empty(), Optional.of(days(3)));

assertThat(stats).isEqualTo(new CollectionStats(2, kbytes(2), 0, 0, false));
assertThat(stats).isEqualTo(new CollectionStats(2, kbytes(2), 0, 0, false, Duration.ZERO));
assertFilesExist("ac/123", "cas/456");
}

Expand All @@ -117,7 +119,8 @@ public void agePolicy_collectsOldest() throws Exception {

CollectionStats stats = runGarbageCollector(Optional.empty(), Optional.of(Duration.ofDays(3)));

assertThat(stats).isEqualTo(new CollectionStats(4, kbytes(4), 2, kbytes(2), false));
assertThat(stats)
.isEqualTo(new CollectionStats(4, kbytes(4), 2, kbytes(2), false, Duration.ZERO));
assertFilesExist("ac/123", "cas/456");
assertFilesDoNotExist("ac/abc", "cas/def");
}
Expand All @@ -130,7 +133,7 @@ public void sizeAndAgePolicy_noCollection() throws Exception {

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

assertThat(stats).isEqualTo(new CollectionStats(2, kbytes(2), 0, 0, false));
assertThat(stats).isEqualTo(new CollectionStats(2, kbytes(2), 0, 0, false, Duration.ZERO));
assertFilesExist("ac/123", "cas/456");
}

Expand All @@ -144,7 +147,8 @@ public void sizeAndAgePolicy_sizeMoreRestrictiveThanAge_collectsOldest() throws

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

assertThat(stats).isEqualTo(new CollectionStats(4, kbytes(4), 2, kbytes(2), false));
assertThat(stats)
.isEqualTo(new CollectionStats(4, kbytes(4), 2, kbytes(2), false, Duration.ZERO));
assertFilesExist("ac/123", "cas/456");
assertFilesDoNotExist("ac/abc", "cas/def");
}
Expand All @@ -159,7 +163,8 @@ public void sizeAndAgePolicy_ageMoreRestrictiveThanSize_collectsOldest() throws

CollectionStats stats = runGarbageCollector(Optional.of(kbytes(3)), Optional.of(days(3)));

assertThat(stats).isEqualTo(new CollectionStats(4, kbytes(4), 2, kbytes(2), false));
assertThat(stats)
.isEqualTo(new CollectionStats(4, kbytes(4), 2, kbytes(2), false, Duration.ZERO));
assertFilesExist("ac/123", "cas/456");
assertFilesDoNotExist("ac/abc", "cas/def");
}
Expand All @@ -171,7 +176,7 @@ public void ignoresTmpAndGcSubdirectories() throws Exception {

CollectionStats stats = runGarbageCollector(Optional.of(1L), Optional.of(days(1)));

assertThat(stats).isEqualTo(new CollectionStats(0, 0, 0, 0, false));
assertThat(stats).isEqualTo(new CollectionStats(0, 0, 0, 0, false, Duration.ZERO));
assertFilesExist("gc/foo", "tmp/foo");
}

Expand Down Expand Up @@ -212,7 +217,14 @@ private CollectionStats runGarbageCollector(
rootDir,
executorService,
new DiskCacheGarbageCollector.CollectionPolicy(maxSizeBytes, maxAge));
return gc.run();
CollectionStats resultStats = gc.run();
return new CollectionStats(
resultStats.totalEntries(),
resultStats.totalBytes(),
resultStats.deletedEntries(),
resultStats.deletedBytes(),
resultStats.concurrentUpdate(),
Duration.ZERO);
}

private void writeFiles(Entry... entries) throws IOException {
Expand Down

0 comments on commit 20a793a

Please sign in to comment.