Skip to content

fix ReadLocalOp assert#334

Merged
MrGuin merged 1 commit into
mainfrom
fix_read_local
Jan 4, 2026
Merged

fix ReadLocalOp assert#334
MrGuin merged 1 commit into
mainfrom
fix_read_local

Conversation

@MrGuin

@MrGuin MrGuin commented Jan 4, 2026

Copy link
Copy Markdown
Collaborator

Here are some reminders before you submit the pull request

  • Add tests for the change
  • Document changes
  • Reference the link of issue using fixes eloqdb/tx_service#issue_id
  • Reference the link of RFC if exists
  • Pass ./mtr --suite=mono_main,mono_multi,mono_basic

Summary by CodeRabbit

Release Notes

  • Bug Fixes
    • Enhanced transaction operation reliability by improving error handling for term change scenarios. The system now treats certain failure conditions with consistent logic, enabling more graceful recovery and appropriate retry decisions when transactions encounter specific error states.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai

coderabbitai Bot commented Jan 4, 2026

Copy link
Copy Markdown

Caution

Review failed

The pull request is closed.

Walkthrough

This pull request modifies the error-handling logic in ReadLocalOperation::Forward within the transaction service. It expands the set of acceptable finished-error codes by adding CcErrorCode::NG_TERM_CHANGED to the existing condition that handles CcErrorCode::LEADER_NODE_UNREACHABLE, ensuring both error codes trigger the same retry or abort flow.

Changes

Cohort / File(s) Summary
Error Handling Expansion
tx_service/src/tx_operation.cpp
Added CcErrorCode::NG_TERM_CHANGED to the finished-error condition in ReadLocalOperation::Forward, allowing it to be treated identically to LEADER_NODE_UNREACHABLE in the error-handling branch for retry/abort logic.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

  • fix retry oom error in read op #285: Modifies error-handling logic in ReadOperation::Forward within the same file, affecting the same retry/finish flow.
  • fix ReadLocalOp assert #331: Also expands error-handling paths in ReadLocalOperation::Forward (tx_operation.cpp) by adding additional finished-error codes to trigger retry/abort logic.

Suggested reviewers

  • liunyl
  • yi-xmu

Poem

🐰 A term has changed, and errors flow,
No more alone does the leader go—
One code, then two, in harmony they dance,
Retry or abort, they waltz in cadence!

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix_read_local

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6cea0a9 and 28e9e29.

📒 Files selected for processing (1)
  • tx_service/src/tx_operation.cpp

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 and usage tips.

@MrGuin MrGuin merged commit 3aa4cf5 into main Jan 4, 2026
3 of 4 checks passed
@MrGuin MrGuin deleted the fix_read_local branch January 4, 2026 10:48
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