[
https://issues.apache.org/jira/browse/CASSANDRA-21665?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18112529#comment-18112529
]
Johnny Miller commented on CASSANDRA-21665:
-------------------------------------------
While preparing the patch I checked what else the f48de8343c merge dropped. It
turns out the whole of CASSANDRA-18948 was lost, not just the volatile. That
commit also changed CommitLogSegmentManagerCDCTest, and none of it made it into
5.0 either.
Most of the test side has since come back through CASSANDRA-20091. Four small
pieces are still missing on 5.0 while 4.0, 4.1, 6.0 and trunk all have them.
The patch will restore these along with the volatile so it matches the other
branches again.
> Restore volatile on CommitLogSegment.cdcState, lost in the cassandra-4.1 to
> cassandra-5.0 merge
> -----------------------------------------------------------------------------------------------
>
> Key: CASSANDRA-21665
> URL: https://issues.apache.org/jira/browse/CASSANDRA-21665
> Project: Apache Cassandra
> Issue Type: Bug
> Components: Local/Commit Log
> Reporter: Johnny Miller
> Assignee: Johnny Miller
> Priority: Normal
> Fix For: 5.0.x
>
>
> While doing some research into the commit log code I came across something
> that looks like it was dropped by mistake during a merge.
> CASSANDRA-18948 fixed flakiness in CommitLogSegmentManagerCDCTest by making
> CommitLogSegment.cdcState volatile. That fix is present on cassandra-4.0 and
> cassandra-4.1, and the 4.0 to 4.1 merge (6cac24f581) carried it correctly.
> The next merge up, f48de8343c "Merge branch 'cassandra-4.1' into
> cassandra-5.0" on 2023-11-20, has the volatile on its 4.1 parent but resolved
> CommitLogSegment.java without it.
> As a result every 5.0.x release ships "private CDCState cdcState =
> CDCState.PERMITTED;" (CommitLogSegment.java, line 70 at current 5.0 HEAD)
> while 4.0, 4.1, 6.0 and trunk all have the volatile.
> This matters because getCDCState() is read without holding cdcStateLock on
> the CDC allocation hot path (CommitLogSegmentManagerCDC.permitSegmentMaybe
> and throwIfForbidden, lines 197 and 214 at 5.0 HEAD, and createSegment at
> line 243) and in CommitLogSegment.sync (line 363), so 5.0 has the
> cross-thread visibility gap CASSANDRA-18948 fixed, along with the CDC test
> flakiness it was addressing.
> I think this would result in threads sometimes seeing an out of date CDC
> state for a segment. So a CDC write could be rejected when there is actually
> space, or let through when there is not, and the _cdc.idx file could be
> written one sync later than it should. Nothing gets lost, it just makes the
> wrong call for a moment, and it brings back the flaky CDC tests that
> CASSANDRA-18948 was fixing. Happy to be told it is fine without it, but every
> other branch has it, so it looks like a merge accident.
> The fix is the original one line, applied to cassandra-5.0 only;
> cassandra-6.0 and trunk already carry the volatile.
> Patch to follow.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
