[jira] [Commented] (CASSANDRA-21330) EACH_QUORUM writes fail during topology changes - DatacenterSyncWriteResponseHandler doesn't call collectSuccess

["Ariel Weisberg (Jira)" <ji...@apache.org>]

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

Ariel Weisberg commented on CASSANDRA-21330:
--------------------------------------------

Need CASSANDRA-20993 for this to work in a lot of cases. The expanded QUORUM will auto-fail.

> EACH_QUORUM writes fail during topology changes - DatacenterSyncWriteResponseHandler doesn't call collectSuccess
> ----------------------------------------------------------------------------------------------------------------
>
>                 Key: CASSANDRA-21330
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-21330
>             Project: Apache Cassandra
>          Issue Type: Bug
>          Components: Consistency/Coordination
>            Reporter: Ariel Weisberg
>            Priority: Normal
>         Attachments: EachQuorumDuringReplaceTest.java
>
>
> Found two issues causing spurious failures during host replacement. The first is a clear bug in {{DatacenterSyncWriteResponseHandler}}. The second is a questionable use of {{stillAppliesTo}} in the blocking read repair path. Pending quorum inflation (CASSANDRA-20993) is also a factor but tracked separately.
> h3. 1) DatacenterSyncWriteResponseHandler never calls collectSuccess
> {{DatacenterSyncWriteResponseHandler.onResponse()}} doesn't call {{replicaPlan.collectSuccess()}}. {{WriteResponseHandler.onResponse()}} does — it's right there at the top of the method. The DSWRH version just... doesn't have it.
> This means the {{contacted}} list in the {{ForWrite}} plan is always empty for EACH_QUORUM. That list is used by {{ForWrite.stillAppliesTo()}} after the write completes — it recomputes the plan at the current epoch and checks whether the responses we already got are still valid. It iterates {{contacted}} and decrements {{writeQuorum}} for each responder still in the new {{liveAndDown}}. With an empty {{contacted}}, nothing gets decremented, and it throws every time the epoch has changed and the placements differ.
> The practical effect is that any EACH_QUORUM write in flight during a host replacement step ({{StartReplace}}, {{MidReplace}}, {{FinishReplace}} — anything that changes placements) will fail with {{IllegalStateException}}. Doesn't matter that all the replicas responded fine.
> The fix is one line — add the {{collectSuccess}} call to match {{WriteResponseHandler}}.
> I put together a dtest ({{EachQuorumDuringReplaceTest}}) that reproduces this. It runs EACH_QUORUM and QUORUM writes in a loop during a replacement. EACH_QUORUM fails, QUORUM doesn't. With the fix applied the ISE goes away (only remaining failure is a write timeout from CASSANDRA-20993).
> h3. 2) BlockingReadRepair.awaitWrites vs stillAppliesTo
> Also found 3 QUORUM ISEs from {{BlockingReadRepair.awaitWrites()}} — these are read-repair writes that failed during SELECTs. Different root cause. This is a spurious error, not a correctness problem — read monotonicity is not broken.
> Read-repair only sends mutations to the stale replicas. {{BlockingPartitionRepair.onResponse()}} does call {{collectSuccess}}, but only for the replicas that actually got repair mutations. The repair plan's {{contacts}} include all replicas from the read (enough for write quorum), but most of them don't need repair and never trigger {{onResponse}}.
> So {{contacted}} ends up with say 2 entries (the stale replicas), but {{stillAppliesTo}} checks against the full {{writeQuorum}} of 5. That's never going to work.
> {{contacted}} can also be completely empty due to the {{adjustedBlockFor}} logic in {{BlockingPartitionRepair}}'s constructor. The latch starts at {{writeQuorum}} and decrements for each contact that does NOT need a repair mutation — the reasoning being that those contacts already have the correct data and count toward quorum without needing a write. When the number of already-correct contacts meets or exceeds {{writeQuorum}}, {{adjustedBlockFor}} hits zero, the latch is pre-satisfied, and {{awaitRepairsUntil}} returns immediately. The repair mutations are still sent fire-and-forget, and a subsequent read at the same consistency level will intersect with the already-correct replicas. This is semantically fine — there are already enough correct replicas to satisfy quorum. But {{stillAppliesTo}} doesn't know any of that. It just sees an empty {{contacted}} list and throws.
> The issue is that {{stillAppliesTo}} is being applied to a plan where {{contacted}} was never intended to reflect the full write quorum. The repair was complete (or unnecessary to block on), but {{stillAppliesTo}} doesn't understand that context and kills the read with a spurious exception.
> h3. Evidence
> Collected 630 ISEs during concurrent host replacements on a 3-DC NTS cluster (RF=3/DC, EACH_QUORUM):
> ||Type||Count||
> |EACH_QUORUM stillAppliesTo (issue 1)|574|
> |Repair "complete already" (unrelated)|27|
> |Unknown stream session (expected)|26|
> |QUORUM stillAppliesTo from BRR (issue 2)|3|
> All 577 "ring has changed" ISEs looked the same — single IP swap (old node out, replacement in) at a {{FinishReplace}} epoch boundary. No pending replicas. Epoch bumped by 1. Every single EACH_QUORUM one had {{Received responses from: []}}.
> EACH_QUORUM example (IPs obfuscated):
> {noformat}
> Received responses from: []
> Old candidates: [Full(/0.0.0.98), Full(/0.0.0.82), Full(/0.0.0.14),
>                 Full(/0.0.0.12), Full(/0.0.0.46), Full(/0.0.0.18),
>                 Full(/0.0.0.102), Full(/0.0.0.70), Full(/0.0.0.201)]
> New candidates: [Full(/0.0.0.98), Full(/0.0.0.82), Full(/0.0.0.14),
>                 Full(/0.0.0.12), Full(/0.0.0.242), Full(/0.0.0.18),
>                 Full(/0.0.0.102), Full(/0.0.0.70), Full(/0.0.0.201)]
> Remaining required: 6
> {noformat}                                                                                                                                                            
> One IP swapped, contacted is empty, remaining is the full EACH_QUORUM quorum (2/DC * 3 DCs = 6). Never decremented.
> BRR QUORUM example — the non-empty contacted case:
> {noformat}
> Received responses from: [/0.0.0.108, /0.0.0.28]
> Old candidates: [9 replicas including /0.0.0.133]
> New candidates: [9 replicas, /0.0.0.133 replaced by /0.0.0.64]
> Remaining required: 3
> Stack: ReplicaPlan$ForWrite.stillAppliesTo
>       BlockingReadRepair.awaitWrites
>       StorageProxy$6.close
>       SelectStatement.execute
> {noformat}
> 2 repair targets responded, but full QUORUM needs 5. 5 - 2 = 3 remaining. The repair was semantically complete but {{stillAppliesTo}} doesn't account for replicas that already had correct data.
> BRR QUORUM example — the empty contacted case:
> {noformat}
> Received responses from: []
> Remaining required: 5
> {noformat}
> Latch was pre-satisfied because enough contacts already had correct data to meet quorum. Repair mutations sent fire-and-forget. Monotonicity is preserved — but {{stillAppliesTo}} ran before any callbacks and threw anyway.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

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