Skip to content

[fix](compaction) Make creating tablet idempotently to keep compaction Job idempotent - #56061

Merged
dataroaring merged 1 commit into
apache:masterfrom
wyxxxcat:compaction_idempotent
Sep 16, 2025
Merged

[fix](compaction) Make creating tablet idempotently to keep compaction Job idempotent#56061
dataroaring merged 1 commit into
apache:masterfrom
wyxxxcat:compaction_idempotent

Conversation

@wyxxxcat

@wyxxxcatwyxxxcat commented Sep 15, 2025

Copy link
Copy Markdown
Collaborator

What problem does this PR solve?

If rpc create_tablets is not idempotent, it will recreate tablet stats, which is tragic for compaction task idempotence

if (compaction.base_compaction_cnt() < stats.base_compaction_cnt() ||
compaction.cumulative_compaction_cnt() < stats.cumulative_compaction_cnt()) {
code = MetaServiceCode::STALE_TABLET_CACHE;
SS << "could not perform compaction on expired tablet cache."
<< " req_base_compaction_cnt=" << compaction.base_compaction_cnt()
<< ", base_compaction_cnt=" << stats.base_compaction_cnt()
<< ", req_cumulative_compaction_cnt=" << compaction.cumulative_compaction_cnt()
<< ", cumulative_compaction_cnt=" << stats.cumulative_compaction_cnt();
msg = ss.str();
return;
}

Release note

None

Check List (For Author)

  • Test

    • Regression test
    • Unit Test
    • Manual test (add detailed scripts or steps below)
    • No need to test or manual test. Explain why:
      • This is a refactor/code format and no logic has been changed.
      • Previous test can cover this change.
      • No code files have been changed.
      • Other reason
  • Behavior changed:

    • No.
    • Yes.
  • Does this need documentation?

    • No.
    • Yes.

Check List (For Reviewer who merge this PR)

  • Confirm the release note
  • Confirm test cases
  • Confirm document
  • Add branch pick label

@Thearas

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?

@wyxxxcat

Copy link
Copy Markdown
CollaboratorAuthor

run buildall

Comment threadcloud/src/meta-service/meta_service.cpp Outdated
Comment threadcloud/src/meta-service/meta_service.cpp Outdated
Comment threadcloud/src/meta-service/meta_service.cpp
@wyxxxcat

Copy link
Copy Markdown
CollaboratorAuthor

run buildall

@hello-stephen

Copy link
Copy Markdown
Contributor

Cloud UT Coverage Report

Increment line coverage 87.50% (28/32) 🎉

Increment coverage report
Complete coverage report

CategoryCoverage
Function Coverage84.58% (1536/1816)
Line Coverage68.30% (27657/40493)
Region Coverage69.03% (13626/19740)
Branch Coverage59.16% (7282/12310)

@wyxxxcat
wyxxxcatforce-pushed the compaction_idempotent branch 2 times, most recently from 7a8dd26 to c236e70CompareSeptember 15, 2025 12:32
@wyxxxcat

Copy link
Copy Markdown
CollaboratorAuthor

run buildall

@wyxxxcat

Copy link
Copy Markdown
CollaboratorAuthor

run buildall

@doris-robot

Copy link
Copy Markdown

Cloud UT Coverage Report

Increment line coverage 87.88% (29/33) 🎉

Increment coverage report
Complete coverage report

CategoryCoverage
Function Coverage84.01% (1534/1826)
Line Coverage68.08% (27542/40457)
Region Coverage68.56% (13564/19783)
Branch Coverage58.78% (7241/12318)

Comment threadcloud/src/meta-service/meta_service.cpp
Comment threadcloud/src/meta-service/meta_service.h
@wyxxxcat

Copy link
Copy Markdown
CollaboratorAuthor

run buildall

@doris-robot

Copy link
Copy Markdown

Cloud UT Coverage Report

Increment line coverage 88.24% (30/34) 🎉

Increment coverage report
Complete coverage report

CategoryCoverage
Function Coverage84.01% (1534/1826)
Line Coverage68.12% (27558/40458)
Region Coverage68.56% (13564/19783)
Branch Coverage58.87% (7252/12318)

