Author: slebresne
Date: Mon Apr 18 21:44:00 2011
New Revision: 1094780

URL: http://svn.apache.org/viewvc?rev=1094780&view=rev
Log:
Make scrub validate column fields
patch by slebresne; reviewed by jbellis for CASSANDRA-2460

Modified:
    
cassandra/branches/cassandra-0.7/src/java/org/apache/cassandra/db/Column.java
    
cassandra/branches/cassandra-0.7/src/java/org/apache/cassandra/db/ColumnFamily.java
    
cassandra/branches/cassandra-0.7/src/java/org/apache/cassandra/db/DeletedColumn.java
    
cassandra/branches/cassandra-0.7/src/java/org/apache/cassandra/db/ExpiringColumn.java
    
cassandra/branches/cassandra-0.7/src/java/org/apache/cassandra/db/IColumn.java
    
cassandra/branches/cassandra-0.7/src/java/org/apache/cassandra/db/SuperColumn.java
    
cassandra/branches/cassandra-0.7/src/java/org/apache/cassandra/io/sstable/SSTableIdentityIterator.java

Modified: 
cassandra/branches/cassandra-0.7/src/java/org/apache/cassandra/db/Column.java
URL: 
http://svn.apache.org/viewvc/cassandra/branches/cassandra-0.7/src/java/org/apache/cassandra/db/Column.java?rev=1094780&r1=1094779&r2=1094780&view=diff
==============================================================================
--- 
cassandra/branches/cassandra-0.7/src/java/org/apache/cassandra/db/Column.java 
(original)
+++ 
cassandra/branches/cassandra-0.7/src/java/org/apache/cassandra/db/Column.java 
Mon Apr 18 21:44:00 2011
@@ -26,7 +26,9 @@ import java.util.Collection;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import org.apache.cassandra.config.CFMetaData;
 import org.apache.cassandra.db.marshal.AbstractType;
+import org.apache.cassandra.db.marshal.MarshalException;
 import org.apache.cassandra.io.util.DataOutputBuffer;
 import org.apache.cassandra.utils.ByteBufferUtil;
 
@@ -232,5 +234,19 @@ public class Column implements IColumn
     {
         return !isMarkedForDelete();
     }
+
+    protected void validateName(CFMetaData metadata) throws MarshalException
+    {
+        AbstractType nameValidator = metadata.cfType == ColumnFamilyType.Super 
? metadata.subcolumnComparator : metadata.comparator;
+        nameValidator.validate(name());
+    }
+
+    public void validateFields(CFMetaData metadata) throws MarshalException
+    {
+        validateName(metadata);
+        AbstractType valueValidator = metadata.getValueValidator(name());
+        if (valueValidator != null)
+            valueValidator.validate(value());
+    }
 }
 

Modified: 
cassandra/branches/cassandra-0.7/src/java/org/apache/cassandra/db/ColumnFamily.java
URL: 
http://svn.apache.org/viewvc/cassandra/branches/cassandra-0.7/src/java/org/apache/cassandra/db/ColumnFamily.java?rev=1094780&r1=1094779&r2=1094780&view=diff
==============================================================================
--- 
cassandra/branches/cassandra-0.7/src/java/org/apache/cassandra/db/ColumnFamily.java
 (original)
+++ 
cassandra/branches/cassandra-0.7/src/java/org/apache/cassandra/db/ColumnFamily.java
 Mon Apr 18 21:44:00 2011
@@ -35,6 +35,7 @@ import org.apache.cassandra.config.CFMet
 import org.apache.cassandra.config.DatabaseDescriptor;
 import org.apache.cassandra.db.filter.QueryPath;
 import org.apache.cassandra.db.marshal.AbstractType;
+import org.apache.cassandra.db.marshal.MarshalException;
 import org.apache.cassandra.io.IColumnSerializer;
 import org.apache.cassandra.io.util.IIterableColumns;
 import org.apache.cassandra.utils.FBUtilities;
@@ -424,4 +425,18 @@ public class ColumnFamily implements ICo
         remove(column.name());
         addColumn(column.deepCopy());
     }
+
+    /**
+     * Goes over all columns and check the fields are valid (as far as we can
+     * tell).
+     * This is used to detect corruption after deserialization.
+     */
+    public void validateColumnFields() throws MarshalException
+    {
+        CFMetaData metadata = metadata();
+        for (IColumn column : getSortedColumns())
+        {
+            column.validateFields(metadata);
+        }
+    }
 }

Modified: 
cassandra/branches/cassandra-0.7/src/java/org/apache/cassandra/db/DeletedColumn.java
URL: 
http://svn.apache.org/viewvc/cassandra/branches/cassandra-0.7/src/java/org/apache/cassandra/db/DeletedColumn.java?rev=1094780&r1=1094779&r2=1094780&view=diff
==============================================================================
--- 
cassandra/branches/cassandra-0.7/src/java/org/apache/cassandra/db/DeletedColumn.java
 (original)
