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
This commit is contained in:
Simon Willnauer 2018-12-12 16:21:02 +01:00 committed by GitHub
parent 3a56bb0924
commit ff5dd14753
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 10 additions and 14 deletions

View File

@ -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 * 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. * 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 { public void testCorruptFileThenSnapshotAndRestore() throws ExecutionException, InterruptedException, IOException {
int numDocs = scaledRandomIntBetween(100, 1000); int numDocs = scaledRandomIntBetween(100, 1000);
internalCluster().ensureAtLeastNumDataNodes(2); internalCluster().ensureAtLeastNumDataNodes(2);
@ -676,7 +675,10 @@ public class CorruptedFileIT extends ESIntegTestCase {
private void pruneOldDeleteGenerations(Set<Path> files) { private void pruneOldDeleteGenerations(Set<Path> files) {
final TreeSet<Path> delFiles = new TreeSet<>(); final TreeSet<Path> delFiles = new TreeSet<>();
for (Path file : files) { 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); delFiles.add(file);
} }
} }
@ -685,17 +687,9 @@ public class CorruptedFileIT extends ESIntegTestCase {
if (last != null) { if (last != null) {
final String newSegmentName = IndexFileNames.parseSegmentName(current.getFileName().toString()); final String newSegmentName = IndexFileNames.parseSegmentName(current.getFileName().toString());
final String oldSegmentName = IndexFileNames.parseSegmentName(last.getFileName().toString()); final String oldSegmentName = IndexFileNames.parseSegmentName(last.getFileName().toString());
if (newSegmentName.equals(oldSegmentName)) { if (newSegmentName.equals(oldSegmentName) ) {
int oldGen = long oldGen = IndexFileNames.parseGeneration(last.getFileName().toString());
Integer.parseInt( long newGen = IndexFileNames.parseGeneration(current.getFileName().toString());
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 (newGen > oldGen) { if (newGen > oldGen) {
files.remove(last); files.remove(last);
} else { } else {

View File

@ -93,10 +93,12 @@ public final class CorruptionUtils {
long checksumAfterCorruption; long checksumAfterCorruption;
long actualChecksumAfterCorruption; long actualChecksumAfterCorruption;
try (ChecksumIndexInput input = dir.openChecksumInput(fileToCorrupt.getFileName().toString(), IOContext.DEFAULT)) { try (ChecksumIndexInput input = dir.openChecksumInput(fileToCorrupt.getFileName().toString(), IOContext.DEFAULT)) {
assertThat(input.getFilePointer(), is(0L)); 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(); checksumAfterCorruption = input.getChecksum();
input.seek(input.length() - 8);
actualChecksumAfterCorruption = input.readLong(); 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 // we need to add assumptions here that the checksums actually really don't match there is a small chance to get collisions