[
https://issues.apache.org/jira/browse/CASSANDRA-13691?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16088314#comment-16088314
]
Aleksey Yeschenko commented on CASSANDRA-13691:
-----------------------------------------------
Ended up using a special clock id for counter update contexts, so that we can
change for that clock id instead of merely looking at the shard type, to
unambiguously tell regular counter values from counter updates in 3.0.
Branches with the fix:
[3.0|https://github.com/iamaleksey/cassandra/tree/13691-3.0],
[3.11|https://github.com/iamaleksey/cassandra/tree/13691-3.11],
[4.0|https://github.com/iamaleksey/cassandra/tree/13691-4.0].
The commits include a small unit test to confirm that regular counter contexts
with old local shards in first spot are no longer recognized as counter
updates. Unfortunately it's a lot more painful given our existing framework to
create upgrade tests that would span the whole range from 2.0 to 3.0, due to
differences in supported driver protocol version.
Steps for manual reproduction and fix verification:
1. generate some 2.0 counter columns with local shards
{code}
ccm create test -n 3 -s -v 2.0.17
ccm node1 cqlsh
cqlsh> CREATE KEYSPACE test WITH replication = {'class': 'SimpleStrategy',
'replication_factor': 3};
cqlsh> CREATE TABLE test.test (id int PRIMARY KEY, v1 counter, v2 counter, v3
counter);
python
>>> from cassandra.cluster import Cluster
>>> cluster = Cluster(['127.0.0.1', '127.0.0.2', '127.0.0.3'])
>>> session = cluster.connect()
>>> query = "UPDATE test.test SET v1 = v1 + 1, v2 = v2 + 1, v3 = v3 + 1 where
>>> id = ?"
>>> prepared = session.prepare(query)
>>> for i in range(0, 1000):
... session.execute(prepared, [i])
...
ccm flush
{code}
2. upgrade cluster to 2.1
{code}
ccm stop
ccm setdir -v 2.1.17
ccm start
ccm node1 nodetool upgradesstables
ccm node2 nodetool upgradesstables
ccm node3 nodetool upgradesstables
{code}
3. upgrade node3 to 3.0
{code}
ccm node3 stop
ccm node3 setdir -v 3.0.14
ccm node3 start
{code}
4. with a 2.1 coordinator, try to read the table with CL.ALL
{code}
ccm node1 cqlsh
cqlsh> CONSISTENCY ALL;
cqlsh> SELECT COUNT(*) FROM test.test;
ServerError: <ErrorMessage code=0000 [Server error]
message="java.lang.AssertionError: Wrong class type: class
org.apache.cassandra.db.BufferCounterUpdateCell">
{code}
5. upgrade node3 to patched 3.0
{code}
ccm node3 stop
ccm node3 setdir --install-dir=/Users/aleksey/Code/cassandra
ccm node3 start
{code}
6. with a 2.1 coordinator, try again to read the table with CL.ALL
{code}
ccm node1 cqlsh
cqlsh> CONSISTENCY ALL;
cqlsh> SELECT COUNT(*) FROM test.test;
count
-------
1000
(1 rows)
{code}
I'll try to automate it as an upgrade test, too, but for now the manual steps
and the unit test will have to do.
> Fix incorrect [2.1 <— 3.0] serialization of counter cells with pre-2.1 local
> shards
> -----------------------------------------------------------------------------------
>
> Key: CASSANDRA-13691
> URL: https://issues.apache.org/jira/browse/CASSANDRA-13691
> Project: Cassandra
> Issue Type: Bug
> Components: Coordination
> Reporter: Aleksey Yeschenko
> Assignee: Aleksey Yeschenko
> Labels: counters, upgrade
> Fix For: 3.0.x, 3.11.x
>
>
> We stopped generating local shards in C* 2.1, after CASSANDRA-6504 (Counters
> 2.0). But it’s still possible to have counter cell values
> around, remaining from 2.0 times, on 2.1, 3.0, 3.11, and even trunk nodes, if
> they’ve never been overwritten.
> In 2.1, we used two classes for two kinds of counter columns:
> {{CounterCell}} class to store counters - internally as collections of
> {{CounterContext}} blobs, encoding collections of (host id, count, clock)
> tuples
> {{CounterUpdateCell}} class to represent unapplied increments - essentially a
> single long value; this class was never written to commit log, memtables, or
> sstables, and was only used inside {{Mutation}} object graph - in memory, and
> marshalled over network in cases when counter write coordinator and counter
> write leader were different nodes
> 3.0 got rid of {{CounterCell}} and {{CounterUpdateCell}}, among other
> {{Cell}} classes. In order to represent these unapplied increments -
> equivalents of 2.1 {{CounterUpdateCell}} - in 3.0 we encode them as regular
> counter columns, with a ‘special’ {{CounterContext}} value. I.e. a counter
> context with a single local shard. We do that so that we can reuse local
> shard reconcile logic (summing up) to seamlessly support counters with same
> names collapsing to single increments in batches. See
> {{UpdateParameters.addCounter()}} method comments
> [here|https://github.com/apache/cassandra/blob/cassandra-3.0.14/src/java/org/apache/cassandra/cql3/UpdateParameters.java#L157-L171]
> for details. It also assumes that nothing else can generate a counter with
> local shards.
> It works fine in pure 3.0 clusters, and in mixed 2.1/3.0 clusters, assuming
> that there are no counters with legacy local shards remaining from 2.0 era.
> It breaks down badly if there are.
> {{LegacyLayout.serializeAsLegacyPartition()}} and consequently
> {{LegacyCell.isCounterUpdate()}} - classes responsible for serializing and
> deserialising in 2.1 format for compatibility - use the following logic to
> tell if a cell of {{COUNTER}} kind is a regular final counter or an unapplied
> increment:
> {code}
> private boolean isCounterUpdate()
> {
> // See UpdateParameters.addCounter() for more details on this
> return isCounter() && CounterContext.instance().isLocal(value);
> }
> {code}
> {{CounterContext.isLocal()}} method here looks at the first shard of the
> collection of tuples and returns true if it’s a local one.
> This method would correctly identify a cell generated by
> {{UpdateParameters.addCounter()}} as a counter update and serialize it
> correctly as a 2.1 {{CounterUpdateCell}}. However, it would also incorrectly
> flag any regular counter cell that just so happens to have a local shard as
> the first tuple of the counter context as a counter update. If a 2.1 node as
> a coordinator of a read requests fetches such a value from a 3.0 node, during
> a rolling upgrade, instead of the expected {{CounterCell}} object it will
> receive a {{CounterUpdateCell}}, breaking all the things. In the best case
> scenario it will cause an assert in {{AbstractCell.reconcileCounter()}} to be
> raised.
> To fix the problem we must find an unambiguous way, without false positives
> or false negatives, to represent and identify unapplied counter updates on
> 3.0 side.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