+++ 
cassandra/branches/cassandra-0.7/src/java/org/apache/cassandra/db/DeletedColumn.java
 Mon Apr 18 21:44:00 2011
@@ -23,6 +23,8 @@ import java.nio.ByteBuffer;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import org.apache.cassandra.config.CFMetaData;
+import org.apache.cassandra.db.marshal.MarshalException;
 import org.apache.cassandra.utils.ByteBufferUtil;
 
 public class DeletedColumn extends Column
@@ -62,4 +64,14 @@ public class DeletedColumn extends Colum
     {
         return new DeletedColumn(ByteBufferUtil.clone(name), 
ByteBufferUtil.clone(value), timestamp);
     }
+
+    @Override
+    public void validateFields(CFMetaData metadata) throws MarshalException
+    {
+        validateName(metadata);
+        if (value().remaining() != 4)
+            throw new MarshalException("A tombstone value should be 4 bytes 
long");
+        if (getLocalDeletionTime() < 0)
+            throw new MarshalException("The local deletion time should not be 
negative");
+    }
 }

Modified: 
cassandra/branches/cassandra-0.7/src/java/org/apache/cassandra/db/ExpiringColumn.java
URL: 
http://svn.apache.org/viewvc/cassandra/branches/cassandra-0.7/src/java/org/apache/cassandra/db/ExpiringColumn.java?rev=1094780&r1=1094779&r2=1094780&view=diff
==============================================================================
--- 
cassandra/branches/cassandra-0.7/src/java/org/apache/cassandra/db/ExpiringColumn.java
 (original)
+++ 
cassandra/branches/cassandra-0.7/src/java/org/apache/cassandra/db/ExpiringColumn.java
 Mon Apr 18 21:44:00 2011
@@ -24,7 +24,9 @@ import java.security.MessageDigest;
 
 import org.apache.log4j.Logger;
 
+import org.apache.cassandra.config.CFMetaData;
 import org.apache.cassandra.db.marshal.AbstractType;
+import org.apache.cassandra.db.marshal.MarshalException;
 import org.apache.cassandra.io.util.DataOutputBuffer;
 import org.apache.cassandra.utils.ByteBufferUtil;
 
@@ -135,4 +137,14 @@ public class ExpiringColumn extends Colu
             throw new IllegalStateException("column is not marked for delete");
         }
     }
+
+    @Override
+    public void validateFields(CFMetaData metadata) throws MarshalException
+    {
+        super.validateFields(metadata);
+        if (timeToLive <= 0)
+            throw new MarshalException("A column TTL should be > 0");
+        if (localExpirationTime < 0)
+            throw new MarshalException("The local expiration time should not 
be negative");
+    }
 }

Modified: 
cassandra/branches/cassandra-0.7/src/java/org/apache/cassandra/db/IColumn.java
URL: 
http://svn.apache.org/viewvc/cassandra/branches/cassandra-0.7/src/java/org/apache/cassandra/db/IColumn.java?rev=1094780&r1=1094779&r2=1094780&view=diff
==============================================================================
--- 
cassandra/branches/cassandra-0.7/src/java/org/apache/cassandra/db/IColumn.java 
(original)
+++ 
cassandra/branches/cassandra-0.7/src/java/org/apache/cassandra/db/IColumn.java 
Mon Apr 18 21:44:00 2011
@@ -22,7 +22,9 @@ import java.nio.ByteBuffer;
 import java.security.MessageDigest;
 import java.util.Collection;
 
+import org.apache.cassandra.config.CFMetaData;
 import org.apache.cassandra.db.marshal.AbstractType;
+import org.apache.cassandra.db.marshal.MarshalException;
 import org.apache.cassandra.utils.FBUtilities;
 
 public interface IColumn
@@ -45,6 +47,7 @@ public interface IColumn
     public void updateDigest(MessageDigest digest);
     public int getLocalDeletionTime(); // for tombstone GC, so int is 
sufficient granularity
     public String getString(AbstractType comparator);
+    public void validateFields(CFMetaData metadata) throws MarshalException;
 
     /** clones the column, making copies of any underlying byte buffers */
     IColumn deepCopy();

Modified: 
cassandra/branches/cassandra-0.7/src/java/org/apache/cassandra/db/SuperColumn.java
URL: 
http://svn.apache.org/viewvc/cassandra/branches/cassandra-0.7/src/java/org/apache/cassandra/db/SuperColumn.java?rev=1094780&r1=1094779&r2=1094780&view=diff
==============================================================================
--- 
cassandra/branches/cassandra-0.7/src/java/org/apache/cassandra/db/SuperColumn.java
 (original)
+++ 
cassandra/branches/cassandra-0.7/src/java/org/apache/cassandra/db/SuperColumn.java
 Mon Apr 18 21:44:00 2011
@@ -32,7 +32,9 @@ import java.util.concurrent.atomic.Atomi
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import org.apache.cassandra.config.CFMetaData;
 import org.apache.cassandra.db.marshal.AbstractType;
