cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From slebre...@apache.org
Subject [1/2] cassandra git commit: Properly handle static when dealing with range tombstone in backward compatibility/thrift code
Date Fri, 16 Oct 2015 13:54:28 GMT
Repository: cassandra
Updated Branches:
  refs/heads/trunk 4034dc904 -> 1bcf0c7b6


Properly handle static when dealing with range tombstone in backward compatibility/thrift
code

patch by slebresne; reviewed by iamaleksey for CASSANDRA-10174


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

Branch: refs/heads/trunk
Commit: e0ce1dd37055d9fdfb1d7c1bd300c23c47f80be4
Parents: d413ddf
Author: Sylvain Lebresne <sylvain@datastax.com>
Authored: Fri Sep 18 18:24:06 2015 +0200
Committer: Sylvain Lebresne <sylvain@datastax.com>
Committed: Fri Oct 16 15:51:43 2015 +0200

----------------------------------------------------------------------
 CHANGES.txt                                     |   1 +
 .../org/apache/cassandra/db/LegacyLayout.java   | 135 ++++++++++++++-----
 2 files changed, 104 insertions(+), 32 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cassandra/blob/e0ce1dd3/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index a53a299..e2d9dd7 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,4 +1,5 @@
 3.0-rc2
+ * Fix handling of static columns for range tombstones in thrift (CASSANDRA-10174)
  * Support empty ColumnFilter for backward compatility on empty IN (CASSANDRA-10471)
  * Remove Pig support (CASSANDRA-10542)
  * Fix LogFile throws Exception when assertion is disabled (CASSANDRA-10522)

