Skip to content

CASSANALYTICS-150 Change commit log mode for CDC tests#197

Merged
jmckenzie-dev merged 2 commits into
apache:trunkfrom
skoppu22:clmode
Apr 21, 2026
Merged

CASSANALYTICS-150 Change commit log mode for CDC tests#197
jmckenzie-dev merged 2 commits into
apache:trunkfrom
skoppu22:clmode

Conversation

@skoppu22

@skoppu22 skoppu22 commented Apr 16, 2026

Copy link
Copy Markdown
Contributor

CDC tests setup in Cassandra analytics is using commit log disk access mode DIRECT. This is causing CI builds running on containers to fail with OOM, as unable to acquire sufficient DIRECT memory on memory constrained containers. Changing commit log mode to mmap works, and this aligns with Cassandra default commit log mode logic (see resolveCommitLogWriteDiskAccessMode in Cassandra)

@skoppu22 skoppu22 marked this pull request as draft April 16, 2026 12:21
@skoppu22 skoppu22 marked this pull request as ready for review April 16, 2026 14:50
@skoppu22

Copy link
Copy Markdown
Contributor Author

@arjunashok arjunashok left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The git-commit message seems to suggest this is only for tests. I’m wondering if it is only for tests, can we not switch/override it in the test implementation?

Do we intend to switch the default commit log mode?

Also, seems like Cassandra’s default is DiskAccessMode.legacy.
https://github.com/apache/cassandra/blob/addec0ef7ab4c9c023ae8f60c6af565c520368ff/src/java/org/apache/cassandra/config/Config.java#L430

@yifan-c

yifan-c commented Apr 16, 2026

Copy link
Copy Markdown
Contributor

+1. waiting for CI results

@yifan-c

yifan-c commented Apr 16, 2026

Copy link
Copy Markdown
Contributor

The git-commit message seems to suggest this is only for tests. I’m wondering if it is only for tests, can we not switch/override it in the test implementation?

Do we intend to switch the default commit log mode?

Also, seems like Cassandra’s default is DiskAccessMode.legacy. https://github.com/apache/cassandra/blob/addec0ef7ab4c9c023ae8f60c6af565c520368ff/src/java/org/apache/cassandra/config/Config.java#L430

The code is for testing only. It is to generate the commit logs to test CDC function.

@frankgh frankgh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 looks good to me

@skoppu22

Copy link
Copy Markdown
Contributor Author

The git-commit message seems to suggest this is only for tests. I’m wondering if it is only for tests, can we not switch/override it in the test implementation?

Do we intend to switch the default commit log mode?

Also, seems like Cassandra’s default is DiskAccessMode.legacy. https://github.com/apache/cassandra/blob/addec0ef7ab4c9c023ae8f60c6af565c520368ff/src/java/org/apache/cassandra/config/Config.java#L430

The modified code gets invoked only in test path using reflection. Cassandra determines commit log mode as below:

 Resolution logic (resolveCommitLogWriteDiskAccessMode in DatabaseDescriptor.class)

 ┌─────────────────┬─────────────────────────┬─────────────────────────────────────┬───────────────────┐
 │ Configured mode │ Compression/Encryption? │             Linux SSD?              │  Effective mode   │
 ├─────────────────┼─────────────────────────┼─────────────────────────────────────┼───────────────────┤
 │ legacy          │ No                      │ (irrelevant)                        │ mmap              │
 ├─────────────────┼─────────────────────────┼─────────────────────────────────────┼───────────────────┤
 │ legacy          │ Yes                     │ (irrelevant)                        │ standard          │
 ├─────────────────┼─────────────────────────┼─────────────────────────────────────┼───────────────────┤
 │ auto            │ No                      │ Yes (block size > 0 + ssd strategy) │ direct            │
 ├─────────────────┼─────────────────────────┼─────────────────────────────────────┼───────────────────┤
 │ auto            │ No                      │ No                                  │ legacy → mmap     │
 ├─────────────────┼─────────────────────────┼─────────────────────────────────────┼───────────────────┤
 │ auto            │ Yes                     │ (irrelevant)                        │ legacy → standard │
 └─────────────────┴─────────────────────────┴─────────────────────────────────────┴───────────────────┘

@skoppu22

Copy link
Copy Markdown
Contributor Author

@yifan-c this PR already got 2 approvals. can you merge it please?

@jmckenzie-dev jmckenzie-dev merged commit fdd221e into apache:trunk Apr 21, 2026
37 of 38 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants