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 2011/04/19 14:51:56 UTC

svn commit: r1095070 - in /cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra: db/ db/context/ io/sstable/

Author: slebresne
Date: Tue Apr 19 12:51:56 2011
New Revision: 1095070

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

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

Modified: cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/Column.java
URL: http://svn.apache.org/viewvc/cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/Column.java?rev=1095070&r1=1095069&r2=1095070&view=diff
==============================================================================
--- cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/Column.java (original)
+++ cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/Column.java Tue Apr 19 12:51:56 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;
 
@@ -237,5 +239,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.8/src/java/org/apache/cassandra/db/ColumnFamily.java
URL: http://svn.apache.org/viewvc/cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/ColumnFamily.java?rev=1095070&r1=1095069&r2=1095070&view=diff
==============================================================================
--- cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/ColumnFamily.java (original)
+++ cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/ColumnFamily.java Tue Apr 19 12:51:56 2011
@@ -38,6 +38,7 @@ import org.apache.cassandra.config.Datab
 import org.apache.cassandra.db.filter.QueryPath;
 import org.apache.cassandra.db.marshal.AbstractCommutativeType;
 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;
@@ -432,4 +433,18 @@ public class ColumnFamily implements ICo
             size += column.serializedSize();
         return size;
     }
+
+    /**
+     * 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.8/src/java/org/apache/cassandra/db/CounterColumn.java
URL: http://svn.apache.org/viewvc/cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/CounterColumn.java?rev=1095070&r1=1095069&r2=1095070&view=diff
==============================================================================
--- cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/CounterColumn.java (original)
+++ cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/CounterColumn.java Tue Apr 19 12:51:56 2011
@@ -27,9 +27,11 @@ import java.util.Map;
 
 import org.apache.log4j.Logger;
 
+import org.apache.cassandra.config.CFMetaData;
 import org.apache.cassandra.db.context.CounterContext;
 import org.apache.cassandra.db.context.IContext.ContextRelationship;
 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;
 import org.apache.cassandra.utils.FBUtilities;
@@ -204,6 +206,15 @@ public class CounterColumn extends Colum
         return ColumnSerializer.COUNTER_MASK;
     }
 
+    @Override
+    public void validateFields(CFMetaData metadata) throws MarshalException
+    {
+        validateName(metadata);
+        // We cannot use the value validator as for other columns as the CounterColumnType validate a long,
+        // which is not the internal representation of counters
+        contextManager.validateContext(value());
+    }
+
     /**
      * Check if a given nodeId is found in this CounterColumn context.
      */
@@ -269,4 +280,5 @@ public class CounterColumn extends Colum
             }
         }
     }
+
 }

Modified: cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/DeletedColumn.java
URL: http://svn.apache.org/viewvc/cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/DeletedColumn.java?rev=1095070&r1=1095069&r2=1095070&view=diff
==============================================================================
--- cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/DeletedColumn.java (original)
+++ cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/DeletedColumn.java Tue Apr 19 12:51:56 2011
@@ -20,6 +20,8 @@ package org.apache.cassandra.db;
 
 import java.nio.ByteBuffer;
 
+import org.apache.cassandra.config.CFMetaData;
+import org.apache.cassandra.db.marshal.MarshalException;
 import org.apache.cassandra.utils.ByteBufferUtil;
 
 public class DeletedColumn extends Column
@@ -71,4 +73,14 @@ public class DeletedColumn extends Colum
     {
         return ColumnSerializer.DELETION_MASK;
     }
+
+    @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.8/src/java/org/apache/cassandra/db/ExpiringColumn.java
URL: http://svn.apache.org/viewvc/cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/ExpiringColumn.java?rev=1095070&r1=1095069&r2=1095070&view=diff
==============================================================================
--- cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/ExpiringColumn.java (original)
+++ cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/ExpiringColumn.java Tue Apr 19 12:51:56 2011
@@ -22,7 +22,9 @@ import java.io.IOException;
 import java.nio.ByteBuffer;
 import java.security.MessageDigest;
 
+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;
 
@@ -137,4 +139,14 @@ public class ExpiringColumn extends Colu
     {
         return ColumnSerializer.EXPIRATION_MASK;
     }
+
+    @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.8/src/java/org/apache/cassandra/db/IColumn.java
URL: http://svn.apache.org/viewvc/cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/IColumn.java?rev=1095070&r1=1095069&r2=1095070&view=diff
==============================================================================
--- cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/IColumn.java (original)
+++ cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/IColumn.java Tue Apr 19 12:51:56 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
@@ -46,6 +48,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, interning column names and making copies of other underlying byte buffers
      * @param cfs*/

