Repository: cassandra
Updated Branches:
  refs/heads/cassandra-2.0 1b1acae9a -> e2ad8969c
  refs/heads/cassandra-2.1 f7856c225 -> 89a8f4a7e
  refs/heads/trunk 9c482fdcf -> 05a86fd85


Fix MT mismatch between empty and gc-able data

patch by Stefan Podkowinski; reviewed by yukim for CASSANDRA-8979


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

Branch: refs/heads/cassandra-2.0
Commit: e2ad8969cc38a697c6b8176070c1774f4d68af9b
Parents: 1b1acae
Author: Stefan Podkowinski <[email protected]>
Authored: Fri Mar 27 10:04:08 2015 -0500
Committer: Yuki Morishita <[email protected]>
Committed: Fri Mar 27 13:30:40 2015 -0500

----------------------------------------------------------------------
 CHANGES.txt                                     |  1 +
 .../db/compaction/LazilyCompactedRow.java       |  6 ++-
 .../db/compaction/PrecompactedRow.java          |  7 +++-
 .../org/apache/cassandra/repair/Validator.java  | 19 ++++++++--
 .../apache/cassandra/repair/ValidatorTest.java  | 40 +++++++++++++++++++-
 5 files changed, 67 insertions(+), 6 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cassandra/blob/e2ad8969/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index adc0d59..9494f61 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -23,6 +23,7 @@
  * Add ability to limit number of native connections (CASSANDRA-8086)
  * Fix CQLSSTableWriter throwing exception and spawning threads
    (CASSANDRA-8808)
+ * Fix MT mismatch between empty and GC-able data (CASSANDRA-8979)
 
 
 2.0.13:

