cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From slebre...@apache.org
Subject [3/6] cassandra git commit: Exclude static columns from digest on empty static row
Date Thu, 30 Jun 2016 14:54:21 GMT
Exclude static columns from digest on empty static row

patch by Tommy Stendahl; reviewed by Sylvain Lebresne for CASSANDRA-12090


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

Branch: refs/heads/trunk
Commit: 4f14bc50d9037e01e709a925465d195eec4ab6df
Parents: b23fcc2
Author: Sylvain Lebresne <sylvain@datastax.com>
Authored: Thu Jun 30 13:38:36 2016 +0200
Committer: Sylvain Lebresne <sylvain@datastax.com>
Committed: Thu Jun 30 16:51:25 2016 +0200

----------------------------------------------------------------------
 CHANGES.txt                                           |  1 +
 .../org/apache/cassandra/db/PartitionColumns.java     |  7 -------
 .../cassandra/db/partitions/PartitionIterators.java   | 11 -----------
 .../org/apache/cassandra/db/rows/RowIterators.java    |  9 ++++++---
 .../cassandra/db/rows/UnfilteredRowIterators.java     | 14 +++++++++++++-
 5 files changed, 20 insertions(+), 22 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cassandra/blob/4f14bc50/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index 573f704..f12d704 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,4 +1,5 @@
 3.0.9
+ * Avoid digest mismatch with empty but static rows (CASSANDRA-12090)
  * Fix EOF exception when altering column type (CASSANDRA-11820)
 Merged from 2.2:
  * MemoryUtil.getShort() should return an unsigned short also for architectures not supporting
unaligned memory accesses (CASSANDRA-11973)

http://git-wip-us.apache.org/repos/asf/cassandra/blob/4f14bc50/src/java/org/apache/cassandra/db/PartitionColumns.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/PartitionColumns.java b/src/java/org/apache/cassandra/db/PartitionColumns.java
index 93204f4..bf4ac43 100644
--- a/src/java/org/apache/cassandra/db/PartitionColumns.java
+++ b/src/java/org/apache/cassandra/db/PartitionColumns.java
@@ -18,7 +18,6 @@
 package org.apache.cassandra.db;
 
 import java.util.*;
-import java.security.MessageDigest;
 
 import com.google.common.collect.Iterators;
 
@@ -136,12 +135,6 @@ public class PartitionColumns implements Iterable<ColumnDefinition>
         return Objects.hash(statics, regulars);
     }
 
-    public void digest(MessageDigest digest)
-    {
-        regulars.digest(digest);
-        statics.digest(digest);
-    }
-
     public static Builder builder()
     {
         return new Builder();

http://git-wip-us.apache.org/repos/asf/cassandra/blob/4f14bc50/src/java/org/apache/cassandra/db/partitions/PartitionIterators.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/partitions/PartitionIterators.java b/src/java/org/apache/cassandra/db/partitions/PartitionIterators.java
index cc90c40..9b7d7eb 100644
--- a/src/java/org/apache/cassandra/db/partitions/PartitionIterators.java
+++ b/src/java/org/apache/cassandra/db/partitions/PartitionIterators.java
@@ -78,17 +78,6 @@ public abstract class PartitionIterators
         return MorePartitions.extend(iterators.get(0), new Extend());
     }
 
-    public static void digest(PartitionIterator iterator, MessageDigest digest)
-    {
-        while (iterator.hasNext())
-        {
-            try (RowIterator partition = iterator.next())
-            {
-                RowIterators.digest(partition, digest);
-            }
-        }
-    }
-
     public static PartitionIterator singletonIterator(RowIterator iterator)
     {
         return new SingletonPartitionIterator(iterator);

http://git-wip-us.apache.org/repos/asf/cassandra/blob/4f14bc50/src/java/org/apache/cassandra/db/rows/RowIterators.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/rows/RowIterators.java b/src/java/org/apache/cassandra/db/rows/RowIterators.java
index 551edb8..ae051c0 100644
--- a/src/java/org/apache/cassandra/db/rows/RowIterators.java
+++ b/src/java/org/apache/cassandra/db/rows/RowIterators.java
@@ -37,10 +37,13 @@ public abstract class RowIterators
 
     public static void digest(RowIterator iterator, MessageDigest digest)
     {
-        // TODO: we're not computing digest the same way that old nodes so we'll need
-        // to pass the version we're computing the digest for and deal with that.
+        // TODO: we're not computing digest the same way that old nodes. This is
+        // currently ok as this is only used for schema digest and the is no exchange
+        // of schema digest between different versions. If this changes however,
+        // we'll need to agree on a version.
         digest.update(iterator.partitionKey().getKey().duplicate());
-        iterator.columns().digest(digest);
+        iterator.columns().regulars.digest(digest);
+        iterator.columns().statics.digest(digest);
         FBUtilities.updateWithBoolean(digest, iterator.isReverseOrder());
         iterator.staticRow().digest(digest);
 

http://git-wip-us.apache.org/repos/asf/cassandra/blob/4f14bc50/src/java/org/apache/cassandra/db/rows/UnfilteredRowIterators.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/rows/UnfilteredRowIterators.java b/src/java/org/apache/cassandra/db/rows/UnfilteredRowIterators.java
index ce5fffe..6af3944 100644
--- a/src/java/org/apache/cassandra/db/rows/UnfilteredRowIterators.java
+++ b/src/java/org/apache/cassandra/db/rows/UnfilteredRowIterators.java
@@ -118,7 +118,19 @@ public abstract class UnfilteredRowIterators
 
         digest.update(iterator.partitionKey().getKey().duplicate());
         iterator.partitionLevelDeletion().digest(digest);
-        iterator.columns().digest(digest);
+        iterator.columns().regulars.digest(digest);
+        // When serializing an iterator, we skip the static columns if the iterator has not
static row, even if the
+        // columns() object itself has some (the columns() is a superset of what the iterator
actually contains, and
+        // will correspond to the queried columns pre-serialization). So we must avoid taking
the satic column names
+        // into account if there is no static row or we'd have a digest mismatch between
depending on whether the digest
+        // is computed on an iterator that has been serialized or not (see CASSANDRA-12090)
+        // TODO: in practice we could completely skip digesting the columns since they are
more informative of what the
+        // iterator may contain, and digesting the actual content is enough. And in fact,
that would be more correct
+        // (since again, the columns could be different without the information represented
by the iterator being
+        // different), but removing them entirely is stricly speaking a breaking change (it
would create mismatches on
+        // upgrade) so we can only do on the next protocol version bump.
+        if (iterator.staticRow() != Rows.EMPTY_STATIC_ROW)
+            iterator.columns().statics.digest(digest);
         FBUtilities.updateWithBoolean(digest, iterator.isReverseOrder());
         iterator.staticRow().digest(digest);
 


Mime
View raw message