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 2015/08/25 14:44:55 UTC

cassandra git commit: Fix dealing with in-row RTs from thrift

Repository: cassandra
Updated Branches:
  refs/heads/cassandra-3.0 cfb7ec17f -> d96d17588


Fix dealing with in-row RTs from thrift

patch by slebresne; reviewed by iamaleksey for CASSANDRA-10046


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

Branch: refs/heads/cassandra-3.0
Commit: d96d1758813c2800cee53b990463f3dc5d9565ad
Parents: cfb7ec1
Author: Sylvain Lebresne <sy...@datastax.com>
Authored: Sun Aug 16 20:00:52 2015 +0200
Committer: Sylvain Lebresne <sy...@datastax.com>
Committed: Tue Aug 25 14:44:32 2015 +0200

----------------------------------------------------------------------
 CHANGES.txt                                     |  1 +
 .../org/apache/cassandra/db/LegacyLayout.java   | 58 +++++++++-----------
 .../cassandra/thrift/CassandraServer.java       | 17 +++---
 3 files changed, 35 insertions(+), 41 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cassandra/blob/d96d1758/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index 508f869..14f48cb 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,4 +1,5 @@
 3.0.0-beta2
+ * Fix incorrect handling of range tombstones in thrift (CASSANDRA-10046)
  * Only use batchlog when paired materialized view replica is remote (CASSANDRA-10061)
  * Reuse TemporalRow when updating multiple MaterializedViews (CASSANDRA-10060)
  * Validate gc_grace_seconds for batchlog writes and MVs (CASSANDRA-9917)

http://git-wip-us.apache.org/repos/asf/cassandra/blob/d96d1758/src/java/org/apache/cassandra/db/LegacyLayout.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/LegacyLayout.java b/src/java/org/apache/cassandra/db/LegacyLayout.java
index 3900d96..1c72d31 100644
--- a/src/java/org/apache/cassandra/db/LegacyLayout.java
+++ b/src/java/org/apache/cassandra/db/LegacyLayout.java
@@ -465,11 +465,11 @@ public abstract class LegacyLayout
     // For thrift sake
     public static UnfilteredRowIterator toUnfilteredRowIterator(CFMetaData metadata,
                                                                 DecoratedKey key,
-                                                                DeletionInfo delInfo,
+                                                                LegacyDeletionInfo delInfo,
                                                                 Iterator<LegacyCell> cells)
     {
         SerializationHelper helper = new SerializationHelper(metadata, 0, SerializationHelper.Flag.LOCAL);
-        return toUnfilteredRowIterator(metadata, key, LegacyDeletionInfo.from(delInfo), cells, false, helper);
+        return toUnfilteredRowIterator(metadata, key, delInfo, cells, false, helper);
     }
 
     // For deserializing old wire format
@@ -1404,8 +1404,7 @@ public abstract class LegacyLayout
         public final LegacyBound stop;
         public final DeletionTime deletionTime;
 
-        // Do not use directly, use create() instead.
-        private LegacyRangeTombstone(LegacyBound start, LegacyBound stop, DeletionTime deletionTime)
+        public LegacyRangeTombstone(LegacyBound start, LegacyBound stop, DeletionTime deletionTime)
         {
             // Because of the way RangeTombstoneList work, we can have a tombstone where only one of
             // the bound has a collectionName. That happens if we have a big tombstone A (spanning one
@@ -1488,33 +1487,35 @@ public abstract class LegacyLayout
 
     public static class LegacyDeletionInfo
     {
-        public final DeletionInfo deletionInfo;
-        public final List<LegacyRangeTombstone> inRowTombstones;
+        public final MutableDeletionInfo deletionInfo;
+        public final List<LegacyRangeTombstone> inRowTombstones = new ArrayList<>();
 
-        private LegacyDeletionInfo(DeletionInfo deletionInfo, List<LegacyRangeTombstone> inRowTombstones)
+        private LegacyDeletionInfo(MutableDeletionInfo deletionInfo)
         {
             this.deletionInfo = deletionInfo;
-            this.inRowTombstones = inRowTombstones;
         }
 
-        public static LegacyDeletionInfo from(DeletionInfo info)
+        public static LegacyDeletionInfo live()
         {
-            List<LegacyRangeTombstone> rangeTombstones = new ArrayList<>(info.rangeCount());
-            Iterator<RangeTombstone> iterator = info.rangeIterator(false);
-            while (iterator.hasNext())
-            {
-                RangeTombstone rt = iterator.next();
-                Slice slice = rt.deletedSlice();
-                rangeTombstones.add(new LegacyRangeTombstone(new LegacyBound(slice.start(), false, null),
-                                                             new LegacyBound(slice.end(), false, null),
-                                                             rt.deletionTime()));
-            }
-            return new LegacyDeletionInfo(info, rangeTombstones);
+            return new LegacyDeletionInfo(MutableDeletionInfo.live());
         }
 
-        public static LegacyDeletionInfo live()
+        public void add(DeletionTime topLevel)
         {
-            return from(DeletionInfo.LIVE);
+            deletionInfo.add(topLevel);
+        }
+
+        public void add(CFMetaData metadata, LegacyRangeTombstone tombstone)
+        {
+            if (tombstone.isCollectionTombstone() || tombstone.isRowDeletion(metadata))
+                inRowTombstones.add(tombstone);
+            else
+                add(metadata, new RangeTombstone(Slice.make(tombstone.start.bound, tombstone.stop.bound), tombstone.deletionTime));
+        }
+
+        public void add(CFMetaData metadata, RangeTombstone tombstone)
+        {
+            deletionInfo.add(tombstone, metadata.comparator);
         }
 
         public Iterator<LegacyRangeTombstone> inRowRangeTombstones()
@@ -1528,10 +1529,9 @@ public abstract class LegacyLayout
 
             int rangeCount = in.readInt();
             if (rangeCount == 0)
-                return from(new MutableDeletionInfo(topLevel));
+                return new LegacyDeletionInfo(new MutableDeletionInfo(topLevel));
 
-            RangeTombstoneList ranges = new RangeTombstoneList(metadata.comparator, rangeCount);
-            List<LegacyRangeTombstone> inRowTombsones = new ArrayList<>();
+            LegacyDeletionInfo delInfo = new LegacyDeletionInfo(new MutableDeletionInfo(topLevel));
             for (int i = 0; i < rangeCount; i++)
             {
                 LegacyBound start = decodeBound(metadata, ByteBufferUtil.readWithShortLength(in), true);
@@ -1539,13 +1539,9 @@ public abstract class LegacyLayout
                 int delTime =  in.readInt();
                 long markedAt = in.readLong();
 
-                LegacyRangeTombstone tombstone = new LegacyRangeTombstone(start, end, new DeletionTime(markedAt, delTime));
-                if (tombstone.isCollectionTombstone() || tombstone.isRowDeletion(metadata))
-                    inRowTombsones.add(tombstone);
-                else
-                    ranges.add(start.bound, end.bound, markedAt, delTime);
+                delInfo.add(metadata, new LegacyRangeTombstone(start, end, new DeletionTime(markedAt, delTime)));
             }
-            return new LegacyDeletionInfo(new MutableDeletionInfo(topLevel, ranges), inRowTombsones);
+            return delInfo;
         }
     }
 

