Skip to content

fix the bug the mimalloc heap switch not protected#267

Merged
liunyl merged 1 commit into
mainfrom
fix_heap
Dec 5, 2025
Merged

fix the bug the mimalloc heap switch not protected#267
liunyl merged 1 commit into
mainfrom
fix_heap

Conversation

@liunyl

@liunyl liunyl commented Dec 5, 2025

Copy link
Copy Markdown
Contributor

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

  • Refactor
    • Updated internal synchronization mechanisms for improved performance and resource efficiency.

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

@coderabbitai

coderabbitai Bot commented Dec 5, 2025

Copy link
Copy Markdown

Caution

Review failed

The pull request is closed.

Walkthrough

The PR replaces the synchronization primitive for hash_partition_ckpt_heap_mux_ from std::shared_mutex to std::mutex in the LocalCcShards class. Lock usage in the implementation file is updated from shared_lock to unique_lock, and an explicit unlock call is removed.

Changes

Cohort / File(s) Summary
Synchronization primitive swap
tx_service/include/cc/local_cc_shards.h
Changed member variable type from std::shared_mutex to std::mutex for hash_partition_ckpt_heap_mux_
Lock usage updates
tx_service/src/cc/local_cc_shards.cpp
Updated DataSyncForHashPartition and ReportHashPartitionCkptHeapUsage to use unique_lock<std::mutex> instead of shared_lock<std::shared_mutex>; removed explicit unlock call

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Verify that concurrent reader access is no longer required for hash partition checkpoint heap operations
  • Confirm all lock acquisition sites have been updated consistently with the new mutex type
  • Ensure the removal of the explicit unlock does not introduce any resource leaks or deadlock scenarios

Possibly related PRs

Poem

🐰 Locks once shared, now exclusive reign,
No more readers in this domain—
Mutex guards the checkpoint heap,
One at a time, the promises we keep!

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

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ddc3724 and b985484.

📒 Files selected for processing (2)
  • tx_service/include/cc/local_cc_shards.h (1 hunks)
  • tx_service/src/cc/local_cc_shards.cpp (2 hunks)

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.

@liunyl liunyl merged commit 17837f6 into main Dec 5, 2025
3 checks passed
@liunyl liunyl deleted the fix_heap branch December 5, 2025 06:34
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.

1 participant