[jira] [Commented] (CASSANDRA-13747) Fix AssertionError in short read protection

["Aleksey Yeschenko (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/CASSANDRA-13747?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16145148#comment-16145148 ] 

Aleksey Yeschenko commented on CASSANDRA-13747:
-----------------------------------------------

Didn’t get a clean CI run, but these are the issues flagged:

On 3.0 and 3.11, {{RemoveTest}} failed due to a port being used by another process. Passed locally, with and without compression.
On 3.0, {{ClientWarningsTest}} timed out. Passed locally.

5 dtest failures in 3.0 with unrelated failure reasons. Unfortunately currently unable to get a clean run on ASF Jenkins.

Committed to 3.0 as [a7cb009f8a3f4d0e0293111bfcfff3d404a37a89|https://github.com/apache/cassandra/commit/a7cb009f8a3f4d0e0293111bfcfff3d404a37a89] and merged with 3.11 and trunk.

> Fix AssertionError in short read protection
> -------------------------------------------
>
>                 Key: CASSANDRA-13747
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-13747
>             Project: Cassandra
>          Issue Type: Bug
>          Components: Coordination
>            Reporter: Aleksey Yeschenko
>            Assignee: Aleksey Yeschenko
>             Fix For: 3.0.15, 3.11.1
>
>
> {{ShortReadRowProtection.moreContents()}} expects that by the time we get to that method, the global post-reconciliation counter was already applied to the current partition. However, sometimes it won’t get applied, and the global counter continues counting with {{rowInCurrentPartition}} value not reset from previous partition, which in the most obvious case would trigger the assertion we are observing - {{assert !postReconciliationCounter.isDoneForPartition();}}. In other cases it’s possible because of this lack of reset to query a node for too few extra rows, causing unnecessary SRP data requests.
> Why is the counter not always applied to the current partition?
> The merged {{PartitionIterator}} returned from {{DataResolver.resolve()}} has two transformations applied to it, in the following order:
> {{Filter}} - to purge non-live data from partitions, and to discard empty partitions altogether (except for Thrift)
> {{Counter}}, to count and stop iteration
> Problem is, {{Filter}} ’s {{applyToPartition()}} code that discards empty partitions ({{closeIfEmpty()}} method) would sometimes consume the iterator, triggering short read protection *before* {{Counter}} ’s {{applyToPartition()}} gets called and resets its {{rowInCurrentPartition}} sub-counter.
> We should not be consuming iterators until all transformations are applied to them. For transformations it means that they cannot consume iterators unless they are the last transformation on the stack.
> The linked branch fixes the problem by splitting {{Filter}} into two transformations. The original - {{Filter}} - that does filtering within partitions - and a separate {{EmptyPartitionsDiscarder}}, that discards empty partitions from {{PartitionIterators}}. Thus {{DataResolve.resolve()}}, when constructing its {{PartitionIterator}}, now does merge first, then applies {{Filter}}, then {{Counter}}, and only then, as its last (third) transformation - the {{EmptyPartitionsDiscarder}}. Being the last one applied, it’s legal for it to consume the iterator, and triggering {{moreContents()}} is now no longer a problem.
> Fixes: [3.0|https://github.com/iamaleksey/cassandra/commits/13747-3.0], [3.11|https://github.com/iamaleksey/cassandra/commits/13747-3.11], [4.0|https://github.com/iamaleksey/cassandra/commits/13747-4.0]. dtest [here|https://github.com/iamaleksey/cassandra-dtest/commits/13747].



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cassandra.apache.org
For additional commands, e-mail: commits-help@cassandra.apache.org