http://git-wip-us.apache.org/repos/asf/cassandra/blob/d96d1758/src/java/org/apache/cassandra/thrift/CassandraServer.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/thrift/CassandraServer.java b/src/java/org/apache/cassandra/thrift/CassandraServer.java
index e827e95..fc1b638 100644
--- a/src/java/org/apache/cassandra/thrift/CassandraServer.java
+++ b/src/java/org/apache/cassandra/thrift/CassandraServer.java
@@ -1118,7 +1118,7 @@ public class CassandraServer implements Cassandra.Iface
                 if (metadata.isCounter())
                     ThriftConversion.fromThrift(consistency_level).validateCounterForWrite(metadata);
 
-                MutableDeletionInfo delInfo = MutableDeletionInfo.live();
+                LegacyLayout.LegacyDeletionInfo delInfo = LegacyLayout.LegacyDeletionInfo.live();
                 List<LegacyLayout.LegacyCell> cells = new ArrayList<>();
                 for (Mutation m : muts)
                 {
@@ -1199,12 +1199,12 @@ public class CassandraServer implements Cassandra.Iface
         }
     }
 
-    private void addRange(CFMetaData cfm, MutableDeletionInfo delInfo, Slice.Bound start, Slice.Bound end, long timestamp, int nowInSec)
+    private void addRange(CFMetaData cfm, LegacyLayout.LegacyDeletionInfo delInfo, Slice.Bound start, Slice.Bound end, long timestamp, int nowInSec)
     {
-        delInfo.add(new RangeTombstone(Slice.make(start, end), new DeletionTime(timestamp, nowInSec)), cfm.comparator);
+        delInfo.add(cfm, new RangeTombstone(Slice.make(start, end), new DeletionTime(timestamp, nowInSec)));
     }
 
-    private void deleteColumnOrSuperColumn(MutableDeletionInfo delInfo, List<LegacyLayout.LegacyCell> cells, CFMetaData cfm, Deletion del, int nowInSec)
+    private void deleteColumnOrSuperColumn(LegacyLayout.LegacyDeletionInfo delInfo, List<LegacyLayout.LegacyCell> cells, CFMetaData cfm, Deletion del, int nowInSec)
     throws InvalidRequestException
     {
         if (del.predicate != null && del.predicate.column_names != null)
@@ -1230,12 +1230,9 @@ public class CassandraServer implements Cassandra.Iface
         {
             if (del.super_column == null)
             {
-                addRange(cfm,
-                         delInfo,
-                         LegacyLayout.decodeBound(cfm, del.predicate.getSlice_range().start, true).bound,
-                         LegacyLayout.decodeBound(cfm, del.predicate.getSlice_range().finish, false).bound,
-                         del.timestamp,
-                         nowInSec);
+                LegacyLayout.LegacyBound start = LegacyLayout.decodeBound(cfm, del.predicate.getSlice_range().start, true);
+                LegacyLayout.LegacyBound end = LegacyLayout.decodeBound(cfm, del.predicate.getSlice_range().finish, false);
+                delInfo.add(cfm, new LegacyLayout.LegacyRangeTombstone(start, end, new DeletionTime(del.timestamp, nowInSec)));
             }
             else
             {