-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
[fix](cloud-mow) Fix sending commiting rpc to FE twice problem #41395
Conversation
Thank you for your contribution to Apache Doris. Since 2024-03-18, the Document has been moved to doris-website. |
run buildall |
TeamCity be ut coverage result: |
TPC-H: Total hot run time: 42419 ms
|
TPC-DS: Total hot run time: 192131 ms
|
ClickBench: Total hot run time: 33.47 s
|
run buildall |
TeamCity be ut coverage result: |
run buildall |
TeamCity be ut coverage result: |
run buildall |
TPC-H: Total hot run time: 41651 ms
|
TPC-DS: Total hot run time: 193293 ms
|
ClickBench: Total hot run time: 33.44 s
|
fe/fe-core/src/main/java/org/apache/doris/cloud/transaction/CloudGlobalTransactionMgr.java
Show resolved
Hide resolved
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
PR approved by at least one committer and no changes requested. |
PR approved by anyone and no changes requested. |
run buildall |
@@ -475,6 +475,10 @@ private void commitTransaction(long dbId, List<Table> tableList, long transactio | |||
|
|||
List<OlapTable> mowTableList = getMowTableList(tableList, tabletCommitInfos); | |||
if (!mowTableList.isEmpty()) { | |||
// may be this txn has been calculated by previously task but commit rpc is timeout, |
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.
use BE rather than "be", it's confused with the english word "be"
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.
@zhannngchen here it means may be, not BE
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
PR approved by at least one committer and no changes requested. |
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 performance |
Here is an expample while commit rpc will send twice: 1. first commit request try to get delete bitmap lock, there is 2 lock(fe and ms), which take over rpc timeout(60s default) but not send DELETE_BITMAP_LOCK_ERR to be, and fe will continue to send calculate delete bitmap task to be 2. be calculate delete bitmap success and remove delete bitmap cache 3. because step 1 take over 60s, be will resend commit rpc to fe 4. after first commit request done, the second commit request from step 3 will do the same thing, but delete bitmap cache has been delete by first commit, so it will fail on be 5. client will see commit fail this pr check transaction status before sending delete bitmap task to be, if transaction status is committed or visible, it no need to recalculate delete bitmap again, just retrun rpc success to be.
…oblem (#43854) Cherry-picked from #41395 Co-authored-by: huanghaibin <[email protected]>
Here is an expample while commit rpc will send twice:
this pr check transaction status before sending delete bitmap task to be, if transaction status is committed or visible, it no need to recalculate delete bitmap again, just retrun rpc success to be.