http://git-wip-us.apache.org/repos/asf/cassandra/blob/e0ce1dd3/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 d57bc6b..194b6e8 100644
--- a/src/java/org/apache/cassandra/db/LegacyLayout.java
+++ b/src/java/org/apache/cassandra/db/LegacyLayout.java
@@ -506,13 +506,37 @@ public abstract class LegacyLayout
                                                                  boolean reversed,
                                                                  SerializationHelper helper)
     {
+        // A reducer that basically does nothing, we know the 2 merged iterators can't have
conflicting atoms (since we merge cells with range tombstones).
+        MergeIterator.Reducer<LegacyAtom, LegacyAtom> reducer = new MergeIterator.Reducer<LegacyAtom,
LegacyAtom>()
+        {
+            private LegacyAtom atom;
+
+            public void reduce(int idx, LegacyAtom current)
+            {
+                // We're merging cell with range tombstones, so we should always only have
a single atom to reduce.
+                assert atom == null;
+                atom = current;
+            }
+
+            protected LegacyAtom getReduced()
+            {
+                return atom;
+            }
+
+            protected void onKeyChange()
+            {
+                atom = null;
+            }
+        };
+        List<Iterator<LegacyAtom>> iterators = Arrays.asList(asLegacyAtomIterator(cells),
asLegacyAtomIterator(delInfo.inRowRangeTombstones()));
+        PeekingIterator<LegacyAtom> atoms = Iterators.peekingIterator(MergeIterator.get(iterators,
legacyAtomComparator(metadata), reducer));
+
         // Check if we have some static
-        PeekingIterator<LegacyCell> iter = Iterators.peekingIterator(cells);
-        Row staticRow = iter.hasNext() && iter.peek().name.clustering == Clustering.STATIC_CLUSTERING
-                      ? getNextRow(CellGrouper.staticGrouper(metadata, helper), iter)
+        Row staticRow = atoms.hasNext() && atoms.peek().isStatic()
+                      ? getNextRow(CellGrouper.staticGrouper(metadata, helper), atoms)
                       : Rows.EMPTY_STATIC_ROW;
 
-        Iterator<Row> rows = convertToRows(new CellGrouper(metadata, helper), iter,
delInfo);
+        Iterator<Row> rows = convertToRows(new CellGrouper(metadata, helper), atoms);
         Iterator<RangeTombstone> ranges = delInfo.deletionInfo.rangeIterator(reversed);
         return new RowAndDeletionMergeIterator(metadata,
                                                key,
@@ -587,34 +611,8 @@ public abstract class LegacyLayout
         return (Iterator<LegacyAtom>)iter;
     }
 
-    private static Iterator<Row> convertToRows(final CellGrouper grouper, final Iterator<LegacyCell>
cells, final LegacyDeletionInfo delInfo)
+    private static Iterator<Row> convertToRows(final CellGrouper grouper, final PeekingIterator<LegacyAtom>
atoms)
     {
-        // A reducer that basically does nothing, we know the 2 merge iterators can't have
conflicting atoms.
-        MergeIterator.Reducer<LegacyAtom, LegacyAtom> reducer = new MergeIterator.Reducer<LegacyAtom,
LegacyAtom>()
-        {
-            private LegacyAtom atom;
-
-            public void reduce(int idx, LegacyAtom current)
-            {
-                // We're merging cell with range tombstones, so we should always only have
a single atom to reduce.
-                assert atom == null;
-                atom = current;
-            }
-
-            protected LegacyAtom getReduced()
-            {
-                return atom;
-            }
-
-            protected void onKeyChange()
-            {
-                atom = null;
-            }
-        };
-        List<Iterator<LegacyAtom>> iterators = Arrays.asList(asLegacyAtomIterator(cells),
asLegacyAtomIterator(delInfo.inRowRangeTombstones()));
-        Iterator<LegacyAtom> merged = MergeIterator.get(iterators, legacyAtomComparator(grouper.metadata),
reducer);
-        final PeekingIterator<LegacyAtom> atoms = Iterators.peekingIterator(merged);
-
         return new AbstractIterator<Row>()
         {
             protected Row computeNext()
@@ -871,6 +869,14 @@ public abstract class LegacyLayout
             // First we want to compare by clustering, but we have to be careful with range
tombstone, because
             // we can have collection deletion and we want those to sort properly just before
the column they
             // delete, not before the whole row.
+            // We also want to special case static so they sort before any non-static. Note
in particular that
+            // this special casing is important in the case of one of the Atom being Slice.Bound.BOTTOM:
we want
+            // it to sort after the static as we deal with static first in toUnfilteredAtomIterator
and having
+            // Slice.Bound.BOTTOM first would mess that up (note that static deletion is
handled through a specific
+            // static tombstone, see LegacyDeletionInfo.add()).
+            if (o1.isStatic() != o2.isStatic())
+                return o1.isStatic() ? -1 : 1;
+
             ClusteringPrefix c1 = o1.clustering();
             ClusteringPrefix c2 = o2.clustering();
 
@@ -1290,6 +1296,9 @@ public abstract class LegacyLayout
 
         public Clustering getAsClustering(CFMetaData metadata)
         {
+            if (isStatic)
+                return Clustering.STATIC_CLUSTERING;
+
             assert bound.size() == metadata.comparator.size();
             ByteBuffer[] values = new ByteBuffer[bound.size()];
             for (int i = 0; i < bound.size(); i++)
@@ -1499,6 +1508,16 @@ public abstract class LegacyLayout
             return start.bound;
         }
 
+        public LegacyRangeTombstone withNewStart(LegacyBound newStart)
+        {
+            return new LegacyRangeTombstone(newStart, stop, deletionTime);
+        }
+
+        public LegacyRangeTombstone withNewEnd(LegacyBound newStop)
+        {
+            return new LegacyRangeTombstone(start, newStop, deletionTime);
+        }
+
         public boolean isCell()
         {
             return false;
@@ -1506,7 +1525,7 @@ public abstract class LegacyLayout
 
         public boolean isStatic()
         {
-            return start.isStatic;
+            return start.isStatic || stop.isStatic;
         }
 
         public LegacyCell asCell()
@@ -1565,8 +1584,60 @@ public abstract class LegacyLayout
             deletionInfo.add(topLevel);
         }
 
+        private static Slice.Bound staticBound(CFMetaData metadata, boolean isStart)
+        {
+            // In pre-3.0 nodes, static row started by a clustering with all empty values
so we
+            // preserve that here. Note that in practice, it doesn't really matter since
the rest
+            // of the code will ignore the bound for RT that have their static flag set.
+            ByteBuffer[] values = new ByteBuffer[metadata.comparator.size()];
+            for (int i = 0; i < values.length; i++)
+                values[i] = ByteBufferUtil.EMPTY_BYTE_BUFFER;
+            return isStart
+                 ? Slice.Bound.inclusiveStartOf(values)
+                 : Slice.Bound.inclusiveEndOf(values);
+        }
+
         public void add(CFMetaData metadata, LegacyRangeTombstone tombstone)
         {
+            if (metadata.hasStaticColumns())
+            {
+                /*
+                 * For table having static columns we have to deal with the following cases:
+                 *  1. the end of the tombstone is static (in which case either the start
is static or is BOTTOM, which is the same
+                 *     for our consideration). This mean that either the range only delete
the static row, or that it's a collection
+                 *     tombstone of a static collection. In both case, we just add the tombstone
to the inRowTombstones.
+                 *  2. only the start is static. There is then 2 subcase: either the start
is inclusive, and that mean we include the
+                 *     static row and more (so we add an inRowTombstone for the static and
deal with the rest normally). Or the start
+                 *     is exclusive, and that means we explicitely exclude the static (in
which case we can just add the tombstone
+                 *     as if it started at BOTTOM).
+                 *  3. none of the bound are static but the start is BOTTOM. This means we
intended to delete the static row so we
+                 *     need to add it to the inRowTombstones (and otherwise handle the range
normally).
+                 */
+                if (tombstone.stop.isStatic)
+                {
+                    // If the start is BOTTOM, we replace it by the beginning of the starting
row so as to not confuse the
+                    // RangeTombstone.isRowDeletion() method
+                    if (tombstone.start == LegacyBound.BOTTOM)
+                        tombstone = tombstone.withNewStart(new LegacyBound(staticBound(metadata,
true), true, null));
+                    inRowTombstones.add(tombstone);
+                    return;
+                }
+
+                if (tombstone.start.isStatic)
+                {
+                    if (tombstone.start.bound.isInclusive())
+                        inRowTombstones.add(tombstone.withNewEnd(new LegacyBound(staticBound(metadata,
false), true, null)));
+
+                    tombstone = tombstone.withNewStart(LegacyBound.BOTTOM);
+                }
+                else if (tombstone.start == LegacyBound.BOTTOM)
+                {
+                    inRowTombstones.add(new LegacyRangeTombstone(new LegacyBound(staticBound(metadata,
true), true, null),
+                                                                 new LegacyBound(staticBound(metadata,
false), true, null),
+                                                                 tombstone.deletionTime));
+                }
+            }
+
             if (tombstone.isCollectionTombstone() || tombstone.isRowDeletion(metadata))
                 inRowTombstones.add(tombstone);
             else


Mime
View raw message