fix EscalateStandbyCcmCc: do not set commit_ts when cce is in Unknown status

[MrGuin]

Summary by CodeRabbit

• Refactor
  • Internal improvements to buffer and timestamp handling logic to enhance code robustness and consistency.

---

Note: This release contains no user-visible changes.

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

[coderabbitai]

Walkthrough

Reworked buffer-clearing logic in EscalateStandbyCcmCc::Execute to replace explicit clearing with status-based invariants. The logic now enforces that Normal/Deleted statuses imply no buffered commands, Unknown status permits buffered commands, and assertions verify these invariants rather than performing direct clears.

Changes

Cohort / File(s)	Summary
Buffer-clearing and invariant enforcement tx_service/include/cc/template_cc_map.h	Reworked buffer-clearing logic in EscalateStandbyCcmCc::Execute; replaced explicit BufferedCommandList clearing with status-based invariants; added assertions to enforce relationships between record status (Normal/Deleted/Unknown) and buffered command presence; commit-ts payload status now set only for Normal or Deleted statuses.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Areas requiring extra attention:
  • Verify correctness of the status-to-buffered-commands invariants (Normal/Deleted implies empty, Unknown permits non-empty)
  • Ensure assertions are in the right locations and cover all paths
  • Confirm no unintended behavioral changes from removing explicit clears, particularly for checkpoint handling paths

Suggested reviewers

• liunyl

Poem

🐰 A rabbit cleared the buffered way,
With invariants to guide and stay,
Where status doth the truth now tell,
No more explicit clears to quell—
Just assertions ring their bell!

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	No pull request description was provided. The template requires tests, documentation, issue references, RFC links, and test suite confirmation.	Add a description including issue reference, explanation of the fix, testing information, and confirmation of passing test suites.
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main fix—preventing commit_ts from being set when the cache entry is in Unknown status, which aligns with the reworked buffer-clearing logic.

✨ 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_esc_cc

---

📜 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 afe9672 and 65794c5.

📒 Files selected for processing (1)

• tx_service/include/cc/template_cc_map.h (1 hunks)

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2025-09-25T06:15:41.186Z

Learnt from: liunyl
Repo: eloqdata/tx_service PR: 142
File: include/tx_req_result.h:140-159
Timestamp: 2025-09-25T06:15:41.186Z
Learning: When cc_notify_ is true in TxResult, both Finish() and Wait() are guaranteed to be called on the same thread, eliminating concurrency concerns and making relaxed memory ordering sufficient for status_combo_ updates.

Applied to files:

• tx_service/include/cc/template_cc_map.h

🔇 Additional comments (1)

tx_service/include/cc/template_cc_map.h (1)

7815-7825: EscalateStandbyCcmCc invariants look correct and match the intended fix

The updated logic does what the PR promises: it skips SetCommitTsPayloadStatus when PayloadStatus() is Unknown, while still forcing a fresh commit_ts for Normal/Deleted entries in txservice_skip_wal mode. The two asserts encode and document the important invariant that buffered commands only coexist with Unknown payload status and never with Normal/Deleted, which aligns with the downstream expectations in the data-sync paths. No functional or safety issues spotted here.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}