http://git-wip-us.apache.org/repos/asf/cassandra/blob/e2ad8969/src/java/org/apache/cassandra/db/compaction/LazilyCompactedRow.java
----------------------------------------------------------------------
diff --git 
a/src/java/org/apache/cassandra/db/compaction/LazilyCompactedRow.java 
b/src/java/org/apache/cassandra/db/compaction/LazilyCompactedRow.java
index 2757411..b562ba5 100644
--- a/src/java/org/apache/cassandra/db/compaction/LazilyCompactedRow.java
+++ b/src/java/org/apache/cassandra/db/compaction/LazilyCompactedRow.java
@@ -141,7 +141,11 @@ public class LazilyCompactedRow extends 
AbstractCompactedRow implements Iterable
         try
         {
             
DeletionTime.serializer.serialize(emptyColumnFamily.deletionInfo().getTopLevelDeletion(),
 out);
-            digest.update(out.getData(), 0, out.getLength());
+            // do not update digest in case of missing or purged row level 
tombstones, see CASSANDRA-8979
+            if (emptyColumnFamily.deletionInfo().getTopLevelDeletion() != 
DeletionTime.LIVE)
+            {
+                digest.update(out.getData(), 0, out.getLength());
+            }
         }
         catch (IOException e)
         {

http://git-wip-us.apache.org/repos/asf/cassandra/blob/e2ad8969/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 db72847..4627fa2 100644
--- a/src/java/org/apache/cassandra/db/compaction/PrecompactedRow.java
+++ b/src/java/org/apache/cassandra/db/compaction/PrecompactedRow.java
@@ -161,7 +161,12 @@ public class PrecompactedRow extends AbstractCompactedRow
         try
         {
             
DeletionTime.serializer.serialize(compactedCf.deletionInfo().getTopLevelDeletion(),
 buffer);
-            digest.update(buffer.getData(), 0, buffer.getLength());
+
+            // do not update digest in case of missing or purged row level 
tombstones, see CASSANDRA-8979
+            if (compactedCf.deletionInfo().getTopLevelDeletion() != 
DeletionTime.LIVE)
+            {
+                digest.update(buffer.getData(), 0, buffer.getLength());
+            }
         }
         catch (IOException e)
         {

http://git-wip-us.apache.org/repos/asf/cassandra/blob/e2ad8969/src/java/org/apache/cassandra/repair/Validator.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/repair/Validator.java 
b/src/java/org/apache/cassandra/repair/Validator.java
index abf5eac..5aa0cfe 100644
--- a/src/java/org/apache/cassandra/repair/Validator.java
+++ b/src/java/org/apache/cassandra/repair/Validator.java
@@ -24,9 +24,9 @@ import java.util.List;
 import java.util.Random;
 
 import com.google.common.annotations.VisibleForTesting;
+
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
-
 import org.apache.cassandra.concurrent.Stage;
 import org.apache.cassandra.concurrent.StageManager;
 import org.apache.cassandra.config.DatabaseDescriptor;
@@ -37,6 +37,7 @@ import org.apache.cassandra.net.MessagingService;
 import org.apache.cassandra.repair.messages.ValidationComplete;
 import org.apache.cassandra.utils.FBUtilities;
 import org.apache.cassandra.utils.MerkleTree;
+import org.apache.cassandra.utils.MerkleTree.RowHash;
 
 /**
  * Handles the building of a merkle tree for a column family.
@@ -148,7 +149,11 @@ public class Validator implements Runnable
         }
 
         // case 3 must be true: mix in the hashed row
-        range.addHash(rowHash(row));
+        RowHash rowHash = rowHash(row);
+        if (rowHash != null)
+        {
+            range.addHash(rowHash);
+        }
     }
 
     static class CountingDigest extends MessageDigest
@@ -196,7 +201,15 @@ public class Validator implements Runnable
         // MerkleTree uses XOR internally, so we want lots of output bits here
         CountingDigest digest = new 
CountingDigest(FBUtilities.newMessageDigest("SHA-256"));
         row.update(digest);
-        return new MerkleTree.RowHash(row.key.token, digest.digest(), 
digest.count);
+        // only return new hash for merkle tree in case digest was updated - 
see CASSANDRA-8979
+        if (digest.count > 0)
+        {
+            return new MerkleTree.RowHash(row.key.token, digest.digest(), 
digest.count);
+        }
+        else
+        {
+            return null;
+        }
     }
 
     /**

http://git-wip-us.apache.org/repos/asf/cassandra/blob/e2ad8969/test/unit/org/apache/cassandra/repair/ValidatorTest.java
----------------------------------------------------------------------
diff --git a/test/unit/org/apache/cassandra/repair/ValidatorTest.java 
b/test/unit/org/apache/cassandra/repair/ValidatorTest.java
index 9fa5d89..2a7651c 100644
--- a/test/unit/org/apache/cassandra/repair/ValidatorTest.java
+++ b/test/unit/org/apache/cassandra/repair/ValidatorTest.java
@@ -18,16 +18,18 @@
 package org.apache.cassandra.repair;
 
 import java.net.InetAddress;
+import java.util.List;
 import java.util.UUID;
 
 import org.junit.After;
 import org.junit.Test;
-
 import org.apache.cassandra.SchemaLoader;
 import org.apache.cassandra.db.ColumnFamilyStore;
 import org.apache.cassandra.db.DecoratedKey;
 import org.apache.cassandra.db.Keyspace;
 import org.apache.cassandra.db.TreeMapBackedSortedColumns;
+import org.apache.cassandra.db.compaction.CompactionController;
+import org.apache.cassandra.db.compaction.LazilyCompactedRow;
 import org.apache.cassandra.db.compaction.PrecompactedRow;
 import org.apache.cassandra.dht.IPartitioner;
 import org.apache.cassandra.dht.Range;
@@ -41,6 +43,8 @@ import org.apache.cassandra.repair.messages.RepairMessage;
 import org.apache.cassandra.repair.messages.ValidationComplete;
 import org.apache.cassandra.service.StorageService;
 import org.apache.cassandra.utils.ByteBufferUtil;
+import org.apache.cassandra.utils.MerkleTree;
+import org.apache.cassandra.utils.MerkleTree.TreeRange;
 import org.apache.cassandra.utils.SimpleCondition;
 
 import static org.junit.Assert.assertFalse;
@@ -122,6 +126,40 @@ public class ValidatorTest extends SchemaLoader
     }
 
     @Test
+    public void testPurgedVsNonExisting() throws Throwable
+    {
+        Range<Token> range = new Range<>(partitioner.getMinimumToken(), 
partitioner.getRandomToken());
+        final RepairJobDesc desc = new RepairJobDesc(UUID.randomUUID(), 
keyspace, columnFamily, range);
+
+        InetAddress remote = InetAddress.getByName("127.0.0.2");
+
+        ColumnFamilyStore cfs = 
Keyspace.open(keyspace).getColumnFamilyStore(columnFamily);
+
+        Token mid = partitioner.midpoint(range.left, range.right);
+        DecoratedKey key = new DecoratedKey(mid, 
ByteBufferUtil.bytes("inconceivable!"));
+
+        // create validator with zero rows
+        Validator validator1 = new Validator(desc, remote, 0);
+        validator1.prepare(cfs);
+        validator1.complete();
+
+        LazilyCompactedRow row3 = new LazilyCompactedRow(new 
CompactionController(cfs, null, 0), null);
+        // create validator with a single row with null cf
+        Validator validator2 = new Validator(desc, remote, 0);
+        validator2.prepare(cfs);
+        // a precompacted row with a cf null value indicates that there are no 
columns or tombstones left for this row
+        // this should give us the identical hash compared to the case as if 
the row would not have been added at all
+        // as with validator1
+        PrecompactedRow row2 = new PrecompactedRow(key, null);
+        validator2.add(row2);
+        validator2.complete();
+
+        // confirm that both trees are equal
+        List<TreeRange> diff = MerkleTree.difference(validator1.tree, 
validator2.tree);
+        assertTrue("Found tree mismatch: " + diff, diff.size() == 0);
+    }
+
+    @Test
     public void testValidatorFailed() throws Throwable
     {
         Range<Token> range = new Range<>(partitioner.getMinimumToken(), 
partitioner.getRandomToken());

Reply via email to