Skip to content

branch-4.1: [fix](cloud) Use bthread-aware shared mutex for tablet header lock - #66020

Merged
yiguolei merged 1 commit into
apache:branch-4.1from
liaoxin01:codex/pick-64574-to-branch-4.1
Jul 30, 2026
Merged

branch-4.1: [fix](cloud) Use bthread-aware shared mutex for tablet header lock#66020
yiguolei merged 1 commit into
apache:branch-4.1from
liaoxin01:codex/pick-64574-to-branch-4.1

Conversation

@liaoxin01

Copy link
Copy Markdown
Contributor

Pick #64574

### What problem does this PR solve?
Issue Number: None
Related PR: apache#64574
Problem Summary: The tablet header lock used a thread-affine shared mutex. In cloud mode, a bthread could suspend while holding the write lock and resume on another worker pthread, leaving the lock permanently wedged. Replace the tablet header lock with BthreadSharedMutex, which uses bthread-aware synchronization and supports cross-worker resume safely. Keep unrelated shared mutexes unchanged.
### Release note
Fix a potential permanent tablet header lock wedge after bthread migration in cloud mode.
### Check List (For Author)
- Test: Unit Test
- ./run-be-ut.sh --run --filter=BthreadSharedMutexTest.* -j 32 (7 tests passed)
- Behavior changed: Yes. The tablet header lock is now bthread-aware.
- Does this need documentation: No
CopilotAI review requested due to automatic review settings July 24, 2026 13:17
@liaoxin01
liaoxin01 requested a review from yiguolei as a code ownerJuly 24, 2026 13:17
@hello-stephen

Copy link
Copy Markdown
Contributor

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

  1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
  2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
  3. What features were added. Why was this function added?
  4. Which code was refactored and why was this part of the code refactored?
  5. Which functions were optimized and what is the difference before and after the optimization?

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Ports the tablet header lock from std::shared_mutex to a bthread-aware BthreadSharedMutex to avoid cross-thread unlock UB / wedges and writer-preferring recursive shared-lock deadlocks, and updates call sites accordingly.

Changes:

  • Added BthreadSharedMutex (bthread mutex/CV-based shared mutex) plus a dedicated unit test suite.
  • Replaced tablet/header lock types and updated lock guards (std::lock_guard / std::unique_lock / std::shared_lock) to use the new mutex.
  • Introduced “*_unlocked” helper variants to avoid recursive shared acquisition under a writer-preferring lock.

Reviewed changes

Copilot reviewed 24 out of 24 changed files in this pull request and generated 2 comments.

Show a summary per file
FileDescription
be/src/util/bthread_shared_mutex.hAdds the bthread-compatible shared mutex implementation.
be/test/util/bthread_shared_mutex_test.cppAdds behavioral + regression tests for the new mutex (writer pending, migration, cross-thread unlock).
be/src/storage/tablet/base_tablet.hSwitches _meta_lock to BthreadSharedMutex and adds an unlocked compaction score accessor.
be/src/storage/tablet/base_tablet.cppImplements get_real_compaction_score_unlocked() and routes the locked version through it.
be/src/storage/tablet/tablet.hUpdates header lock accessor type and adds unlocked compaction rowset-picking declarations.
be/src/storage/tablet/tablet.cppReplaces lock types and avoids recursive shared locking via unlocked helpers.
be/src/storage/task/index_builder.cppUpdates header lock guards to use the new lock type (CTAD).
be/src/storage/task/engine_clone_task.cppUpdates header lock guard to use the new lock type (CTAD).
be/src/storage/tablet/tablet_manager.cppUpdates header lock guard to use the new lock type (CTAD).
be/src/storage/schema_change/schema_change.cppUpdates header lock guards to use the new lock type (CTAD).
be/src/storage/rowset_builder.cppUpdates header lock guard to use the new lock type (CTAD).
be/src/storage/compaction/compaction.cppUpdates header lock guards to use the new lock type (CTAD).
be/src/storage/compaction/full_compaction.cppUpdates header lock guard to use the new lock type (CTAD).
be/src/storage/compaction/cumulative_compaction_time_series_policy.cppSwitches to unlocked rowset-picking to avoid recursive shared acquisition.
be/src/cloud/cloud_tablet.hUpdates lock-typed parameters and adds unlocked rowset-picking APIs.
be/src/cloud/cloud_tablet.cppRemoves inner shared locking in some paths and introduces unlocked rowset-picking implementations.
be/src/cloud/cloud_meta_mgr.hUpdates fill_version_holes to accept unique_lock<BthreadSharedMutex>&.
be/src/cloud/cloud_meta_mgr.cppUpdates header-lock guards and fill_version_holes signature/usage.
be/src/cloud/cloud_base_compaction.cppUses unlocked candidate selection under an existing shared header lock.
be/src/cloud/cloud_full_compaction.cppUses unlocked candidate selection under an existing shared header lock.
be/src/cloud/cloud_cumulative_compaction.cppSwitches traversal to an unlocked variant under an existing lock.
be/test/cloud/cloud_tablet_test.cppUpdates test lock guards to use the new lock type (CTAD).
be/test/cloud/cloud_meta_mgr_test.cppUpdates test lock guards to use the new lock type (CTAD).
be/test/cloud/cloud_empty_rowset_compaction_test.cppUpdates test lock guards to use the new lock type (CTAD).
be/src/cloud/cloud_tablet.hUpdates stats comment to reflect BthreadSharedMutex.
Comments suppressed due to low confidence (1)

be/src/cloud/cloud_tablet.cpp:1

  • Same issue as capture_versions_prefer_cache: the internal _meta_lock acquisition has been removed, turning this into an implicitly *_unlocked method without changing its name/signature. Recommendation (mandatory): provide an explicit locked entrypoint and a separate *_unlocked variant (or rename this one to *_unlocked) so callers can’t accidentally invoke it without holding _meta_lock.
// Licensed to the Apache Software Foundation (ASF) under one

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment threadbe/src/cloud/cloud_tablet.cpp
Comment threadbe/test/util/bthread_shared_mutex_test.cpp
@liaoxin01

Copy link
Copy Markdown
ContributorAuthor

run buildall

@hello-stephen

Copy link
Copy Markdown
Contributor

BE UT Coverage Report

Increment line coverage 83.90% (99/118) 🎉

Increment coverage report
Complete coverage report

CategoryCoverage
Function Coverage57.11% (23325/40841)
Line Coverage41.11% (231162/562353)
Region Coverage37.39% (183021/489483)
Branch Coverage38.47% (82226/213744)

@hello-stephen

Copy link
Copy Markdown
Contributor

BE Regression && UT Coverage Report

Increment line coverage 94.00% (94/100) 🎉

Increment coverage report
Complete coverage report

CategoryCoverage
Function Coverage73.08% (29143/39878)
Line Coverage56.82% (317864/559435)
Region Coverage53.93% (265577/492475)
Branch Coverage54.78% (117208/213975)

@yiguolei
yiguolei merged commit af27580 into apache:branch-4.1Jul 30, 2026
30 of 34 checks passed
@yiguoleiyiguolei mentioned this pull request Sep 1, 2026
Sign up for freeto 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.

4 participants

@liaoxin01@hello-stephen@yiguolei