Skip to content

chore: update tx_service for CC message cleanup - #564

Merged
liunyl merged 2 commits into
mainfrom
codex/issue-562-update-tx-service
Sep 2, 2026
Merged

chore: update tx_service for CC message cleanup#564
liunyl merged 2 commits into
mainfrom
codex/issue-562-update-tx-service

Conversation

@liunyl

@liunyl liunyl commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Context

Update the data_substrate submodule to include tx_service#563, which fixes protobuf payload memory retained by the shared CcMessage pool.

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_substrate from bf1dad9 to e643c3e.

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

  • Unit/TCL tests
  • Integration or manual validation
  • Formatting/build checks
  • Compatibility or performance validation, when relevant

Commands and results:

cmake --build /tmp/codex-tx-service-562-build --parallel 16
PASS

LD_LIBRARY_PATH=/home/ubuntu/workspace/eloqkv/data_substrate/third_party/install/lib:${LD_LIBRARY_PATH} ctest --test-dir /tmp/codex-tx-service-562-build --output-on-failure --parallel 4
PASS: 64/64 tests, including two-node cross-NG coverage

clang-format-18 --dry-run --Werror <changed tx_service C++ files>
PASS

git diff --check
PASS

Ad hoc parse/recycle microbenchmark:
standby-4KiB: -0.66%
read-response-4KiB: +2.26%

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 the data_substrate gitlink 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

  • Chores
    • Updated the bundled data component to a newer revision.
    • No user-facing functionality or behavior changes.

@coderabbitai

coderabbitai Bot commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 78ffa17d-4d5f-4920-9a80-29f51bef0157

📥 Commits

Reviewing files that changed from the base of the PR and between f823f37 and 51df862.

📒 Files selected for processing (1)
  • data_substrate
🚧 Files skipped from review as they are similar to previous changes (1)
  • data_substrate

Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review.


Walkthrough

The data_substrate gitlink now points to commit ea8b3c464e4b8fffda5fd2854dec803a8f6296ba instead of bf1dad98663608d42b6abe648a524bead83c09d0. No source code changed.

Changes

Subproject revision

Layer / File(s) Summary
Update pinned revision
data_substrate
The gitlink points to the new subproject commit.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Merge Risk: ⚪ Minimal · up to 51df8

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

A rabbit checks the revision line
One commit hops into place
The source remains unchanged
The pin now marks its target
The repository rests in sync

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the tx_service cleanup included by the data_substrate submodule update.
Description check ✅ Passed 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 wo…
Docstring Coverage ✅ Passed 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…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

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 Coverage

Explanation

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)
  • Create PR with unit tests
  • Commit unit tests in branch codex/issue-562-update-tx-service

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@liunyl
liunyl merged commit 74bf2d3 into main Sep 2, 2026
18 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.

2 participants