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/07/28 08:22:28 UTC
[3/6] cassandra git commit: Lost counter writes in compact table and
static columns
Lost counter writes in compact table and static columns
patch by Sylvain Lebresne; reviewed by Aleksey Yeschenko for CASSANDRA-12219
Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo
Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/a11f210f
Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/a11f210f
Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/a11f210f
Branch: refs/heads/trunk
Commit: a11f210f133ef8026e278381d3a0b703ff7165fb
Parents: d6232e0
Author: Sylvain Lebresne <sy...@datastax.com>
Authored: Wed Jul 27 17:15:18 2016 +0200
Committer: Sylvain Lebresne <sy...@datastax.com>
Committed: Thu Jul 28 10:19:09 2016 +0200
----------------------------------------------------------------------
CHANGES.txt | 1 +
.../apache/cassandra/cql3/UpdateParameters.java | 17 +++++++++--------
.../apache/cassandra/db/CounterMutation.java | 3 ++-
.../db/partitions/PartitionUpdate.java | 20 ++++++++++++--------
4 files changed, 24 insertions(+), 17 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/cassandra/blob/a11f210f/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index 90f1ee9..b966078 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,4 +1,5 @@
3.0.9
+ * Lost counter writes in compact table and static columns (CASSANDRA-12219)
* AssertionError with MVs on updating a row that isn't indexed due to a null value (CASSANDRA-12247)
* Disable RR and speculative retry with EACH_QUORUM reads (CASSANDRA-11980)
* Add option to override compaction space check (CASSANDRA-12180)
http://git-wip-us.apache.org/repos/asf/cassandra/blob/a11f210f/src/java/org/apache/cassandra/cql3/UpdateParameters.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/cql3/UpdateParameters.java b/src/java/org/apache/cassandra/cql3/UpdateParameters.java
index 572365b..0c58097 100644
--- a/src/java/org/apache/cassandra/cql3/UpdateParameters.java
+++ b/src/java/org/apache/cassandra/cql3/UpdateParameters.java
@@ -158,14 +158,15 @@ public class UpdateParameters
{
assert ttl == LivenessInfo.NO_TTL;
- // In practice, the actual CounterId (and clock really) that we use doesn't matter, because we will
- // ignore it in CounterMutation when we do the read-before-write to create the actual value that is
- // applied. In other words, this is not the actual value that will be written to the memtable
- // because this will be replaced in CounterMutation.updateWithCurrentValue().
- // As an aside, since we don't care about the CounterId/clock, we used to only send the incremement,
- // but that makes things a bit more complex as this means we need to be able to distinguish inside
- // PartitionUpdate between counter updates that has been processed by CounterMutation and those that
- // haven't.
+ // Because column is a counter, we need the value to be a CounterContext. However, we're only creating a
+ // "counter update", which is a temporary state until we run into 'CounterMutation.updateWithCurrentValue()'
+ // which does the read-before-write and sets the proper CounterId, clock and updated value.
+ //
+ // We thus create a "fake" local shard here. The CounterId/clock used don't matter as this is just a temporary
+ // state that will be replaced when processing the mutation in CounterMutation, but the reason we use a 'local'
+ // shard is due to the merging rules: if a user includes multiple updates to the same counter in a batch, those
+ // multiple updates will be merged in the PartitionUpdate *before* they even reach CounterMutation. So we need
+ // such update to be added together, and that's what a local shard gives us.
builder.addCell(BufferCell.live(metadata, column, timestamp, CounterContext.instance().createLocal(increment)));
}
http://git-wip-us.apache.org/repos/asf/cassandra/blob/a11f210f/src/java/org/apache/cassandra/db/CounterMutation.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/CounterMutation.java b/src/java/org/apache/cassandra/db/CounterMutation.java
index 6818513..8aafa5c 100644
--- a/src/java/org/apache/cassandra/db/CounterMutation.java
+++ b/src/java/org/apache/cassandra/db/CounterMutation.java
@@ -238,7 +238,8 @@ public class CounterMutation implements IMutation
BTreeSet.Builder<Clustering> names = BTreeSet.builder(cfs.metadata.comparator);
for (PartitionUpdate.CounterMark mark : marks)
{
- names.add(mark.clustering());
+ if (mark.clustering() != Clustering.STATIC_CLUSTERING)
+ names.add(mark.clustering());
if (mark.path() == null)
builder.add(mark.column());
else
http://git-wip-us.apache.org/repos/asf/cassandra/blob/a11f210f/src/java/org/apache/cassandra/db/partitions/PartitionUpdate.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/partitions/PartitionUpdate.java b/src/java/org/apache/cassandra/db/partitions/PartitionUpdate.java
index 6331440..2a881a3 100644
--- a/src/java/org/apache/cassandra/db/partitions/PartitionUpdate.java
+++ b/src/java/org/apache/cassandra/db/partitions/PartitionUpdate.java
@@ -484,19 +484,23 @@ public class PartitionUpdate extends AbstractBTreePartition
assert metadata().isCounter();
maybeBuild();
// We will take aliases on the rows of this update, and update them in-place. So we should be sure the
- // update is no immutable for all intent and purposes.
+ // update is now immutable for all intent and purposes.
canReOpen = false;
- List<CounterMark> l = new ArrayList<>();
+ List<CounterMark> marks = new ArrayList<>();
+ addMarksForRow(staticRow(), marks);
for (Row row : this)
+ addMarksForRow(row, marks);
+ return marks;
+ }
+
+ private void addMarksForRow(Row row, List<CounterMark> marks)
+ {
+ for (Cell cell : row.cells())
{
- for (Cell cell : row.cells())
- {
- if (cell.isCounterCell())
- l.add(new CounterMark(row, cell.column(), cell.path()));
- }
+ if (cell.isCounterCell())
+ marks.add(new CounterMark(row, cell.column(), cell.path()));
}
- return l;
}
private void assertNotBuilt()