Skip to content

[Optimize](Variant) optimize schema update performance #45480

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
Merged
eldenmoon merged 4 commits into from
Dec 20, 2024
Merged

[Optimize](Variant) optimize schema update performance #45480

eldenmoon merged 4 commits into from
Dec 20, 2024

Conversation

@eldenmoon
Copy link
Member

@eldenmoon eldenmoon commented Dec 16, 2024

When update schema with high concurrency, updaing schemas cost is expensive.

  1. update schema only when rows is not 0
  2. copy_from is expensive, use copy constructor

What problem does this PR solve?

Issue Number: close #xxx

Related PR: #xxx

Problem Summary:

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

@eldenmoon
[Optimize](Variant) optimize schema update performance
8930824 
When update schema with high concurrency, updaing schemas cost is expensive.
1. update schema only when rows is not 0
2. copy_from is expensive, use copy constructor
@Thearas
Copy link
Contributor

Thearas commented Dec 16, 2024

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?

@eldenmoon
Copy link
Member Author

eldenmoon commented Dec 16, 2024

run buildall

@github-actions
Copy link
Contributor

github-actions bot commented Dec 16, 2024

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

@doris-robot
Copy link

doris-robot commented Dec 16, 2024

TeamCity be ut coverage result:
Function Coverage: 38.85% (10125/26064)
Line Coverage: 29.76% (85084/285909)
Region Coverage: 28.82% (43653/151486)
Branch Coverage: 25.35% (22169/87454)
Coverage Report: http://coverage.selectdb-in.cc/coverage/893082451f3bf30d28a50d4676ee25f381eb759d_893082451f3bf30d28a50d4676ee25f381eb759d/report/index.html

@eldenmoon
Copy link
Member Author

eldenmoon commented Dec 16, 2024

run buildall

@github-actions
Copy link
Contributor

github-actions bot commented Dec 16, 2024

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

@doris-robot
Copy link

doris-robot commented Dec 16, 2024

TeamCity be ut coverage result:
Function Coverage: 38.85% (10125/26064)
Line Coverage: 29.77% (85122/285911)
Region Coverage: 28.82% (43654/151488)
Branch Coverage: 25.35% (22168/87456)
Coverage Report: http://coverage.selectdb-in.cc/coverage/9efc0374647ddd24045555eba91e2629db234735_9efc0374647ddd24045555eba91e2629db234735/report/index.html

@eldenmoon
Copy link
Member Author

eldenmoon commented Dec 17, 2024

run buildall

@github-actions
Copy link
Contributor

github-actions bot commented Dec 17, 2024

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

@doris-robot
Copy link

doris-robot commented Dec 17, 2024

TeamCity be ut coverage result:
Function Coverage: 38.86% (10128/26064)
Line Coverage: 29.78% (85139/285887)
Region Coverage: 28.82% (43660/151481)
Branch Coverage: 25.36% (22174/87438)
Coverage Report: http://coverage.selectdb-in.cc/coverage/39a47e7eff29868d1f87687dddca769dfbe550e3_39a47e7eff29868d1f87687dddca769dfbe550e3/report/index.html

@eldenmoon
Copy link
Member Author

eldenmoon commented Dec 17, 2024

run buildall

@github-actions
Copy link
Contributor

github-actions bot commented Dec 17, 2024

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

@doris-robot
Copy link

doris-robot commented Dec 17, 2024

TeamCity be ut coverage result:
Function Coverage: 38.86% (10128/26064)
Line Coverage: 29.77% (85125/285911)
Region Coverage: 28.83% (43666/151486)
Branch Coverage: 25.37% (22182/87442)
Coverage Report: http://coverage.selectdb-in.cc/coverage/38b5a6a8ea4e1b48f2f999f05fd733df89638eb2_38b5a6a8ea4e1b48f2f999f05fd733df89638eb2/report/index.html

