Skip to content

Commit 77e606c

Browse files
authored
[7.7.1] Preserve corrupted files after detecting action cache corruption. (#27601)
The logic introduced in ca183fb is too aggressive, and would also delete files that have been renamed to *.bad after detecting corruption.
1 parent 3397a82 commit 77e606c

File tree

2 files changed

+30
-3
lines changed

2 files changed

+30
-3
lines changed

src/main/java/com/google/devtools/build/lib/actions/cache/CompactPersistentActionCache.java

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -268,7 +268,11 @@ private static void deleteUnrecognizedFiles(Path cacheRoot) throws IOException {
268268
cacheFile(cacheRoot),
269269
journalFile(cacheRoot),
270270
indexFile(cacheRoot),
271-
indexJournalFile(cacheRoot));
271+
indexJournalFile(cacheRoot),
272+
corruptedPath(cacheFile(cacheRoot)),
273+
corruptedPath(journalFile(cacheRoot)),
274+
corruptedPath(indexFile(cacheRoot)),
275+
corruptedPath(indexJournalFile(cacheRoot)));
272276
for (Path child : cacheRoot.getDirectoryEntries()) {
273277
if (!knownFiles.contains(child)) {
274278
child.delete();
@@ -314,13 +318,13 @@ private static void renameCorruptedFiles(Path cacheRoot) {
314318
new UnixGlob.Builder(cacheRoot, SyscallCache.NO_CACHE)
315319
.addPattern("action_*_v" + VERSION + ".*")
316320
.glob()) {
317-
path.renameTo(path.getParentDirectory().getChild(path.getBaseName() + ".bad"));
321+
path.renameTo(corruptedPath(path));
318322
}
319323
for (Path path :
320324
new UnixGlob.Builder(cacheRoot, SyscallCache.NO_CACHE)
321325
.addPattern("filename_*_v" + VERSION + ".*")
322326
.glob()) {
323-
path.renameTo(path.getParentDirectory().getChild(path.getBaseName() + ".bad"));
327+
path.renameTo(corruptedPath(path));
324328
}
325329
} catch (UnixGlob.BadPattern ex) {
326330
throw new IllegalStateException(ex); // can't happen
@@ -329,6 +333,10 @@ private static void renameCorruptedFiles(Path cacheRoot) {
329333
}
330334
}
331335

336+
public static Path corruptedPath(Path path) {
337+
return path.getParentDirectory().getChild(path.getBaseName() + ".bad");
338+
}
339+
332340
private static final String FAILURE_PREFIX = "Failed action cache referential integrity check: ";
333341
/** Throws IOException if indexer contains no data or integrity check has failed. */
334342
private static void validateIntegrity(int indexerSize, byte[] validationRecord)

src/test/java/com/google/devtools/build/lib/actions/cache/CompactPersistentActionCacheTest.java

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,8 @@ public class CompactPersistentActionCacheTest {
5252
private Path dataRoot;
5353
private Path mapFile;
5454
private Path journalFile;
55+
private Path indexFile;
56+
private Path indexJournalFile;
5557
private final ManualClock clock = new ManualClock();
5658
private CompactPersistentActionCache cache;
5759
private ArtifactRoot artifactRoot;
@@ -62,6 +64,8 @@ public final void createFiles() throws Exception {
6264
cache = CompactPersistentActionCache.create(dataRoot, clock, NullEventHandler.INSTANCE);
6365
mapFile = CompactPersistentActionCache.cacheFile(dataRoot);
6466
journalFile = CompactPersistentActionCache.journalFile(dataRoot);
67+
indexFile = CompactPersistentActionCache.indexFile(dataRoot);
68+
indexJournalFile = CompactPersistentActionCache.indexJournalFile(dataRoot);
6569
artifactRoot =
6670
ArtifactRoot.asDerivedRoot(
6771
scratch.getFileSystem().getPath("/output"), ArtifactRoot.RootType.Output, "bin");
@@ -77,6 +81,21 @@ public void testDeleteUnrecognizedFiles() throws Exception {
7781
assertThat(unrecognizedFile.exists()).isFalse();
7882
}
7983

84+
@Test
85+
public void testRenameCorruptedFiles() throws Exception {
86+
FileSystemUtils.writeContentAsLatin1(mapFile, "corrupted data");
87+
FileSystemUtils.writeContentAsLatin1(journalFile, "corrupted data");
88+
FileSystemUtils.writeContentAsLatin1(indexFile, "corrupted data");
89+
FileSystemUtils.writeContentAsLatin1(indexJournalFile, "corrupted data");
90+
91+
cache = CompactPersistentActionCache.create(dataRoot, clock, NullEventHandler.INSTANCE);
92+
93+
assertThat(CompactPersistentActionCache.corruptedPath(mapFile).exists()).isTrue();
94+
assertThat(CompactPersistentActionCache.corruptedPath(journalFile).exists()).isTrue();
95+
assertThat(CompactPersistentActionCache.corruptedPath(indexFile).exists()).isTrue();
96+
assertThat(CompactPersistentActionCache.corruptedPath(indexJournalFile).exists()).isTrue();
97+
}
98+
8099
@Test
81100
public void testGetInvalidKey() {
82101
assertThat(cache.get("key")).isNull();

0 commit comments

Comments
 (0)