-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[Opt](streamload) Commit txn on BE coordinator in cloud mode to avoid RPC and lock overhead #36237
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
[Opt](streamload) Commit txn on BE coordinator in cloud mode to avoid RPC and lock overhead #36237
Conversation
…c and lock overhead
|
Thank you for your contribution to Apache Doris. Since 2024-03-18, the Document has been moved to doris-website. |
|
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.
clang-tidy made some suggestions
| put_result.params.is_mow_table) || | ||
| (put_result.__isset.pipeline_params && put_result.pipeline_params.__isset.is_mow_table && | ||
| put_result.pipeline_params.is_mow_table)) { | ||
| return true; |
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.
warning: redundant boolean literal in conditional return statement [readability-simplify-boolean-expr]
be/src/runtime/stream_load/stream_load_context.cpp:353:
- if ((put_result.__isset.params && put_result.params.__isset.is_mow_table &&
- put_result.params.is_mow_table) ||
- (put_result.__isset.pipeline_params && put_result.pipeline_params.__isset.is_mow_table &&
- put_result.pipeline_params.is_mow_table)) {
- return true;
- }
- return false;
+ return (put_result.__isset.params && put_result.params.__isset.is_mow_table &&
+ put_result.params.is_mow_table) ||
+ (put_result.__isset.pipeline_params && put_result.pipeline_params.__isset.is_mow_table &&
+ put_result.pipeline_params.is_mow_table);|
TeamCity be ut coverage result: |
TPC-H: Total hot run time: 39630 ms |
TPC-DS: Total hot run time: 173660 ms |
ClickBench: Total hot run time: 31.7 s |
|
refer to #34548 |
5dbd545 to
062b257
Compare
|
run buildall |
062b257 to
22b306e
Compare
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.
clang-tidy made some suggestions
|
run buildall |
|
clang-tidy review says "All clean, LGTM! 👍" |
1 similar comment
|
clang-tidy review says "All clean, LGTM! 👍" |
liaoxin01
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
|
PR approved by at least one committer and no changes requested. |
|
PR approved by anyone and no changes requested. |
|
TeamCity be ut coverage result: |
zhannngchen
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
TPC-H: Total hot run time: 39877 ms |
TPC-DS: Total hot run time: 173411 ms |
ClickBench: Total hot run time: 30.74 s |
Fix incomplete optimization introduced by apache#36237 [Opt](streamload) Commit txn on BE coordinator in cloud mode to avoid RPC and lock overhead (apache#36237)
Fix incomplete optimization introduced by apache#36237 [Opt](streamload) Commit txn on BE coordinator in cloud mode to avoid RPC and lock overhead (apache#36237)
Fix incomplete optimization introduced by apache#36237 [Opt](streamload) Commit txn on BE coordinator in cloud mode to avoid RPC and lock overhead (apache#36237)
…troduced by apache#36237 (apache#36496) Fix incomplete optimization introduced by apache#36237 [Opt](streamload) Commit txn on BE coordinator in cloud mode to avoid RPC and lock overhead (apache#36237)
… RPC and lock overhead (#36237) In cloud mode, FE master is not mandatory for committing load txn, and there is overhead (locks and PRC) if we commit load txn via FE master.
…7347) ## Proposed changes introduce by #36237 If meets some error like `out of range`, routine load job state should change from running to pause. When rollback transaction, the RPC will send to meta service directly, which can not change state by transaction information. This pr sends RPC to FE first to solve this problem.
…7347) ## Proposed changes introduce by #36237 If meets some error like `out of range`, routine load job state should change from running to pause. When rollback transaction, the RPC will send to meta service directly, which can not change state by transaction information. This pr sends RPC to FE first to solve this problem.
In cloud mode, FE master is not mandatory for committing load txn, and there is overhead (locks and PRC) if we commit load txn via FE master.