Repository navigation
Conversation
|
No actionable comments were generated in the recent review. ℹ️ Recent review info
Walkthrough
Priority: Change: Bug fix Merge Risk:
 |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 2 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title clearly identifies the affected connection behavior and the fix for partial protocol v5 segment headers. Description check The description explains the cause, fix, affected scenarios, regression test, and validation results. It does not include the template checklist or a Fixes annotation, but the required change and test… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Description check
-
Explanation
The description explains the cause, fix, affected scenarios, regression test, and validation results. It does not include the template checklist or a Fixes annotation, but the required change and testing information are substantially complete.
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
Code Review by Qodo🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)
Great, no issues found!Qodo reviewed your code and found no material issues that require reviewTip of the day💡 Did you know, you can enable the Remediation agent and Qodo fixes findings in a dedicated fix PR Powered by Qodo |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1141 +/- ##
==========================================
+ Coverage 86.63% 86.76% +0.13%
==========================================
Files 44 44
Lines 18987 19006 +19
Branches 2963 2965 +2
==========================================
+ Hits 16449 16491 +42
+ Misses 1864 1844 -20
+ Partials 674 671 -3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
The same bug is in the Apache driver. I confirmed it with the same unit test on trunk (3.30.1) and sent the fix there: apache#1308. (drafted with Claude Code) |
With protocol v5 framing, _process_segment_buffer() returned without rewinding the io buffer when fewer than 6 bytes (one segment header plus its CRC24) had arrived. process_io_buffer() then called reset_io_buffer(), which keeps only the bytes after the current position, and the position was at the end, so those 1-5 header bytes were dropped. The next read was parsed from the wrong offset and the connection was defuncted with "CRC mismatch on header". It needs a socket read to end inside a segment header, so it shows up rarely and mostly with the asyncio reactor, which processes the buffer after every 4 KB read. It was seen in CI against Cassandra 5.0 during a large schema refresh. Rewind in that branch too, like the "header but incomplete payload" branch already does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
90f8d45 to
201b324
Compare
With protocol v5, a read that ends 1-5 bytes into a segment header loses those bytes, and the connection is then defuncted with
CRC mismatch on header.Cause
Connection._process_segment_buffer()has three cases:seek(0)and wait for more.process_io_buffer()then callsreset_io_buffer(), which keeps only the bytes after the current position. In case 3 the position is at the end, so the partial header is discarded. The next read is parsed from the wrong offset and its header fails the CRC24 check.The fix adds the missing
seek(0)to case 3.How it was found
The new Cassandra CI job in #1135 failed once on Cassandra 5.0 + asyncio, in
test_pool_management, during a large schema refresh:It needs a socket read to end inside a header, so it's rare. It's more likely with the asyncio reactor, which processes the buffer after every 4 KB read, than with libev, which reads until the socket is empty. It doesn't affect Scylla, which negotiates down to v4, but it affects default-configured clients on Cassandra 4.0+.
The same code is in apache/cassandra-python-driver.
Testing
test_v5_segment_header_split_across_readsfeeds two segments split at every offset from 0 to 7 bytes into the second header. On master, offsets 1-5 fail. With the fix, all pass.test_pool_managementon v5 failed 10/10 runs without the fix and passed 10/10 with it.🤖 Generated with Claude Code