[7.4.1] Prevent excessive thread creation in disk cache garbage colle…

…ction (#24114)

Fixes #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
3746583

Co-authored-by: Roman Salvador <[email protected]>

src/main/java/com/google/devtools/build/lib/remote/disk/DiskCacheGarbageCollector.java

@@ -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)" : "");
     }
   }
@@ -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();
 
@@ -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. */

src/main/java/com/google/devtools/build/lib/remote/disk/DiskCacheGarbageCollectorIdleTask.java

@@ -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) {

src/test/java/com/google/devtools/build/lib/remote/disk/DiskCacheGarbageCollectorTest.java

@@ -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");
   }
 
@@ -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");
   }
@@ -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");
   }
@@ -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");
   }
 
@@ -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");
   }
@@ -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");
   }
 
@@ -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");
   }
@@ -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");
   }
@@ -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");
   }
 
@@ -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 {