-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[fix](replica) skip missing version should care catchup #49999
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.
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
Conversation
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
run buildall |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (1)
fe/fe-core/src/main/java/org/apache/doris/catalog/Tablet.java:308
- The removal of the dedicated check for replicas with a failed version (lastFailedVersion > 0) might alter the intended behavior. Please verify that the new condition combined with allowMissingVersion accurately handles both missing and failed versions.
boolean allowMissingVersion) {
TPC-H: Total hot run time: 34806 ms |
TPC-DS: Total hot run time: 192761 ms |
ClickBench: Total hot run time: 31.28 s |
|
PR approved by at least one committer and no changes requested. |
|
PR approved by anyone and no changes requested. |
yujun777
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
| continue; | ||
| } | ||
|
|
||
| // Skip the missing version replica |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if not allow missing version, must check last fail version < 0, line 318 ~ 321 cannot deleted, keep it
yujun777
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
need update
| continue; | ||
| } | ||
|
|
||
| if (!replica.checkVersionCatchUp(visibleVersion, false)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if be not found rowset [1, visible version], be maybe also throw a error
yujun777
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
run buildall |
|
PR approved by at least one committer and no changes requested. |
TPC-H: Total hot run time: 34599 ms |
TPC-DS: Total hot run time: 193276 ms |
ClickBench: Total hot run time: 29.1 s |
What problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
Release note
None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)