You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by sl...@apache.org on 2015/06/02 14:46:43 UTC

cassandra git commit: Always mark sstable suspected on corruption

Repository: cassandra
Updated Branches:
  refs/heads/cassandra-2.0 63819cbee -> 9b10928c1


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.0
Commit: 9b10928c159317160fb3049727679a48232b6041
Parents: 63819cb
Author: Sylvain Lebresne <sy...@datastax.com>
Authored: Mon May 25 18:26:56 2015 +0200
Committer: Sylvain Lebresne <sy...@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);
     }
 }