+import org.apache.cassandra.db.marshal.MarshalException;
 import org.apache.cassandra.io.IColumnSerializer;
 import org.apache.cassandra.io.util.ColumnSortedMap;
 import org.apache.cassandra.io.util.DataOutputBuffer;
@@ -306,6 +308,15 @@ public class SuperColumn implements ICol
     {
         throw new UnsupportedOperationException("This operation is unsupported 
on super columns.");
     }
+
+    public void validateFields(CFMetaData metadata) throws MarshalException
+    {
+        metadata.comparator.validate(name());
+        for (IColumn column : getSubColumns())
+        {
+            column.validateFields(metadata);
+        }
+    }
 }
 
 class SuperColumnSerializer implements IColumnSerializer

Modified: 
cassandra/branches/cassandra-0.7/src/java/org/apache/cassandra/io/sstable/SSTableIdentityIterator.java
URL: 
http://svn.apache.org/viewvc/cassandra/branches/cassandra-0.7/src/java/org/apache/cassandra/io/sstable/SSTableIdentityIterator.java?rev=1094780&r1=1094779&r2=1094780&view=diff
==============================================================================
--- 
cassandra/branches/cassandra-0.7/src/java/org/apache/cassandra/io/sstable/SSTableIdentityIterator.java
 (original)
+++ 
cassandra/branches/cassandra-0.7/src/java/org/apache/cassandra/io/sstable/SSTableIdentityIterator.java
 Mon Apr 18 21:44:00 2011
@@ -34,6 +34,7 @@ import org.apache.cassandra.db.ColumnFam
 import org.apache.cassandra.db.DecoratedKey;
 import org.apache.cassandra.db.IColumn;
 import org.apache.cassandra.db.columniterator.IColumnIterator;
+import org.apache.cassandra.db.marshal.MarshalException;
 import org.apache.cassandra.io.util.BufferedRandomAccessFile;
 import org.apache.cassandra.utils.Filter;
 
@@ -55,6 +56,8 @@ public class SSTableIdentityIterator imp
     // Used by lazilyCompactedRow, so that we see the same things when 
deserializing the first and second time
     private final int expireBefore;
 
+    private final boolean validateColumns;
+
     /**
      * Used to iterate through the columns of a row.
      * @param sstable SSTable we are reading ffrom.
@@ -70,7 +73,17 @@ public class SSTableIdentityIterator imp
         this(sstable, file, key, dataStart, dataSize, false);
     }
 
-    public SSTableIdentityIterator(SSTableReader sstable, 
BufferedRandomAccessFile file, DecoratedKey key, long dataStart, long dataSize, 
boolean deserializeRowHeader)
+    /**
+     * Used to iterate through the columns of a row.
+     * @param sstable SSTable we are reading ffrom.
+     * @param file Reading using this file.
+     * @param key Key of this row.
+     * @param dataStart Data for this row starts at this pos.
+     * @param dataSize length of row data
+     * @param checkData if true, do its best to deserialize and check the 
coherence of row data
+     * @throws IOException
+     */
+    public SSTableIdentityIterator(SSTableReader sstable, 
BufferedRandomAccessFile file, DecoratedKey key, long dataStart, long dataSize, 
boolean checkData)
     throws IOException
     {
         this.sstable = sstable;
@@ -79,12 +92,13 @@ public class SSTableIdentityIterator imp
         this.dataStart = dataStart;
         this.dataSize = dataSize;
         this.expireBefore = (int)(System.currentTimeMillis() / 1000);
+        this.validateColumns = checkData;
         finishedAt = dataStart + dataSize;
 
         try
         {
             file.seek(this.dataStart);
-            if (deserializeRowHeader)
+            if (checkData)
             {
                 try
                 {
@@ -141,12 +155,19 @@ public class SSTableIdentityIterator imp
     {
         try
         {
-            return sstable.getColumnSerializer().deserialize(file, 
expireBefore);
+            IColumn column = sstable.getColumnSerializer().deserialize(file, 
expireBefore);
+            if (validateColumns)
+                column.validateFields(sstable.metadata);
+            return column;
         }
         catch (IOException e)
         {
             throw new IOError(e);
         }
+        catch (MarshalException e)
+        {
+            throw new IOError(new IOException("Error validating row " + key, 
e));
+        }
     }
 
     public void remove()
@@ -178,6 +199,17 @@ public class SSTableIdentityIterator imp
         file.seek(columnPosition - 4); // seek to before column count int
         ColumnFamily cf = columnFamily.cloneMeShallow();
         ColumnFamily.serializer().deserializeColumns(file, cf);
+        if (validateColumns)
+        {
+            try
+            {
+                cf.validateColumnFields();
+            }
+            catch (MarshalException e)
+            {
+                throw new IOException("Error validating row " + key, e);
+            }
+        }
         return cf;
     }
 


Reply via email to