Fix assert bug

[yangsw26]

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

• Bug Fixes
  • Fixed incorrect post-checkpoint size calculation for certain slice configurations.
  • Improved system stability by properly handling edge cases where slices don't require splitting.
  • Resolved crash condition that occurred in specific scenarios.

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

[coderabbitai]

Walkthrough

The pull request modifies slice update logic in a transaction service to handle edge cases when a slice has exactly one pending split key or when split keys are empty. Post-checkpoint sizes are set appropriately, and a crash-inducing assertion is removed with clarifying comments added.

Changes

Cohort / File(s)	Summary
Slice Split Key Handling tx_service/src/cc/local_cc_shards.cpp	Fixed post-checkpoint size assignment for slices with a single split key during paused-slice updates and in-memory updates. Removed assertion that crashed when slice_split_keys is empty. Added comments explaining edge cases where split keys may be absent.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

• Review the updated size-setting logic for single split-key scenarios to ensure correctness
• Verify the removed assertion doesn't cause issues in other code paths that depended on it
• Confirm comments accurately describe the edge case scenarios

Suggested reviewers

• xiexiaoy
• liunyl

Poem

🐰 A single slice, once thought to split,
Now settles small with careful bit,
When keys are bare, we do not break,
Fixed assertions for safety's sake,
Comments bloom where edge cases hide!

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings, 1 inconclusive)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The PR description contains only the template checklist with no actual implementation details, context, or explanation of the changes made.	Complete the description with details on what the assert bug was, why it occurred, how the fix addresses it, and check off completed items from the template.
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.
Title check	❓ Inconclusive	The title 'Fix assert bug' is vague and generic, lacking specific context about what assert was fixed or which component was affected.	Provide a more descriptive title that specifies the component or feature affected, such as 'Fix assert in UpdateSlices for empty slice_split_keys' or similar.

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

---

📜 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 6072390 and 34d6302.

📒 Files selected for processing (1)

• tx_service/src/cc/local_cc_shards.cpp (3 hunks)

🧰 Additional context used

🧠 Learnings (3)

📚 Learning: 2025-11-11T07:10:40.346Z

Learnt from: lzxddz
Repo: eloqdata/tx_service PR: 199
File: include/cc/local_cc_shards.h:233-234
Timestamp: 2025-11-11T07:10:40.346Z
Learning: In the LocalCcShards class in include/cc/local_cc_shards.h, the EnqueueCcRequest methods use `shard_code & 0x3FF` followed by `% cc_shards_.size()` to distribute work across processor cores for load balancing. This is intentional and separate from partition ID calculation. The 0x3FF mask creates a consistent distribution range (0-1023) before modulo by actual core count.

Applied to files:

• tx_service/src/cc/local_cc_shards.cpp

📚 Learning: 2025-10-09T03:56:58.811Z

Learnt from: thweetkomputer
Repo: eloqdata/tx_service PR: 150
File: include/cc/local_cc_shards.h:626-631
Timestamp: 2025-10-09T03:56:58.811Z
Learning: For the LocalCcShards class in include/cc/local_cc_shards.h: Writer locks (unique_lock) should continue using the original meta_data_mux_ (std::shared_mutex) rather than fast_meta_data_mux_ (FastMetaDataMutex) at this stage. Only reader locks may use the FastMetaDataMutex wrapper.

Applied to files:

• tx_service/src/cc/local_cc_shards.cpp

📚 Learning: 2025-12-02T10:43:27.431Z

Learnt from: lokax
Repo: eloqdata/tx_service PR: 254
File: tx_service/src/cc/local_cc_shards.cpp:2949-3188
Timestamp: 2025-12-02T10:43:27.431Z
Learning: In tx_service/src/cc/local_cc_shards.cpp, whenever TryPinNodeGroupData is used, only call Sharder::Instance().UnpinNodeGroupData(node_group) if the recorded term is >= 0 (i.e., pin succeeded). Example: LocalCcShards::PostProcessFlushTaskEntries guards the unpin with `if (term >= 0)`.

Applied to files:

• tx_service/src/cc/local_cc_shards.cpp

🔇 Additional comments (3)

tx_service/src/cc/local_cc_shards.cpp (3)

5242-5247: Single paused-slice split-key handling looks correct

Using the single SliceChangeInfo’s post_update_size_ to fix paused_slice_->SetPostCkptSize when paused_split_keys_.size() == 1 is consistent with how the subslice sizes are computed and avoids an unnecessary in-memory split while still correcting the post‑ckpt size. No functional issues spotted.

---

5398-5405: Consistent single-split-key handling for current slice

Mirroring the paused-slice logic here—treating slice_split_keys.size() == 1 as “no split, just update curr_slice->PostCkptSize using post_update_size_”—is reasonable and keeps slice metadata consistent when a split turns out to be unnecessary after key-level inspection.

---

5416-5419: Empty slice_split_keys case is documented; double‑check all callers tolerate it

The comment clarifies that slice_split_keys (and by extension UpdateSliceStatus::paused_split_keys_) can legitimately be empty when leading keys are deleted. Given that some helpers (e.g., range cache sending paths) iterate or binary‑search over paused_split_keys_, please ensure none of them still assume the vector is non‑empty (e.g., via unchecked front()/back() or asserts on emptiness).

---

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.}