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 2016/06/30 14:54:21 UTC
[3/6] cassandra git commit: Exclude static columns from digest on
empty static row
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 <sy...@datastax.com>
Authored: Thu Jun 30 13:38:36 2016 +0200
Committer: Sylvain Lebresne <sy...@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);