From 5e3abf5d3d68b47ccbc3e4db425f844cc0916e5f Mon Sep 17 00:00:00 2001 From: Roman Salvador Date: Sun, 27 Oct 2024 09:19:09 +0100 Subject: [PATCH 1/7] Log all errors in DiskCacheGarbageCollectorIdleTask --- .../lib/remote/disk/DiskCacheGarbageCollectorIdleTask.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 7c03e16fbb6b3c..30ccf1a12811cb 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 @@ -93,10 +93,10 @@ public void run() { logger.atInfo().log("Disk cache garbage collection started"); CollectionStats stats = gc.run(); logger.atInfo().log("%s", stats.displayString()); - } catch (IOException e) { - logger.atInfo().withCause(e).log("Disk cache garbage collection failed"); } catch (InterruptedException e) { logger.atInfo().withCause(e).log("Disk cache garbage collection interrupted"); + } catch (Throwable e) { + logger.atWarning().withCause(e).log("Disk cache garbage collection failed"); } } } From 59754c6a63a29f932ca96f807148cb6429d57830 Mon Sep 17 00:00:00 2001 From: Roman Salvador Date: Sun, 27 Oct 2024 09:21:03 +0100 Subject: [PATCH 2/7] Use newFixedThreadPool to avoid "unable to create native thread" error --- .../lib/remote/disk/DiskCacheGarbageCollectorIdleTask.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 30ccf1a12811cb..2633d685d2c861 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,7 @@ public final class DiskCacheGarbageCollectorIdleTask implements IdleTask { private final DiskCacheGarbageCollector gc; private static final ExecutorService executorService = - Executors.newCachedThreadPool( + Executors.newFixedThreadPool(Runtime.getRuntime().availableProcessors(), new ThreadFactoryBuilder().setNameFormat("disk-cache-gc-%d").build()); private DiskCacheGarbageCollectorIdleTask(Duration delay, DiskCacheGarbageCollector gc) { From d1a835456d44ca3ce9e9d2944a2eb2e68e1ce75e Mon Sep 17 00:00:00 2001 From: Roman Salvador Date: Sun, 27 Oct 2024 15:25:14 +0100 Subject: [PATCH 3/7] Show garbage collection performance --- .../remote/disk/DiskCacheGarbageCollector.java | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) 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 a57c2bf5fff105..5e19689cbec326 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 @@ 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. */ From c8fe194b5c10f1212fa711f9fef51a28b2b2388d Mon Sep 17 00:00:00 2001 From: Roman Salvador Date: Sun, 27 Oct 2024 18:43:36 +0100 Subject: [PATCH 4/7] Revert "Log all errors in DiskCacheGarbageCollectorIdleTask" This reverts commit 5e3abf5d3d68b47ccbc3e4db425f844cc0916e5f. --- .../lib/remote/disk/DiskCacheGarbageCollectorIdleTask.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 2633d685d2c861..24b06bc6268583 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 @@ -93,10 +93,10 @@ public void run() { logger.atInfo().log("Disk cache garbage collection started"); CollectionStats stats = gc.run(); logger.atInfo().log("%s", stats.displayString()); + } catch (IOException e) { + logger.atInfo().withCause(e).log("Disk cache garbage collection failed"); } catch (InterruptedException e) { logger.atInfo().withCause(e).log("Disk cache garbage collection interrupted"); - } catch (Throwable e) { - logger.atWarning().withCause(e).log("Disk cache garbage collection failed"); } } } From d9afaa0f24f7891aa94a61fff9b51f662d54bc00 Mon Sep 17 00:00:00 2001 From: Roman Salvador Date: Sun, 27 Oct 2024 18:32:43 +0100 Subject: [PATCH 5/7] Switch to unbounded virtual threads --- .../lib/remote/disk/DiskCacheGarbageCollectorIdleTask.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 24b06bc6268583..089e83904de2af 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,8 +36,8 @@ public final class DiskCacheGarbageCollectorIdleTask implements IdleTask { private final DiskCacheGarbageCollector gc; private static final ExecutorService executorService = - Executors.newFixedThreadPool(Runtime.getRuntime().availableProcessors(), - new ThreadFactoryBuilder().setNameFormat("disk-cache-gc-%d").build()); + Executors.newThreadPerTaskExecutor( + Thread.ofVirtual().name("disk-cache-gc-", 0).factory()); private DiskCacheGarbageCollectorIdleTask(Duration delay, DiskCacheGarbageCollector gc) { this.delay = delay; From f4bb05732f362b37230170b2c803333fe790d444 Mon Sep 17 00:00:00 2001 From: Roman Salvador Date: Sun, 27 Oct 2024 21:20:22 +0100 Subject: [PATCH 6/7] Fix DiskCacheGarbageCollectorTest --- .../disk/DiskCacheGarbageCollectorTest.java | 28 ++++++++++++------- 1 file changed, 18 insertions(+), 10 deletions(-) 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 41b778aacb3d98..b12e517dee325d 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 @@ 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,7 @@ 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 +90,7 @@ 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 +103,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 +117,7 @@ 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 +130,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 +144,7 @@ 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 +159,7 @@ 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 +171,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 +212,15 @@ 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 { From af155a3a1598fc6ca91379bb70d6df80c2c3d2be Mon Sep 17 00:00:00 2001 From: Roman Salvador Date: Mon, 28 Oct 2024 07:55:01 +0100 Subject: [PATCH 7/7] Switch to fixed threads with 4 lower bound --- .../lib/remote/disk/DiskCacheGarbageCollectorIdleTask.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) 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 089e83904de2af..b5ebaae5d91219 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,8 +36,9 @@ public final class DiskCacheGarbageCollectorIdleTask implements IdleTask { private final DiskCacheGarbageCollector gc; private static final ExecutorService executorService = - Executors.newThreadPerTaskExecutor( - Thread.ofVirtual().name("disk-cache-gc-", 0).factory()); + Executors.newFixedThreadPool( + Math.max(4, Runtime.getRuntime().availableProcessors()), + new ThreadFactoryBuilder().setNameFormat("disk-cache-gc-%d").build()); private DiskCacheGarbageCollectorIdleTask(Duration delay, DiskCacheGarbageCollector gc) { this.delay = delay;