Repository: cassandra Updated Branches: refs/heads/cassandra-2.2 94a68a17c -> a30d8bd21
Always mark sstable suspected on corruption patch by slebresne; reviewed by benedict for CASSANDRA-9478 Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/9b10928c Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/9b10928c Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/9b10928c Branch: refs/heads/cassandra-2.2 Commit: 9b10928c159317160fb3049727679a48232b6041 Parents: 63819cb Author: Sylvain Lebresne <sylv...@datastax.com> Authored: Mon May 25 18:26:56 2015 +0200 Committer: Sylvain Lebresne <sylv...@datastax.com> Committed: Tue Jun 2 14:46:09 2015 +0200 ---------------------------------------------------------------------- CHANGES.txt | 1 + .../io/sstable/SSTableIdentityIterator.java | 45 ++++++++++++++++---- .../compaction/BlacklistingCompactionsTest.java | 16 ++++--- 3 files changed, 48 insertions(+), 14 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cassandra/blob/9b10928c/CHANGES.txt ---------------------------------------------------------------------- diff --git a/CHANGES.txt b/CHANGES.txt index d23661d..1aad965 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -1,4 +1,5 @@ 2.0.16: + * Always mark sstable suspect when corrupted (CASSANDRA-9478) * Add database users and permissions to CQL3 documentation (CASSANDRA-7558) * Allow JVM_OPTS to be passed to standalone tools (CASSANDRA-5969) * Fix bad condition in RangeTombstoneList (CASSANDRA-9485) http://git-wip-us.apache.org/repos/asf/cassandra/blob/9b10928c/src/java/org/apache/cassandra/io/sstable/SSTableIdentityIterator.java ---------------------------------------------------------------------- diff --git a/src/java/org/apache/cassandra/io/sstable/SSTableIdentityIterator.java b/src/java/org/apache/cassandra/io/sstable/SSTableIdentityIterator.java index 52da9bb..8b45005 100644 --- a/src/java/org/apache/cassandra/io/sstable/SSTableIdentityIterator.java +++ b/src/java/org/apache/cassandra/io/sstable/SSTableIdentityIterator.java @@ -50,6 +50,9 @@ public class SSTableIdentityIterator implements Comparable<SSTableIdentityIterat private final boolean validateColumns; private final String filename; + // Not every SSTableIdentifyIterator is attached to a sstable, so this can be null. + private final SSTableReader sstable; + /** * Used to iterate through the columns of a row. * @param sstable SSTable we are reading ffrom. @@ -96,6 +99,7 @@ public class SSTableIdentityIterator implements Comparable<SSTableIdentityIterat this.flag = flag; this.validateColumns = checkData; this.dataVersion = sstable == null ? Descriptor.Version.CURRENT : sstable.descriptor.version; + this.sstable = sstable; try { @@ -132,9 +136,15 @@ public class SSTableIdentityIterator implements Comparable<SSTableIdentityIterat { // catch here b/c atomIterator is an AbstractIterator; hasNext reads the value if (e.getCause() instanceof IOException) + { + if (sstable != null) + sstable.markSuspect(); throw new CorruptSSTableException((IOException)e.getCause(), filename); + } else + { throw e; + } } } @@ -181,22 +191,39 @@ public class SSTableIdentityIterator implements Comparable<SSTableIdentityIterat { ColumnFamily cf = columnFamily.cloneMeShallow(containerFactory, false); // since we already read column count, just pass that value and continue deserialization - Iterator<OnDiskAtom> iter = cf.metadata().getOnDiskIterator(in, columnCount, flag, expireBefore, dataVersion); - while (iter.hasNext()) - cf.addAtom(iter.next()); + try + { + Iterator<OnDiskAtom> iter = cf.metadata().getOnDiskIterator(in, columnCount, flag, expireBefore, dataVersion); + while (iter.hasNext()) + cf.addAtom(iter.next()); - if (validateColumns) + if (validateColumns) + { + try + { + cf.metadata().validateColumns(cf); + } + catch (MarshalException e) + { + throw new RuntimeException("Error validating row " + key, e); + } + } + return cf; + } + catch (IOError e) { - try + // catch here b/c atomIterator is an AbstractIterator; hasNext reads the value + if (e.getCause() instanceof IOException) { - cf.metadata().validateColumns(cf); + if (sstable != null) + sstable.markSuspect(); + throw new CorruptSSTableException((IOException)e.getCause(), filename); } - catch (MarshalException e) + else { - throw new RuntimeException("Error validating row " + key, e); + throw e; } } - return cf; } public int compareTo(SSTableIdentityIterator o) http://git-wip-us.apache.org/repos/asf/cassandra/blob/9b10928c/test/unit/org/apache/cassandra/db/compaction/BlacklistingCompactionsTest.java ---------------------------------------------------------------------- diff --git a/test/unit/org/apache/cassandra/db/compaction/BlacklistingCompactionsTest.java b/test/unit/org/apache/cassandra/db/compaction/BlacklistingCompactionsTest.java index e392a4b..08d1d66 100644 --- a/test/unit/org/apache/cassandra/db/compaction/BlacklistingCompactionsTest.java +++ b/test/unit/org/apache/cassandra/db/compaction/BlacklistingCompactionsTest.java @@ -22,9 +22,7 @@ package org.apache.cassandra.db.compaction; import java.io.RandomAccessFile; -import java.util.Collection; -import java.util.HashSet; -import java.util.Set; +import java.util.*; import org.junit.BeforeClass; import org.junit.Test; @@ -41,6 +39,7 @@ import org.apache.cassandra.utils.ByteBufferUtil; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; public class BlacklistingCompactionsTest extends SchemaLoader { @@ -121,7 +120,14 @@ public class BlacklistingCompactionsTest extends SchemaLoader { raf = new RandomAccessFile(sstable.getFilename(), "rw"); assertNotNull(raf); - raf.write(0xFFFFFF); + assertTrue(raf.length() > 20); + raf.seek(new Random().nextInt((int)(raf.length() - 20))); + // We want to write something large enough that the corruption cannot get undetected + // (even without compression) + byte[] corruption = new byte[20]; + Arrays.fill(corruption, (byte)0xFF); + raf.write(corruption); + } finally { @@ -155,6 +161,6 @@ public class BlacklistingCompactionsTest extends SchemaLoader cfs.truncateBlocking(); - assertEquals(failures, sstablesToCorrupt); + assertEquals(sstablesToCorrupt, failures); } }