[
https://issues.apache.org/jira/browse/CASSANDRA-21340?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Michael Semb Wever updated CASSANDRA-21340:
-------------------------------------------
Test and Documentation Plan: ci
Status: Patch Available (was: Open)
> GROUP BY queries silently return incomplete results due to premature SRP abort
> ------------------------------------------------------------------------------
>
> Key: CASSANDRA-21340
> URL: https://issues.apache.org/jira/browse/CASSANDRA-21340
> Project: Apache Cassandra
> Issue Type: Bug
> Components: CQL/Semantics
> Reporter: C. Scott Andreas
> Priority: Normal
> Fix For: 6.0-alpha
>
> Attachments: GroupByShortReadProtectionTest.java
>
> Time Spent: 10m
> Remaining Estimate: 0h
>
> GROUP BY queries can silently return fewer groups than requested when
> tombstones distributed across replicas cause short read protection to trigger.
> The current implementation in trunk incorrectly calculates the remaining
> result count sought for a query. The SRP retry calculation in
> {{ShortReadPartitionsProtection#moreContents}} subtracts a pending row count
> from a group count, which can result in a negative count that results in SRP
> aborting prematurely.
> This issue is related to {{CASSANDRA-15459}} in that 15459 addressed an issue
> preventing SRP from triggering at all. It predated 15459 which addressed
> incomplete results due to SRP in part, but the mixup between counting rows
> and groups survived this fix.
> This looks like it's due to how
> {{ShortReadPartitionsProtection#moreContents}} is calculated:
> ShortReadPartitionsProtection#moreContents (152-154):
> {{int toQuery = command.limits().count() != DataLimits.NO_LIMIT}}
> {{ ? command.limits().count() -
> mergedResultCounter.rowsCounted()}}
> {{ : command.limits().perPartitionCount();}}
> Rather than subtracting {{{}mergedResultsCounter.rowsCounted(){}}}, should
> this calculation use {{mergedResultCounter.counted()}} to reflect the number
> of groups returned?
> {{Datalimits.CQLGroupByLimits#forShortReadRetry}} also looks a bit fishy.
> This method returns a new CQLLimits instance, but doesn't pass through the
> AggregationSpecification, GroupingState, and groupPerPartitionLimit -- should
> it?
> I'd propose this patch, but am not familiar enough with SRP internals to feel
> confident making the change myself.
> An in-JVM dtest demonstrating the SRP short-circuit and incomplete result
> count is attached (written with model assistance).
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
