From f806a2ac39d1cf073207b8ecd5cb5cbcb8c4337b Mon Sep 17 00:00:00 2001 From: Googler Date: Mon, 15 Jul 2024 06:05:25 -0700 Subject: [PATCH] Consolidate error message on action-cache corruption Most users will move on and not do anything with the .bad files, so avoid printing them to the console in order to make the error message more succinct. Instead, log the stashed corrupted files to info logging, with the rest of the relevant debug info. Motivation is side-stepping dealing with printing not-locally-accessible paths to the console when a build is running off-host (CI, etc). I don't think we have any good reason to console log the full path here beyond it's just been that way since the beginning. PiperOrigin-RevId: 652455344 Change-Id: I6068bae15b542320628624f6a5101242e33bc283 --- .../cache/CompactPersistentActionCache.java | 15 ++++++++------- .../lib/buildtool/CorruptedActionCacheTest.java | 3 +-- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/actions/cache/CompactPersistentActionCache.java b/src/main/java/com/google/devtools/build/lib/actions/cache/CompactPersistentActionCache.java index cb9ae97e453619..54bbe6471cd72f 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/cache/CompactPersistentActionCache.java +++ b/src/main/java/com/google/devtools/build/lib/actions/cache/CompactPersistentActionCache.java @@ -268,20 +268,18 @@ private static CompactPersistentActionCache logAndThrowOrRecurse( if (message != null) { e = new IOException(message, e); } - logger.atWarning().withCause(e).log("Failed to load action cache"); + logger.atWarning().withCause(e).log( + "Failed to load action cache, corrupted files to %s/*.bad", cacheRoot); reporterForInitializationErrors.handle( Event.error( "Error during action cache initialization: " + e.getMessage() - + ". Corrupted files were renamed to '" - + cacheRoot - + "/*.bad'. " - + "Bazel will now reset action cache data, potentially causing rebuilds")); + + ". Data will be reset, potentially causing target rebuilds")); if (alreadyFoundCorruption) { throw e; } return create( - cacheRoot, clock, reporterForInitializationErrors, /*alreadyFoundCorruption=*/ true); + cacheRoot, clock, reporterForInitializationErrors, /* alreadyFoundCorruption= */ true); } /** @@ -310,6 +308,7 @@ private static void renameCorruptedFiles(Path cacheRoot) { } private static final String FAILURE_PREFIX = "Failed action cache referential integrity check: "; + /** Throws IOException if indexer contains no data or integrity check has failed. */ private static void validateIntegrity(int indexerSize, byte[] validationRecord) throws IOException { @@ -546,7 +545,9 @@ private static RemoteFileArtifactValue decodeRemoteMetadata( digest, size, locationIndex, expireAtEpochMilli, materializationExecPath); } - /** @return action data encoded as a byte[] array. */ + /** + * @return action data encoded as a byte[] array. + */ private static byte[] encode(StringIndexer indexer, ActionCache.Entry entry) throws IOException { Preconditions.checkState(!entry.isCorrupted()); diff --git a/src/test/java/com/google/devtools/build/lib/buildtool/CorruptedActionCacheTest.java b/src/test/java/com/google/devtools/build/lib/buildtool/CorruptedActionCacheTest.java index 62dd9c56fb020d..3d4814feb2131d 100644 --- a/src/test/java/com/google/devtools/build/lib/buildtool/CorruptedActionCacheTest.java +++ b/src/test/java/com/google/devtools/build/lib/buildtool/CorruptedActionCacheTest.java @@ -65,7 +65,6 @@ public void testCorruptionActionCacheErrorMessage() throws Exception { assertThat(buildTarget("//foo:foo").getSuccess()).isTrue(); assertThat(events.errors()).hasSize(1); events.assertContainsError("Error during action cache initialization"); - events.assertContainsError( - "Bazel will now reset action cache data, potentially causing rebuilds"); + events.assertContainsError("Data will be reset, potentially causing target rebuilds"); } }