Modified: cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/SuperColumn.java
URL: http://svn.apache.org/viewvc/cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/SuperColumn.java?rev=1095070&r1=1095069&r2=1095070&view=diff
==============================================================================
--- cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/SuperColumn.java (original)
+++ cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/SuperColumn.java Tue Apr 19 12:51:56 2011
@@ -29,7 +29,9 @@ import java.util.concurrent.ConcurrentSk
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicLong;
 
+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;
@@ -321,6 +323,15 @@ public class SuperColumn implements ICol
     {
         throw new UnsupportedOperationException("Super columns don't have a serialization mask");
     }
+
+    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.8/src/java/org/apache/cassandra/db/context/CounterContext.java
URL: http://svn.apache.org/viewvc/cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/context/CounterContext.java?rev=1095070&r1=1095069&r2=1095070&view=diff
==============================================================================
--- cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/context/CounterContext.java (original)
+++ cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/context/CounterContext.java Tue Apr 19 12:51:56 2011
@@ -21,6 +21,7 @@ import java.nio.ByteBuffer;
 import java.security.MessageDigest;
 import java.util.*;
 
+import org.apache.cassandra.db.marshal.MarshalException;
 import org.apache.cassandra.db.DBConstants;
 import org.apache.cassandra.utils.ByteBufferUtil;
 import org.apache.cassandra.utils.NodeId;
@@ -452,6 +453,13 @@ public class CounterContext implements I
         return cleaned;
     }
 
+    public void validateContext(ByteBuffer context) throws MarshalException
+    {
+        int headerLength = headerLength(context);
+        if (headerLength < 0 || (context.remaining() - headerLength) %  STEP_LENGTH != 0)
+            throw new MarshalException("Invalid size for a counter context");
+    }
+
     /**
      * Update a MessageDigest with the content of a context.
      * Note that this skips the header entirely since the header information

Modified: cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/io/sstable/SSTableIdentityIterator.java
URL: http://svn.apache.org/viewvc/cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/io/sstable/SSTableIdentityIterator.java?rev=1095070&r1=1095069&r2=1095070&view=diff
==============================================================================
--- cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/io/sstable/SSTableIdentityIterator.java (original)
+++ cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/io/sstable/SSTableIdentityIterator.java Tue Apr 19 12:51:56 2011
@@ -35,6 +35,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;
 
@@ -56,6 +57,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.
@@ -71,10 +74,20 @@ 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.metadata, file, key, dataStart, dataSize, deserializeRowHeader, sstable, false);
+        this(sstable.metadata, file, key, dataStart, dataSize, checkData, sstable, false);
     }
 
     public SSTableIdentityIterator(CFMetaData metadata, BufferedRandomAccessFile file, DecoratedKey key, long dataStart, long dataSize, boolean fromRemote)
@@ -84,7 +97,7 @@ public class SSTableIdentityIterator imp
     }
 
     // sstable may be null *if* deserializeRowHeader is false
-    private SSTableIdentityIterator(CFMetaData metadata, BufferedRandomAccessFile file, DecoratedKey key, long dataStart, long dataSize, boolean deserializeRowHeader, SSTableReader sstable, boolean fromRemote)
+    private SSTableIdentityIterator(CFMetaData metadata, BufferedRandomAccessFile file, DecoratedKey key, long dataStart, long dataSize, boolean checkData, SSTableReader sstable, boolean fromRemote)
     throws IOException
     {
         this.file = file;
@@ -93,12 +106,13 @@ public class SSTableIdentityIterator imp
         this.dataSize = dataSize;
         this.expireBefore = (int)(System.currentTimeMillis() / 1000);
         this.fromRemote = fromRemote;
+        this.validateColumns = checkData;
         finishedAt = dataStart + dataSize;
 
         try
         {
             file.seek(this.dataStart);
-            if (deserializeRowHeader)
+            if (checkData)
             {
                 try
                 {
@@ -155,12 +169,19 @@ public class SSTableIdentityIterator imp
     {
         try
         {
-            return columnFamily.getColumnSerializer().deserialize(file, null, fromRemote, expireBefore);
+            IColumn column = columnFamily.getColumnSerializer().deserialize(file, null, fromRemote, expireBefore);
+            if (validateColumns)
+                column.validateFields(columnFamily.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()
@@ -192,6 +213,17 @@ public class SSTableIdentityIterator imp
         file.seek(columnPosition - 4); // seek to before column count int
         ColumnFamily cf = columnFamily.cloneMeShallow();
         ColumnFamily.serializer().deserializeColumns(file, cf, false, fromRemote);
+        if (validateColumns)
+        {
+            try
+            {
+                cf.validateColumnFields();
+            }
+            catch (MarshalException e)
+            {
+                throw new IOException("Error validating row " + key, e);
+            }
+        }
         return cf;
     }