Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Enhancement](compaction) Do not set failure time when cumulative compaction dealing with delete rowset #43466

Merged
merged 11 commits into from
Dec 13, 2024
Merged

[Enhancement](compaction) Do not set failure time when cumulative compaction dealing with delete rowset #43466

merged 11 commits into from
Dec 13, 2024
+148 −2

Conversation

Yukang-Lian
Copy link
Collaborator

@Yukang-Lian Yukang-Lian commented Nov 7, 2024 •
edited
Loading

What problem does this PR solve?

Problem Summary:

Before this PR, in cases where there is an alternating distribution of data rowset -> delete rowset -> data rowset -> delete rowset, cumulative compaction would only move the cumulative point forward to allow base compaction to handle the delete rowset. Cumulative compaction itself would not process the data and would return be marked as failure. This would cause the compaction submission task process to pause for 5 seconds, impacting efficiency.

This PR modifies the return status to OK for such cases, which improves the efficiency of the compaction submission task.

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

@doris-robot
Copy link

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?

@Yukang-Lian
Copy link
Collaborator Author

run buildall

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Nov 7, 2024

clang-tidy review says "All clean, LGTM! 👍"

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 37.90% (9864/26026)
Line Coverage: 29.07% (82064/282331)
Region Coverage: 28.28% (42257/149422)
Branch Coverage: 24.85% (21432/86240)
Coverage Report: http://coverage.selectdb-in.cc/coverage/b8742bb3c57c6fea9325807e086a313a66c73f75_b8742bb3c57c6fea9325807e086a313a66c73f75/report/index.html

gavinchou
gavinchou previously approved these changes Nov 10, 2024
View reviewed changes
@github-actions GitHub Actions
Copy link
Contributor

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

@github-actions github-actions bot added the approved Indicates a PR has been approved by one committer. label Nov 10, 2024
@github-actions GitHub Actions
Copy link
Contributor

PR approved by anyone and no changes requested.

@Yukang-Lian
Copy link
Collaborator Author

run cloud_p0

@Yukang-Lian Yukang-Lian force-pushed the Enhance-Delete-Compaction branch from b8742bb to b17d93c Compare November 12, 2024 08:25
@github-actions github-actions bot removed the approved Indicates a PR has been approved by one committer. label Nov 12, 2024
@github-actions GitHub Actions
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@Yukang-Lian
Copy link
Collaborator Author

run buildall

@Yukang-Lian
Copy link
Collaborator Author

run beut

@Yukang-Lian Yukang-Lian force-pushed the Enhance-Delete-Compaction branch from b17d93c to 899ea90 Compare November 27, 2024 03:00
@Yukang-Lian
Copy link
Collaborator Author

run buildall

@Yukang-Lian Yukang-Lian changed the title [Enhancement](compaction) set status to ok for alternating rowsets compaction [Enhancement](compaction) Do not set failure time when cumulative compaction dealing with delete rowset Nov 27, 2024
@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 38.34% (9975/26016)
Line Coverage: 29.43% (83466/283610)
Region Coverage: 28.58% (42969/150337)
Branch Coverage: 25.19% (21828/86658)
Coverage Report: http://coverage.selectdb-in.cc/coverage/899ea902c8c49e92a8488e7b524fdcc0febd1afe_899ea902c8c49e92a8488e7b524fdcc0febd1afe/report/index.html

@Yukang-Lian Yukang-Lian force-pushed the Enhance-Delete-Compaction branch 2 times, most recently from 96ab537 to 75d9dbc Compare December 4, 2024 15:43
@Yukang-Lian
Copy link
Collaborator Author

run buildall

1 similar comment
@Yukang-Lian
Copy link
Collaborator Author

run buildall

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 38.48% (10006/26003)
Line Coverage: 29.50% (83881/284316)
Region Coverage: 28.61% (43115/150700)
Branch Coverage: 25.20% (21915/86958)
Coverage Report: http://coverage.selectdb-in.cc/coverage/d350da140c018b64820d262f6abda8f96642e7fc_d350da140c018b64820d262f6abda8f96642e7fc/report/index.html

@Yukang-Lian
Copy link
Collaborator Author

run buildall

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 38.48% (10007/26003)
Line Coverage: 29.50% (83885/284315)
Region Coverage: 28.61% (43118/150700)
Branch Coverage: 25.21% (21918/86958)
Coverage Report: http://coverage.selectdb-in.cc/coverage/4407fc4f8facf3f771f017231a69aa110d183a8a_4407fc4f8facf3f771f017231a69aa110d183a8a/report/index.html

@Yukang-Lian
Copy link
Collaborator Author

run buildall

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 38.48% (10007/26003)
Line Coverage: 29.51% (83894/284318)
Region Coverage: 28.62% (43128/150700)
Branch Coverage: 25.21% (21918/86958)
Coverage Report: http://coverage.selectdb-in.cc/coverage/fcd8adb6e72de7e4d73b02f951238beba4e06592_fcd8adb6e72de7e4d73b02f951238beba4e06592/report/index.html

@Yukang-Lian Yukang-Lian force-pushed the Enhance-Delete-Compaction branch from ec93e74 to abb7d02 Compare December 10, 2024 13:24
@Yukang-Lian
Copy link
Collaborator Author

