You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by jb...@apache.org on 2011/11/23 16:38:47 UTC

svn commit: r1205452 - in /cassandra/branches/cassandra-0.8: ./ src/java/org/apache/cassandra/db/ src/java/org/apache/cassandra/db/compaction/ test/unit/org/apache/cassandra/ test/unit/org/apache/cassandra/db/compaction/

Author: jbellis
Date: Wed Nov 23 15:38:44 2011
New Revision: 1205452

URL: http://svn.apache.org/viewvc?rev=1205452&view=rev
Log:
avoid dropping tombstones when they might still be needed to shadow data in another sstable
patch by slebresne and jbellis for CASSANDRA-2786

Modified:
    cassandra/branches/cassandra-0.8/CHANGES.txt
    cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/EchoedRow.java
    cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/compaction/CompactionController.java
    cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/compaction/LazilyCompactedRow.java
    cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/compaction/PrecompactedRow.java
    cassandra/branches/cassandra-0.8/test/unit/org/apache/cassandra/SchemaLoader.java
    cassandra/branches/cassandra-0.8/test/unit/org/apache/cassandra/db/compaction/CompactionsTest.java

Modified: cassandra/branches/cassandra-0.8/CHANGES.txt
URL: http://svn.apache.org/viewvc/cassandra/branches/cassandra-0.8/CHANGES.txt?rev=1205452&r1=1205451&r2=1205452&view=diff
==============================================================================
--- cassandra/branches/cassandra-0.8/CHANGES.txt (original)
+++ cassandra/branches/cassandra-0.8/CHANGES.txt Wed Nov 23 15:38:44 2011
@@ -1,4 +1,6 @@
 0.8.8
+ * avoid dropping tombstones when they might still be needed to shadow
+   data in a different sstable (CASSANDRA-2786)
  * fix truncate allowing data to be replayed post-restart (CASSANDRA-3297)
  * make iwriter final in IndexWriter to avoid NPE (CASSANDRA-2863)
  * (CQL) update grammar to require key clause in DELETE statement

