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/cassandra-3.9 Commit: 4f14bc50d9037e01e709a925465d195eec4ab6df Parents: b23fcc2 Author: Sylvain Lebresne <sylv...@datastax.com> Authored: Thu Jun 30 13:38:36 2016 +0200 Committer: Sylvain Lebresne <sylv...@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);