From ff5dd14753c2588f3be1ac16e3adb4fe38c3f590 Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Wed, 12 Dec 2018 16:21:02 +0100 Subject: [PATCH] Fix test failures related to file corruption (#36530) * Fix CorruptFileIT to also take last DV generation into account We currently only prune old .liv generations. With soft_deletes it's important to also prune DV generations. * Fix CorruptionUtils to skip the footer bytes after the checksum is read. Today we read a broken checksum since we also checksum the 8 footer bytes that include the checksum algorithm and the footer magic. Closes #36526 --- .../index/store/CorruptedFileIT.java | 20 +++++++------------ .../elasticsearch/test/CorruptionUtils.java | 4 +++- 2 files changed, 10 insertions(+), 14 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/index/store/CorruptedFileIT.java b/server/src/test/java/org/elasticsearch/index/store/CorruptedFileIT.java index 13f0e775526..8585c53d4f1 100644 --- a/server/src/test/java/org/elasticsearch/index/store/CorruptedFileIT.java +++ b/server/src/test/java/org/elasticsearch/index/store/CorruptedFileIT.java @@ -475,7 +475,6 @@ public class CorruptedFileIT extends ESIntegTestCase { * TODO once checksum verification on snapshotting is implemented this test needs to be fixed or split into several * parts... We should also corrupt files on the actual snapshot and check that we don't restore the corrupted shard. */ - @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/36526") public void testCorruptFileThenSnapshotAndRestore() throws ExecutionException, InterruptedException, IOException { int numDocs = scaledRandomIntBetween(100, 1000); internalCluster().ensureAtLeastNumDataNodes(2); @@ -676,7 +675,10 @@ public class CorruptedFileIT extends ESIntegTestCase { private void pruneOldDeleteGenerations(Set files) { final TreeSet delFiles = new TreeSet<>(); for (Path file : files) { - if (file.getFileName().toString().endsWith(".liv")) { + if (file.getFileName().toString().endsWith(".liv") + || file.getFileName().toString().endsWith(".dvm") + || file.getFileName().toString().endsWith(".dvd") + ) { delFiles.add(file); } } @@ -685,17 +687,9 @@ public class CorruptedFileIT extends ESIntegTestCase { if (last != null) { final String newSegmentName = IndexFileNames.parseSegmentName(current.getFileName().toString()); final String oldSegmentName = IndexFileNames.parseSegmentName(last.getFileName().toString()); - if (newSegmentName.equals(oldSegmentName)) { - int oldGen = - Integer.parseInt( - IndexFileNames.stripExtension( - IndexFileNames.stripSegmentName(last.getFileName().toString())).replace("_", ""), - Character.MAX_RADIX - ); - int newGen = Integer.parseInt( - IndexFileNames.stripExtension( - IndexFileNames.stripSegmentName(current.getFileName().toString())).replace("_", ""), - Character.MAX_RADIX); + if (newSegmentName.equals(oldSegmentName) ) { + long oldGen = IndexFileNames.parseGeneration(last.getFileName().toString()); + long newGen = IndexFileNames.parseGeneration(current.getFileName().toString()); if (newGen > oldGen) { files.remove(last); } else { diff --git a/test/framework/src/main/java/org/elasticsearch/test/CorruptionUtils.java b/test/framework/src/main/java/org/elasticsearch/test/CorruptionUtils.java index b2c562d43a6..fcd7fd999c4 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/CorruptionUtils.java +++ b/test/framework/src/main/java/org/elasticsearch/test/CorruptionUtils.java @@ -93,10 +93,12 @@ public final class CorruptionUtils { long checksumAfterCorruption; long actualChecksumAfterCorruption; + try (ChecksumIndexInput input = dir.openChecksumInput(fileToCorrupt.getFileName().toString(), IOContext.DEFAULT)) { assertThat(input.getFilePointer(), is(0L)); - input.seek(input.length() - 8); // one long is the checksum... 8 bytes + input.seek(input.length() - CodecUtil.footerLength()); checksumAfterCorruption = input.getChecksum(); + input.seek(input.length() - 8); actualChecksumAfterCorruption = input.readLong(); } // we need to add assumptions here that the checksums actually really don't match there is a small chance to get collisions