Modified: cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/EchoedRow.java
URL: http://svn.apache.org/viewvc/cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/EchoedRow.java?rev=1205452&r1=1205451&r2=1205452&view=diff
==============================================================================
--- cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/EchoedRow.java (original)
+++ cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/EchoedRow.java Wed Nov 23 15:38:44 2011
@@ -36,13 +36,11 @@ import org.apache.cassandra.io.sstable.S
 public class EchoedRow extends AbstractCompactedRow
 {
     private final SSTableIdentityIterator row;
-    private final int gcBefore;
 
-    public EchoedRow(CompactionController controller, SSTableIdentityIterator row)
+    public EchoedRow(SSTableIdentityIterator row)
     {
         super(row.getKey());
         this.row = row;
-        this.gcBefore = controller.gcBefore;
         // Reset SSTableIdentityIterator because we have not guarantee the filePointer hasn't moved since the Iterator was built
         row.reset();
     }
@@ -62,7 +60,8 @@ public class EchoedRow extends AbstractC
 
     public boolean isEmpty()
     {
-        return !row.hasNext() && ColumnFamilyStore.removeDeletedCF(row.getColumnFamily(), gcBefore) == null;
+        // never okay to purge a EchoedRow -- if it were, we'd need to deserialize instead of echoing
+        return false;
     }
 
     public int columnCount()

Modified: cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/compaction/CompactionController.java
URL: http://svn.apache.org/viewvc/cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/compaction/CompactionController.java?rev=1205452&r1=1205451&r2=1205452&view=diff
==============================================================================
--- cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/compaction/CompactionController.java (original)
+++ cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/compaction/CompactionController.java Wed Nov 23 15:38:44 2011
@@ -31,7 +31,6 @@ import org.apache.cassandra.db.Decorated
 import org.apache.cassandra.db.EchoedRow;
 import org.apache.cassandra.io.sstable.SSTableIdentityIterator;
 import org.apache.cassandra.io.sstable.SSTableReader;
-import org.apache.cassandra.utils.ByteBufferUtil;
 
 /**
  * Manage compaction options.
@@ -87,6 +86,10 @@ public class CompactionController
         return cfs.columnFamily;
     }
 
+    /**
+     * @return true if it's okay to drop tombstones for the given row, i.e., if we know all the verisons of the row
+     * are included in the compaction set
+     */
     public boolean shouldPurge(DecoratedKey key)
     {
         return isMajor || !cfs.isKeyInRemainingSSTables(key, sstables);
@@ -131,7 +134,7 @@ public class CompactionController
     public AbstractCompactedRow getCompactedRow(List<SSTableIdentityIterator> rows)
     {
         if (rows.size() == 1 && !needDeserialize() && !shouldPurge(rows.get(0).getKey()))
-            return new EchoedRow(this, rows.get(0));
+            return new EchoedRow(rows.get(0));
 
         long rowSize = 0;
         for (SSTableIdentityIterator row : rows)

Modified: cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/compaction/LazilyCompactedRow.java
URL: http://svn.apache.org/viewvc/cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/compaction/LazilyCompactedRow.java?rev=1205452&r1=1205451&r2=1205452&view=diff
==============================================================================
--- cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/compaction/LazilyCompactedRow.java (original)
+++ cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/compaction/LazilyCompactedRow.java Wed Nov 23 15:38:44 2011
@@ -146,7 +146,9 @@ public class LazilyCompactedRow extends 
 
     public boolean isEmpty()
     {
-        boolean cfIrrelevant = ColumnFamilyStore.removeDeletedCF(emptyColumnFamily, controller.gcBefore) == null;
+        boolean cfIrrelevant = shouldPurge
+                             ? ColumnFamilyStore.removeDeletedCF(emptyColumnFamily, controller.gcBefore) == null
+                             : !emptyColumnFamily.isMarkedForDelete(); // tombstones are relevant
         return cfIrrelevant && columnCount == 0;
     }
 

Modified: cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/compaction/PrecompactedRow.java
URL: http://svn.apache.org/viewvc/cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/compaction/PrecompactedRow.java?rev=1205452&r1=1205451&r2=1205452&view=diff
==============================================================================
--- cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/compaction/PrecompactedRow.java (original)
+++ cassandra/branches/cassandra-0.8/src/java/org/apache/cassandra/db/compaction/PrecompactedRow.java Wed Nov 23 15:38:44 2011
@@ -45,14 +45,12 @@ public class PrecompactedRow extends Abs
     private static Logger logger = LoggerFactory.getLogger(PrecompactedRow.class);
 
     private final ColumnFamily compactedCf;
-    private final int gcBefore;
 
     // For testing purposes
     public PrecompactedRow(DecoratedKey key, ColumnFamily compacted)
     {
         super(key);
         this.compactedCf = compacted;
-        this.gcBefore = Integer.MAX_VALUE;
     }
 
     public static ColumnFamily removeDeletedAndOldShards(DecoratedKey key, CompactionController controller, ColumnFamily cf)
@@ -71,7 +69,6 @@ public class PrecompactedRow extends Abs
     public PrecompactedRow(CompactionController controller, List<SSTableIdentityIterator> rows)
     {
         super(rows.get(0).getKey());
-        gcBefore = controller.gcBefore;
         compactedCf = removeDeletedAndOldShards(rows.get(0).getKey(), controller, merge(rows));
     }
 
@@ -133,7 +130,7 @@ public class PrecompactedRow extends Abs
 
     public boolean isEmpty()
     {
-        return compactedCf == null || ColumnFamilyStore.removeDeletedCF(compactedCf, gcBefore) == null;
+        return compactedCf == null;
     }
 
     public int columnCount()

Modified: cassandra/branches/cassandra-0.8/test/unit/org/apache/cassandra/SchemaLoader.java
URL: http://svn.apache.org/viewvc/cassandra/branches/cassandra-0.8/test/unit/org/apache/cassandra/SchemaLoader.java?rev=1205452&r1=1205451&r2=1205452&view=diff
==============================================================================
--- cassandra/branches/cassandra-0.8/test/unit/org/apache/cassandra/SchemaLoader.java (original)
+++ cassandra/branches/cassandra-0.8/test/unit/org/apache/cassandra/SchemaLoader.java Wed Nov 23 15:38:44 2011
@@ -145,6 +145,7 @@ public class SchemaLoader
                                                           bytes,
                                                           bytes)
                                                    .defaultValidator(CounterColumnType.instance),
+                                           superCFMD(ks1, "SuperDirectGC", BytesType.instance).gcGraceSeconds(0),
                                            jdbcCFMD(ks1, "JdbcInteger", IntegerType.instance).columnMetadata(integerColumn),
                                            jdbcCFMD(ks1, "JdbcUtf8", UTF8Type.instance).columnMetadata(utf8Column),
                                            jdbcCFMD(ks1, "JdbcLong", LongType.instance),

Modified: cassandra/branches/cassandra-0.8/test/unit/org/apache/cassandra/db/compaction/CompactionsTest.java
URL: http://svn.apache.org/viewvc/cassandra/branches/cassandra-0.8/test/unit/org/apache/cassandra/db/compaction/CompactionsTest.java?rev=1205452&r1=1205451&r2=1205452&view=diff
==============================================================================
--- cassandra/branches/cassandra-0.8/test/unit/org/apache/cassandra/db/compaction/CompactionsTest.java (original)
+++ cassandra/branches/cassandra-0.8/test/unit/org/apache/cassandra/db/compaction/CompactionsTest.java Wed Nov 23 15:38:44 2011
@@ -213,15 +213,18 @@ public class CompactionsTest extends Cle
     public void testDontPurgeAccidentaly() throws IOException, ExecutionException, InterruptedException
     {
         // Testing with and without forcing deserialization. Without deserialization, EchoedRow will be used.
-        testDontPurgeAccidentaly("test1", false);
-        testDontPurgeAccidentaly("test2", true);
+        testDontPurgeAccidentaly("test1", "Super5", false);
+        testDontPurgeAccidentaly("test2", "Super5", true);
+
+        // Use CF with gc_grace=0, see last bug of CASSANDRA-2786
+        testDontPurgeAccidentaly("test1", "SuperDirectGC", false);
+        testDontPurgeAccidentaly("test2", "SuperDirectGC", true);
     }
 
-    private void testDontPurgeAccidentaly(String k, boolean forceDeserialize) throws IOException, ExecutionException, InterruptedException
+    private void testDontPurgeAccidentaly(String k, String cfname, boolean forceDeserialize) throws IOException, ExecutionException, InterruptedException
     {
         // This test catches the regression of CASSANDRA-2786
         Table table = Table.open(TABLE1);
-        String cfname = "Super5";
         ColumnFamilyStore store = table.getColumnFamilyStore(cfname);
 
         // disable compaction while flushing
@@ -247,7 +250,7 @@ public class CompactionsTest extends Cle
         rm.apply();
 
         ColumnFamily cf = store.getColumnFamily(filter);
-        assert cf.isEmpty() : "should be empty: " + cf;
+        assert cf == null || cf.isEmpty() : "should be empty: " + cf;
 
         store.forceBlockingFlush();
 
@@ -261,6 +264,6 @@ public class CompactionsTest extends Cle
         CompactionManager.instance.doCompactionWithoutSizeEstimation(store, toCompact, (int) (System.currentTimeMillis() / 1000) - store.metadata.getGcGraceSeconds(), location, forceDeserialize);
 
         cf = store.getColumnFamily(filter);
-        assert cf.isEmpty() : "should be empty: " + cf;
+        assert cf == null || cf.isEmpty() : "should be empty: " + cf;
     }
 }