run buildall

@github-actions github-actions bot removed the approved Indicates a PR has been approved by one committer. label Dec 10, 2024
@github-actions GitHub Actions
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 38.80% (10106/26046)
Line Coverage: 29.70% (84750/285321)
Region Coverage: 28.77% (43502/151216)
Branch Coverage: 25.32% (22093/87272)
Coverage Report: http://coverage.selectdb-in.cc/coverage/abb7d02432ff6a3e030f6b89f7d08115e5765ccf_abb7d02432ff6a3e030f6b89f7d08115e5765ccf/report/index.html

@Yukang-Lian
Copy link
Collaborator Author

run buildall

@github-actions GitHub Actions
Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@doris-robot
Copy link

TeamCity be ut coverage result:
Function Coverage: 38.75% (10103/26073)
Line Coverage: 29.68% (84735/285540)
Region Coverage: 28.74% (43501/151335)
Branch Coverage: 25.30% (22100/87338)
Coverage Report: http://coverage.selectdb-in.cc/coverage/5e9fc93c7d52f682ed437443f445662727caa8b1_5e9fc93c7d52f682ed437443f445662727caa8b1/report/index.html

@github-actions GitHub Actions
Copy link
Contributor

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

@github-actions github-actions bot added the approved Indicates a PR has been approved by one committer. label Dec 12, 2024
luwei16
luwei16 reviewed Dec 12, 2024
View reviewed changes
Copy link
Contributor

@luwei16 luwei16 left a comment

Choose a reason for hiding this comment

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

Continuous deletes (without inserts) will cause a 5-second interval for tablet compaction?

@gavinchou gavinchou merged commit 55f450c into apache:master Dec 13, 2024
27 of 30 checks passed
github-actions bot pushed a commit that referenced this pull request Dec 13, 2024
@Yukang-Lian
[Enhancement](compaction) Do not set failure time when cumulative com…
1ccd088 
…paction dealing with delete rowset (#43466)

Before this PR, in cases where there is an alternating distribution of
data rowset -> delete rowset -> data rowset -> delete rowset, cumulative
compaction would only move the cumulative point forward to allow base
compaction to handle the delete rowset. Cumulative compaction itself
would not process the data and would return be marked as failure. This
would cause the compaction submission task process to pause for 5
seconds, impacting efficiency.

This PR modifies the return status to OK for such cases, which improves
the efficiency of the compaction submission task.
@github-actions github-actions bot mentioned this pull request Dec 13, 2024
Merged
mymeiyi added a commit to mymeiyi/doris that referenced this pull request Dec 16, 2024
@mymeiyi
Revert "[Enhancement](compaction) Do not set failure time when cumula…
7547650 
…tive compaction dealing with delete rowset (apache#43466)"

This reverts commit 55f450c.
gavinchou pushed a commit that referenced this pull request Dec 17, 2024
@github-actions @Yukang-Lian
branch-3.0: [Enhancement](compaction) Do not set failure time when cu…
0d7d700 
…mulative compaction dealing with delete rowset #43466 (#45404)

Cherry-picked from #43466

Co-authored-by: abmdocrt <[email protected]>
Yukang-Lian added a commit to Yukang-Lian/doris that referenced this pull request Dec 27, 2024
@Yukang-Lian
[Enhancement](compaction) Do not set failure time when cumulative com…
d95effb 
…paction dealing with delete rowset (apache#43466)

Before this PR, in cases where there is an alternating distribution of
data rowset -> delete rowset -> data rowset -> delete rowset, cumulative
compaction would only move the cumulative point forward to allow base
compaction to handle the delete rowset. Cumulative compaction itself
would not process the data and would return be marked as failure. This
would cause the compaction submission task process to pause for 5
seconds, impacting efficiency.

This PR modifies the return status to OK for such cases, which improves
the efficiency of the compaction submission task.
@Yukang-Lian Yukang-Lian mentioned this pull request Dec 27, 2024
Merged
16 tasks
yiguolei pushed a commit that referenced this pull request Dec 30, 2024
@Yukang-Lian
[Cherry-pick](branch-2.1) Pick "[Enhancement](compaction) Do not set …
1d742b5 
…failure time when cumulative compaction dealing with delete rowset (#43466)" (#46117)

Before this PR, in cases where there is an alternating distribution of
data rowset -> delete rowset -> data rowset -> delete rowset, cumulative
compaction would only move the cumulative point forward to allow base
compaction to handle the delete rowset. Cumulative compaction itself
would not process the data and would return be marked as failure. This
would cause the compaction submission task process to pause for 5
seconds, impacting efficiency.

This PR modifies the return status to OK for such cases, which improves
the efficiency of the compaction submission task.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@luwei16 luwei16 luwei16 approved these changes

@gavinchou gavinchou gavinchou approved these changes

Assignees
No one assigned
Labels
approved Indicates a PR has been approved by one committer. dev/2.1.x-conflict dev/2.1.8-merged dev/3.0.4-merged reviewed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@Yukang-Lian @doris-robot @gavinchou @luwei16 @wm1581066 @dataroaring