chore: update tx_service for CC message cleanup - #564
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review. WalkthroughThe ChangesSubproject revision
Estimated code review effort: 1 (Trivial) | ~2 minutes Merge Risk: ⚪ Minimal · up to This PR only updates the data_substrate dependency to release protobuf payloads when CC messages return to the shared pool; no actionable merge-blocking risk remains after normal checks and review. Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Description checkExplanation The description includes all required template sections, explains the behavior change and implementation, documents tests and results, and identifies risks, rollback, review guidance, and follow-up work. Full details: Docstring CoverageExplanation No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (1 skipped: 1 unsupported.) ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Context
Update the
data_substratesubmodule to include tx_service#563, which fixes protobuf payload memory retained by the sharedCcMessagepool.Behavior before and after
Before: recycled CC protobufs could retain large standby-forward and other message payload allocations while idle in the pool.
After: the updated tx_service clears every CC protobuf before returning it to the shared pool, retaining only the reusable message shell.
Implementation
Advance
data_substratefrombf1dad9toe643c3e.The submodule change introduces a centralized
CcMessagePool, routes all acquire/recycle paths through it, adds focused unit coverage, and updates the tx_service architecture documentation.Design decisions and alternatives
This parent PR contains only the submodule pointer update. Memory budgeting, transport backpressure, subscription fencing, and pool-size limits remain outside this change.
Test plan
Commands and results:
A full EloqKV build and production-duration RSS/throughput A/B test were not run locally. This PR is intended to run the EloqKV CI matrix.
Risk assessment
The change moves protobuf payload destruction from the next parse to recycle time. The microbenchmark stayed within the 5% regression threshold, and the two-node transaction test passed.
The parent repository behavior otherwise changes only through the referenced submodule commit.
Rollback plan
Revert this submodule pointer update and the corresponding tx_service PR.
Reviewer guide
Review tx_service PR #563 first, especially
CcMessagePool::Recycle()and the migration away from direct queue operations. This PR itself should contain only thedata_substrategitlink update.Follow-up work
Evaluate memory budgets and transport-level backpressure separately if retained shell count or in-flight concurrency remains a problem.
Summary by CodeRabbit