[8.0.0] Prevent excessive thread creation in disk cache garbage collection (#24113)
Fixes https://github.com/bazelbuild/bazel/issues/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 #24099.
PiperOrigin-RevId: 690652512
Change-Id: Ie8d1fa6b2afb0bd5bd85fdb6835871023a64ad24
Commit
https://github.com/bazelbuild/bazel/commit/374658377f662701f1e92f757090949fdd0da1ab
Co-authored-by: Roman Salvador <rsalvador@salesforce.com>
diff --git a/src/main/java/com/google/devtools/build/lib/remote/disk/DiskCacheGarbageCollector.java b/src/main/java/com/google/devtools/build/lib/remote/disk/DiskCacheGarbageCollector.java
index a57c2bf..5e19689 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/disk/DiskCacheGarbageCollector.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/disk/DiskCacheGarbageCollector.java
@@ -121,16 +121,24 @@
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)" : "");
}
}
@@ -180,6 +188,7 @@
}
private CollectionStats runUnderLock() throws IOException, InterruptedException {
+ Instant startTime = Instant.now();
EntryScanner scanner = new EntryScanner();
EntryDeleter deleter = new EntryDeleter();
@@ -191,13 +200,15 @@
}
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. */
diff --git a/src/main/java/com/google/devtools/build/lib/remote/disk/DiskCacheGarbageCollectorIdleTask.java b/src/main/java/com/google/devtools/build/lib/remote/disk/DiskCacheGarbageCollectorIdleTask.java
index 7c03e16..b5ebaae 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/disk/DiskCacheGarbageCollectorIdleTask.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/disk/DiskCacheGarbageCollectorIdleTask.java
@@ -36,7 +36,8 @@
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) {
diff --git a/src/test/java/com/google/devtools/build/lib/remote/disk/DiskCacheGarbageCollectorTest.java b/src/test/java/com/google/devtools/build/lib/remote/disk/DiskCacheGarbageCollectorTest.java
index 41b778a..9e164c6 100644
--- a/src/test/java/com/google/devtools/build/lib/remote/disk/DiskCacheGarbageCollectorTest.java
+++ b/src/test/java/com/google/devtools/build/lib/remote/disk/DiskCacheGarbageCollectorTest.java
@@ -61,7 +61,7 @@
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");
}
@@ -75,7 +75,8 @@
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");
}
@@ -90,7 +91,8 @@
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");
}
@@ -103,7 +105,7 @@
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");
}
@@ -117,7 +119,8 @@
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");
}
@@ -130,7 +133,7 @@
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");
}
@@ -144,7 +147,8 @@
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");
}
@@ -159,7 +163,8 @@
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");
}
@@ -171,7 +176,7 @@
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");
}
@@ -212,7 +217,14 @@
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 {
