[
https://issues.apache.org/jira/browse/CASSANDRA-21665?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Johnny Miller updated CASSANDRA-21665:
--------------------------------------
Component/s: Local/Commit Log
> 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, so the merge upwards is a
> no-op.
> 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]