csun5285
csun5285 reviewed Dec 20, 2024
View reviewed changes
be/src/olap/rowset_builder.cpp
// After adding a column v2, the schema version increases, max_version_schema needs to be updated.
// _tablet_schema includes k, v, and v2
// if v is a variant, need to add the columns decomposed from the v to the _tablet_schema.
if (_tablet_schema->num_variant_columns() > 0) {
Copy link
Contributor

@csun5285 csun5285 Dec 20, 2024

Choose a reason for hiding this comment

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

may be need to revert it

Copy link
Member Author

@eldenmoon eldenmoon Dec 20, 2024

Choose a reason for hiding this comment

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

done

@eldenmoon eldenmoon force-pushed the opt-update-schema branch 2 times, most recently from 77eee24 to e9f59bf Compare December 20, 2024 04:33
@eldenmoon
Copy link
Member Author

eldenmoon commented Dec 20, 2024

run buildall

@eldenmoon
Copy link
Member Author

eldenmoon commented Dec 20, 2024

run buildall

csun5285
csun5285 approved these changes Dec 20, 2024
View reviewed changes
Copy link
Contributor

@csun5285 csun5285 left a comment

Choose a reason for hiding this comment

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

LGTM

@github-actions
Copy link
Contributor

github-actions bot commented Dec 20, 2024

PR approved by anyone and no changes requested.

@doris-robot
Copy link

doris-robot commented Dec 20, 2024

TeamCity be ut coverage result:
Function Coverage: 38.88% (10132/26063)
Line Coverage: 29.80% (85230/286006)
Region Coverage: 28.91% (43484/150397)
Branch Coverage: 25.44% (22172/87152)
Coverage Report: http://coverage.selectdb-in.cc/coverage/2ef1bea8cfef13be8c4044fe5d28ff2c3ef8a19a_2ef1bea8cfef13be8c4044fe5d28ff2c3ef8a19a/report/index.html

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

github-actions bot commented Dec 20, 2024

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

@eldenmoon eldenmoon merged commit 62a6360 into apache:master Dec 20, 2024
22 of 25 checks passed
@eldenmoon eldenmoon deleted the opt-update-schema branch December 20, 2024 08:33
github-actions bot pushed a commit that referenced this pull request Dec 20, 2024
@eldenmoon
[Optimize](Variant) optimize schema update performance (#45480)
ebb89c0 
When update schema with high concurrency, updaing schemas cost is
expensive.
1. update schema only when rows is not 0
2. copy_from is expensive, use copy constructor
@github-actions github-actions bot mentioned this pull request Dec 20, 2024
Merged
eldenmoon added a commit to eldenmoon/incubator-doris that referenced this pull request Dec 20, 2024
@eldenmoon
[Optimize](Variant) optimize schema update performance (apache#45480)
85563b7 
When update schema with high concurrency, updaing schemas cost is
expensive.
1. update schema only when rows is not 0
2. copy_from is expensive, use copy constructor
@eldenmoon eldenmoon mentioned this pull request Dec 20, 2024
Merged
eldenmoon added a commit that referenced this pull request Dec 21, 2024
@eldenmoon
[Optimize](Variant) optimize schema update performance (#45480) (#45731)
39c69c7 
(#45480)
eldenmoon added a commit that referenced this pull request Dec 23, 2024
@github-actions @eldenmoon
branch-3.0: [Optimize](Variant) optimize schema update performance #4…
f01dd9e 
…5480 (#45730)

Cherry-picked from #45480

---------

Co-authored-by: lihangyu <lihangyu@selectdb.com>
deardeng pushed a commit to deardeng/incubator-doris that referenced this pull request Jan 6, 2025
@eldenmoon
[Optimize](Variant) optimize schema update performance (apache#45480)
d7a8d77 
When update schema with high concurrency, updaing schemas cost is
expensive.
1. update schema only when rows is not 0
2. copy_from is expensive, use copy constructor
@wm1581066 wm1581066 added the usercase Important user case type label label Feb 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@qidaye qidaye qidaye approved these changes

+1 more reviewer

@csun5285 csun5285 csun5285 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

approved Indicates a PR has been approved by one committer. dev/2.1.8-merged dev/3.0.4-merged reviewed usercase Important user case type label

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@eldenmoon @Thearas @doris-robot @qidaye @csun5285 @wm1581066