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 2014/01/31 16:47:49 UTC

git commit: Fix potential loss of 2ndary index entry during compaction

Updated Branches:
  refs/heads/cassandra-2.0 d76ad2e24 -> 0191b359f


Fix potential loss of 2ndary index entry during compaction

patch by slebresne; reviewed by beobal for CASSANDRA-6517


Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo
Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/0191b359
Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/0191b359
Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/0191b359

Branch: refs/heads/cassandra-2.0
Commit: 0191b359fc18ebb1efa940257729d141c26112d3
Parents: d76ad2e
Author: Sylvain Lebresne <sy...@datastax.com>
Authored: Fri Jan 31 16:47:01 2014 +0100
Committer: Sylvain Lebresne <sy...@datastax.com>
Committed: Fri Jan 31 16:47:01 2014 +0100

----------------------------------------------------------------------
 CHANGES.txt                                     |  1 +
 .../db/compaction/PrecompactedRow.java          |  2 +-
 .../apache/cassandra/db/RangeTombstoneTest.java | 59 ++++++++++++++++++++
 3 files changed, 61 insertions(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cassandra/blob/0191b359/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index 57fbe28..13b4c5b 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -19,6 +19,7 @@
  * sstables from stalled repair sessions can resurrect deleted data (CASSANDRA-6503)
  * Switch stress to use ITransportFactory (CASSANDRA-6641)
  * Fix IllegalArgumentException during prepare (CASSANDRA-6592)
+ * Fix possible loss of 2ndary index entries during compaction (CASSANDRA-6517)
 Merged from 1.2:
  * fsync compression metadata (CASSANDRA-6531)
  * Validate CF existence on execution for prepared statement (CASSANDRA-6535)

http://git-wip-us.apache.org/repos/asf/cassandra/blob/0191b359/src/java/org/apache/cassandra/db/compaction/PrecompactedRow.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/compaction/PrecompactedRow.java b/src/java/org/apache/cassandra/db/compaction/PrecompactedRow.java
index d45bffa..db72847 100644
--- a/src/java/org/apache/cassandra/db/compaction/PrecompactedRow.java
+++ b/src/java/org/apache/cassandra/db/compaction/PrecompactedRow.java
@@ -126,7 +126,7 @@ public class PrecompactedRow extends AbstractCompactedRow
                 // notify the index that the column has been overwritten if the value being reduced has been
                 // superceded by another directly, or indirectly by a range tombstone
                 if ((!column.isMarkedForDelete(System.currentTimeMillis()) && !container.getColumn(column.name()).equals(column))
-                    || returnCF.deletionInfo().isDeleted(column.name(), CompactionManager.NO_GC))
+                    || returnCF.deletionInfo().isDeleted(column))
                 {
                     indexer.remove(column);
                 }

http://git-wip-us.apache.org/repos/asf/cassandra/blob/0191b359/test/unit/org/apache/cassandra/db/RangeTombstoneTest.java
----------------------------------------------------------------------
diff --git a/test/unit/org/apache/cassandra/db/RangeTombstoneTest.java b/test/unit/org/apache/cassandra/db/RangeTombstoneTest.java
index 731b364..d66f6db 100644
--- a/test/unit/org/apache/cassandra/db/RangeTombstoneTest.java
+++ b/test/unit/org/apache/cassandra/db/RangeTombstoneTest.java
@@ -31,6 +31,7 @@ import org.apache.cassandra.exceptions.ConfigurationException;
 import org.apache.cassandra.io.sstable.SSTableReader;
 import org.apache.cassandra.thrift.IndexType;
 
+import org.junit.Ignore;
 import org.junit.Test;
 
 import org.apache.cassandra.SchemaLoader;
@@ -303,6 +304,64 @@ public class RangeTombstoneTest extends SchemaLoader
         assertEquals(index.deletes.get(0), index.inserts.get(0));
     }
 
+    @Test
+    public void testOverwritesToDeletedColumns() throws Exception
+    {
+        Keyspace table = Keyspace.open(KSNAME);
+        ColumnFamilyStore cfs = table.getColumnFamilyStore(CFNAME);
+        ByteBuffer key = ByteBufferUtil.bytes("k6");
+        ByteBuffer indexedColumnName = ByteBufferUtil.bytes(1);
+
+        cfs.truncateBlocking();
+        cfs.disableAutoCompaction();
+        cfs.setCompactionStrategyClass(SizeTieredCompactionStrategy.class.getCanonicalName());
+        if (cfs.indexManager.getIndexForColumn(indexedColumnName) == null)
+        {
+            ColumnDefinition cd = new ColumnDefinition(indexedColumnName,
+                    cfs.getComparator(),
+                    IndexType.CUSTOM,
+                    ImmutableMap.of(SecondaryIndex.CUSTOM_INDEX_OPTION_NAME, TestIndex.class.getName()),
+                    "test_index",
+                    0,
+                    null);
+            cfs.indexManager.addIndexedColumn(cd);
+        }
+
+        TestIndex index = ((TestIndex)cfs.indexManager.getIndexForColumn(indexedColumnName));
+        index.resetCounts();
+
+        RowMutation rm = new RowMutation(KSNAME, key);
+        add(rm, 1, 0);
+        rm.apply();
+
+        // add a RT which hides the column we just inserted
+        rm = new RowMutation(KSNAME, key);
+        ColumnFamily cf = rm.addOrGet(CFNAME);
+        delete(cf, 0, 1, 1);
+        rm.apply();
+
+        // now re-insert that column
+        rm = new RowMutation(KSNAME, key);
+        add(rm, 1, 2);
+        rm.apply();
+
+        cfs.forceBlockingFlush();
+
+        // We should have 2 updates to the indexed "1" column
+        assertEquals(2, index.inserts.size());
+
+        CompactionManager.instance.performMaximal(cfs);
+
+        // verify that the "1" indexed column removed from the index twice:
+        // the first time by processing the RT, the second time by the
+        // re-indexing caused by the second insertion. This second write
+        // deletes from the 2i because the original column was still in the
+        // main cf's memtable (shadowed by the RT). One thing we're checking
+        // for here is that there wasn't an additional, bogus delete issued
+        // to the 2i (CASSANDRA-6517)
+        assertEquals(2, index.deletes.size());
+    }
+
     private void runCompactionWithRangeTombstoneAndCheckSecondaryIndex() throws Exception
     {
         Keyspace table = Keyspace.open(KSNAME);