@github-actionsgithub-actionsBot added the approved Indicates a PR has been approved by one committer. label Sep 16, 2025
@github-actions

Copy link
Copy Markdown
Contributor

PR approved by at least one committer and no changes requested.

@github-actions

Copy link
Copy Markdown
Contributor

PR approved by anyone and no changes requested.

@dataroaringdataroaring 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.

LGTM

@dataroaring
dataroaring merged commit bef8f36 into apache:masterSep 16, 2025
33 of 35 checks passed
wyxxxcat added a commit to wyxxxcat/doris that referenced this pull request Sep 16, 2025
…n Job idempotent (apache#56061)
If rpc create_tablets is not idempotent, it will recreate tablet stats,
which is tragic for compaction task idempotence
```
if (compaction.base_compaction_cnt() < stats.base_compaction_cnt() ||
compaction.cumulative_compaction_cnt() < stats.cumulative_compaction_cnt()) {
code = MetaServiceCode::STALE_TABLET_CACHE;
SS << "could not perform compaction on expired tablet cache."
<< " req_base_compaction_cnt=" << compaction.base_compaction_cnt()
<< ", base_compaction_cnt=" << stats.base_compaction_cnt()
<< ", req_cumulative_compaction_cnt=" << compaction.cumulative_compaction_cnt()
<< ", cumulative_compaction_cnt=" << stats.cumulative_compaction_cnt();
msg = ss.str();
return;
}
```
morrySnow pushed a commit that referenced this pull request Sep 17, 2025
@morrySnowmorrySnow mentioned this pull request Sep 22, 2025
wyxxxcat added a commit to wyxxxcat/doris that referenced this pull request Sep 23, 2025
…n Job idempotent (apache#56061)
If rpc create_tablets is not idempotent, it will recreate tablet stats,
which is tragic for compaction task idempotence
```
if (compaction.base_compaction_cnt() < stats.base_compaction_cnt() ||
compaction.cumulative_compaction_cnt() < stats.cumulative_compaction_cnt()) {
code = MetaServiceCode::STALE_TABLET_CACHE;
SS << "could not perform compaction on expired tablet cache."
<< " req_base_compaction_cnt=" << compaction.base_compaction_cnt()
<< ", base_compaction_cnt=" << stats.base_compaction_cnt()
<< ", req_cumulative_compaction_cnt=" << compaction.cumulative_compaction_cnt()
<< ", cumulative_compaction_cnt=" << stats.cumulative_compaction_cnt();
msg = ss.str();
return;
}
```
yiguolei pushed a commit to yiguolei/incubator-doris that referenced this pull request Sep 28, 2025
hello-stephen pushed a commit that referenced this pull request Aug 14, 2026
Related PR: #56061
Problem Summary: ShortGetTabletStatsDebugStringTest enables the global
idempotent request injection configuration but does not restore it after
completion. Because meta_service_http_test.cpp and meta_service_test.cpp
are linked into the same test binary, later tests may create delayed
background bthreads.
Those tasks capture MetaServiceProxy through a raw this pointer and can
access impl_ after the proxy is destroyed, causing intermittent
heap-use-after-free, heap-buffer-overflow, or SEGV failures. Remove the
test to prevent this test-binary-wide configuration pollution and the
resulting flaky failures.
github-actionsBot pushed a commit that referenced this pull request Aug 14, 2026
Related PR: #56061
Problem Summary: ShortGetTabletStatsDebugStringTest enables the global
idempotent request injection configuration but does not restore it after
completion. Because meta_service_http_test.cpp and meta_service_test.cpp
are linked into the same test binary, later tests may create delayed
background bthreads.
Those tasks capture MetaServiceProxy through a raw this pointer and can
access impl_ after the proxy is destroyed, causing intermittent
heap-use-after-free, heap-buffer-overflow, or SEGV failures. Remove the
test to prevent this test-binary-wide configuration pollution and the
resulting flaky failures.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approvedIndicates a PR has been approved by one committer.dev/3.0.xdev/3.0.x-conflictdev/3.1.1-mergedp0_breviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants

@wyxxxcat@Thearas@hello-stephen@doris-robot@gavinchou@dataroaring@yiguolei@morrySnow