You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by "zhangstar333 (via GitHub)" <gi...@apache.org> on 2026/10/22 03:57:44 UTC
[PR] [improvement](be) Drain queries and loads after reporting shutdown [doris]
zhangstar333 opened a new pull request, #69422:
URL: https://github.com/apache/doris/pull/69422
### What problem does this PR solve?
Issue Number: N/A
Problem Summary: Adapt the BE shutdown part of fd7211e to master's load paths. Wait for an accepted heartbeat to prepare is_shutdown=true, allow a configurable propagation buffer, and drain queries, load channels, load streams and StreamLoadContext lifetimes. Include upload and final transaction RPCs even when no fragment remains. Keep signal and heartbeat state lock-free and atomic; bound both shutdown phases.
### Release note
BE adds grace_shutdown_heartbeat_wait_seconds (default 30) and grace_shutdown_heartbeat_propagation_seconds (default 10). Existing query-drain and post-drain limits continue to apply. The propagation buffer is a time allowance, not proof of acknowledgement from every FE.
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [ ] Regression test
- [x] 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 <!-- Add your reason? -->
- Behavior changed:
- [ ] No.
- [ ] Yes. <!-- Explain the behavior change -->
- Does this need documentation?
- [ ] No.
- [ ] Yes. <!-- Add document PR link here. eg: https://github.com/apache/doris-website/pull/1214 -->
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label <!-- Add branch pick label that this PR should merge into -->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improvement](be) Drain queries and loads after reporting shutdown [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #69422:
URL: https://github.com/apache/doris/pull/69422#discussion_r4344456972
##########
be/src/load/channel/load_stream_mgr.h:
##########
@@ -53,8 +53,10 @@ class LoadStreamMgr {
return result;
}
- // only used by ut
- size_t get_load_stream_num() { return _load_streams_map.size(); }
+ size_t get_load_stream_num() {
+ std::lock_guard<std::mutex> lock(_lock);
Review Comment:
[P1] Keep the shutdown stream census off blocking cleanup locks
This mutex can be held for the entire duration of unfinished file writes: on an abnormal sender disconnect/idle close, `LoadStream::on_closed()` calls `clear_load()`, which erases the owning `unique_ptr` under `_lock`; destruction then reaches `IndexStream::~IndexStream()` -> `TabletStream::wait_for_flush_tasks()` and an untimed token wait/shutdown. If a flush is stalled on storage I/O, the new `wait_for_all_tasks_done()` call to this accessor blocks before it can check `grace_shutdown_wait_seconds`, so even a zero grace period cannot reach the forced-exit path. Use a nonblocking active-stream counter, or release the manager lock before destruction/waits, so polling cannot defeat the shutdown deadline.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improvement](be) Drain queries and loads after reporting shutdown [doris]
Posted by "zhangstar333 (via GitHub)" <gi...@apache.org>.
zhangstar333 commented on PR #69422:
URL: https://github.com/apache/doris/pull/69422#issuecomment-6362231096
run buildall
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improvement](be) Drain queries and loads after reporting shutdown [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #69422:
URL: https://github.com/apache/doris/pull/69422#issuecomment-6332319261
Thank you for your contribution to Apache Doris.
Don't know what should be done next? See [How to process your PR](https://doris.apache.org/community/how-to-contribute/review-and-ci).
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?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improvement](be) Drain queries and loads after reporting shutdown [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #69422:
URL: https://github.com/apache/doris/pull/69422#issuecomment-6369010057
# BE Regression && UT Coverage Report
Increment line coverage `100.00% (8/8)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/69422_7874e7c1f7fb863d2ca235e474dde0238647c55e_merge/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/69422_7874e7c1f7fb863d2ca235e474dde0238647c55e_merge/report/index.html)
| Category | Coverage |
|-------------------|--------------------|
| Function Coverage | 77.33% (36000/46556) |
| Line Coverage | 62.64% (407053/649796) |
| Region Coverage | 59.19% (344579/582153) |
| Branch Coverage | 60.11% (159209/264854) |
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improvement](be) Drain queries and loads after reporting shutdown [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #69422:
URL: https://github.com/apache/doris/pull/69422#issuecomment-6368073176
<details>
<summary>TPC-DS: <b>Total hot run time: 151206 ms</b></summary>
```
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpcds-tools
TPC-DS sf100 test result on commit 7874e7c1f7fb863d2ca235e474dde0238647c55e, data reload: false
query5 4299 602 461 461
query6 425 208 193 193
query7 4804 458 221 221
query8 318 163 150 150
query9 8794 3079 3125 3079
query10 488 322 263 263
query11 5990 3868 3585 3585
query12 143 93 93 93
query13 1252 449 318 318
query14 6172 3940 3640 3640
query14_1 3410 3401 3413 3401
query15 210 201 183 183
query16 957 453 402 402
query17 869 697 557 557
query18 2427 421 314 314
query19 210 172 149 149
query20 94 85 83 83
query21 221 124 106 106
query22 12953 13042 12946 12946
query23 13485 12801 12510 12510
query23_1 12510 12609 12657 12609
query24 7389 814 460 460
query24_1 478 468 470 468
query25 539 418 366 366
query26 1264 256 139 139
query27 2775 441 274 274
query28 4644 1921 1917 1917
query29 1563 605 448 448
query30 301 226 186 186
query31 1016 928 810 810
query32 172 97 95 95
query33 521 299 248 248
query34 921 868 495 495
query35 759 760 647 647
query36 819 814 719 719
query37 124 94 87 87
query38 1759 1743 1661 1661
query39 802 807 739 739
query39_1 743 757 747 747
query40 239 115 99 99
query41 71 72 69 69
query42 87 87 87 87
query43 350 361 310 310
query44 1307 682 684 682
query45 184 181 168 168
query46 817 1015 551 551
query47 2678 2907 2681 2681
query48 293 302 214 214
query49 567 394 294 294
query50 677 269 205 205
query51 10368 10665 10551 10551
query52 76 78 77 77
query53 184 206 149 149
query54 235 195 195 195
query55 70 63 64 63
query56 226 215 220 215
query57 1643 1584 1553 1553
query58 288 257 248 248
query59 2094 2128 1948 1948
query60 280 229 229 229
query61 144 138 144 138
query62 391 323 295 295
query63 184 149 158 149
query64 2768 948 751 751
query65 3325 3255 3273 3255
query66 1788 417 296 296
query67 20203 20253 19973 19973
query68 3307 993 618 618
query69 393 293 261 261
query70 842 769 753 753
query71 300 235 207 207
query72 2897 2620 2369 2369
query73 529 527 291 291
query74 4655 4532 4351 4351
query75 2366 2287 1904 1904
query76 2396 998 596 596
query77 361 361 264 264
query78 9858 9858 9190 9190
query79 1026 851 530 530
query80 1220 416 366 366
query81 595 321 276 276
query82 804 130 107 107
query83 301 210 188 188
query84 312 113 103 103
query85 888 419 377 377
query86 392 243 235 235
query87 2012 1944 1816 1816
query88 3723 2682 2665 2665
query89 338 280 239 239
query90 1925 170 173 170
query91 169 149 135 135
query92 99 86 91 86
query93 1033 995 572 572
query94 630 309 272 272
query95 570 417 310 310
query96 679 498 237 237
query97 2456 2442 2341 2341
query98 161 154 160 154
query99 773 756 677 677
Total cold run time: 235986 ms
Total hot run time: 151206 ms
```
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improvement](be) Drain queries and loads after reporting shutdown [doris]
Posted by "zhangstar333 (via GitHub)" <gi...@apache.org>.
zhangstar333 commented on PR #69422:
URL: https://github.com/apache/doris/pull/69422#issuecomment-6335493857
/review
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improvement](be) Drain queries and loads after reporting shutdown [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #69422:
URL: https://github.com/apache/doris/pull/69422#discussion_r4345147666
##########
be/src/runtime/exec_env.cpp:
##########
@@ -191,20 +194,41 @@ std::map<TNetworkAddress, FrontendInfo> ExecEnv::get_running_frontends() {
return res;
}
+void ExecEnv::wait_for_shutdown_heartbeat() {
+ int32_t elapsed = 0;
+ while (!k_shutdown_heartbeat_sent && elapsed < config::grace_shutdown_heartbeat_wait_seconds) {
+ sleep(1);
+ ++elapsed;
+ }
+ LOG(INFO) << "shutdown heartbeat prepared: " << k_shutdown_heartbeat_sent.load() << ", waited "
+ << elapsed << " seconds";
+ // The response may still be in transit. This buffer also gives followers time
+ // to replay shutdown state. It does not prove that every FE has applied it.
+ sleep(std::max(0, config::grace_shutdown_heartbeat_propagation_seconds));
+}
+
void ExecEnv::wait_for_all_tasks_done() {
- // For graceful shutdown, need to wait for all running queries to stop
+ // Both receiver implementations and the load coordinator must finish. For example,
+ // an HTTP upload or transaction commit can outlive all of its pipeline fragments.
int32_t wait_seconds_passed = 0;
while (true) {
int num_queries = _fragment_mgr->running_query_num();
- if (num_queries < 1) {
+ size_t num_channels = _load_channel_mgr->num_active_channels();
+ size_t num_streams = _load_stream_mgr->get_load_stream_num();
Review Comment:
[P2] Clean up partially opened receivers before counting them as active
`LoadStreamMgr` can retain a receiver after every actual stream has closed. For example, with `total_streams=2`, let the first open succeed and the second be rejected by the receiver's full heavy-work queue before registration (`PInternalService::open_load_stream`). The sender cancels its accepted streams, but `LoadStream::on_closed()` only runs once and waits for the planned total of two before calling `clear_load()`. There is no manager sweeper or other cancellation path to erase this entry, so `get_load_stream_num()` remains positive indefinitely. Adding it to this shutdown predicate therefore makes a later idle shutdown consume all of `grace_shutdown_wait_seconds` (120 seconds by default) for a failed load that cannot progress. Handle partial-open/aborted receiver cleanup before relying on this census, and cover an expected stream that never opens. The new transactional-context cleanup does not cover this receiver path.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improvement](be) Drain queries and loads after reporting shutdown [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #69422:
URL: https://github.com/apache/doris/pull/69422#issuecomment-6368172632
<details>
<summary>ClickBench: <b>Total hot run time: 24.43 s</b></summary>
```
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit 7874e7c1f7fb863d2ca235e474dde0238647c55e, data reload: false
query1 0.01 0.00 0.00
query2 0.09 0.04 0.05
query3 0.26 0.09 0.08
query4 1.62 0.11 0.11
query5 0.23 0.24 0.25
query6 1.17 0.69 0.68
query7 0.04 0.01 0.01
query8 0.06 0.04 0.03
query9 0.41 0.36 0.36
query10 0.59 0.57 0.61
query11 0.21 0.14 0.14
query12 0.18 0.15 0.15
query13 0.48 0.51 0.51
query14 1.01 1.02 1.02
query15 0.63 0.62 0.63
query16 0.36 0.35 0.36
query17 1.16 1.17 1.14
query18 0.21 0.20 0.20
query19 2.12 1.97 1.92
query20 0.01 0.01 0.02
query21 15.44 0.21 0.14
query22 4.87 0.05 0.06
query23 16.31 0.32 0.12
query24 3.04 0.73 0.38
query25 0.11 0.06 0.05
query26 1.37 0.21 0.15
query27 0.05 0.03 0.05
query28 3.38 0.80 0.39
query29 12.60 4.12 3.26
query30 0.28 0.13 0.10
query31 2.77 0.57 0.32
query32 3.23 0.60 0.52
query33 3.17 3.23 3.25
query34 15.56 4.01 3.37
query35 3.34 3.34 3.36
query36 0.55 0.45 0.44
query37 0.10 0.07 0.06
query38 0.05 0.04 0.04
query39 0.04 0.03 0.04
query40 0.18 0.14 0.14
query41 0.09 0.03 0.03
query42 0.03 0.03 0.04
query43 0.04 0.03 0.03
Total cold run time: 97.45 s
Total hot run time: 24.43 s
```
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improvement](be) Drain queries and loads after reporting shutdown [doris]
Posted by "zhangstar333 (via GitHub)" <gi...@apache.org>.
zhangstar333 commented on code in PR #69422:
URL: https://github.com/apache/doris/pull/69422#discussion_r4344812300
##########
be/src/load/channel/load_stream_mgr.h:
##########
@@ -53,8 +53,10 @@ class LoadStreamMgr {
return result;
}
- // only used by ut
- size_t get_load_stream_num() { return _load_streams_map.size(); }
+ size_t get_load_stream_num() {
+ std::lock_guard<std::mutex> lock(_lock);
Review Comment:
https://github.com/apache/doris/pull/69422/changes/77426a5992ccda1ac49cc653420a4cf002f36c70
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improvement](be) Drain queries and loads after reporting shutdown [doris]
Posted by "zhangstar333 (via GitHub)" <gi...@apache.org>.
zhangstar333 commented on PR #69422:
URL: https://github.com/apache/doris/pull/69422#issuecomment-6335909442
run buildall
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improvement](be) Drain queries and loads after reporting shutdown [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #69422:
URL: https://github.com/apache/doris/pull/69422#issuecomment-6344072875
<details>
<summary>TPC-DS: <b>Total hot run time: 151006 ms</b></summary>
```
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpcds-tools
TPC-DS sf100 test result on commit af5fe8d0fc16d9c260018bde325dc314b28bae15, data reload: false
query5 4297 614 455 455
query6 422 212 203 203
query7 4817 467 234 234
query8 318 160 149 149
query9 8793 3096 3082 3082
query10 466 316 261 261
query11 6003 3858 3569 3569
query12 150 94 89 89
query13 1253 435 350 350
query14 6108 3970 3680 3680
query14_1 3469 3456 3502 3456
query15 211 195 182 182
query16 965 438 397 397
query17 868 666 546 546
query18 2412 425 306 306
query19 192 159 139 139
query20 85 83 83 83
query21 208 134 112 112
query22 12912 13114 12882 12882
query23 13570 12925 12453 12453
query23_1 12587 12550 12664 12550
query24 7372 844 455 455
query24_1 468 458 460 458
query25 514 390 345 345
query26 1252 267 135 135
query27 2761 436 269 269
query28 4641 1930 1920 1920
query29 1575 586 426 426
query30 313 216 198 198
query31 1012 919 816 816
query32 141 90 91 90
query33 511 288 226 226
query34 911 863 507 507
query35 744 750 643 643
query36 794 844 765 765
query37 127 101 82 82
query38 1821 1722 1682 1682
query39 806 805 756 756
query39_1 737 720 742 720
query40 212 110 96 96
query41 69 65 63 63
query42 88 87 83 83
query43 358 367 313 313
query44 1372 692 669 669
query45 175 182 162 162
query46 854 976 574 574
query47 2946 3069 2716 2716
query48 291 313 218 218
query49 575 402 292 292
query50 718 306 208 208
query51 10639 10294 10450 10294
query52 83 80 76 76
query53 190 216 152 152
query54 261 197 186 186
query55 84 67 64 64
query56 234 212 215 212
query57 1648 1613 1597 1597
query58 284 266 250 250
query59 2137 2125 1958 1958
query60 293 246 229 229
query61 150 146 149 146
query62 386 332 278 278
query63 198 163 170 163
query64 2767 949 823 823
query65 3351 3283 3272 3272
query66 1766 407 297 297
query67 19948 20225 20008 20008
query68 2938 1013 608 608
query69 373 287 250 250
query70 851 809 791 791
query71 301 240 218 218
query72 2797 2558 2330 2330
query73 542 546 300 300
query74 4655 4537 4364 4364
query75 2347 2278 1910 1910
query76 2321 1022 627 627
query77 363 387 270 270
query78 9679 9729 9122 9122
query79 909 866 523 523
query80 483 423 336 336
query81 528 319 277 277
query82 204 133 104 104
query83 196 205 194 194
query84 283 128 103 103
query85 779 410 367 367
query86 257 235 233 233
query87 2018 1970 1825 1825
query88 3635 2632 2657 2632
query89 320 279 251 251
query90 2117 180 175 175
query91 163 155 126 126
query92 100 79 90 79
query93 1026 1021 574 574
query94 449 313 263 263
query95 580 385 310 310
query96 669 504 230 230
query97 2412 2448 2318 2318
query98 163 157 148 148
query99 767 765 639 639
Total cold run time: 233754 ms
Total hot run time: 151006 ms
```
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improvement](be) Drain queries and loads after reporting shutdown [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #69422:
URL: https://github.com/apache/doris/pull/69422#issuecomment-6368928342
# BE UT Coverage Report
Increment line coverage `100.00% (7/7)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/7874e7c1f7fb863d2ca235e474dde0238647c55e_7874e7c1f7fb863d2ca235e474dde0238647c55e/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/7874e7c1f7fb863d2ca235e474dde0238647c55e_7874e7c1f7fb863d2ca235e474dde0238647c55e/report/index.html)
| Category | Coverage |
|-------------------|--------------------|
| Function Coverage | 66.59% (31995/48050) |
| Line Coverage | 51.39% (337558/656831) |
| Region Coverage | 46.75% (273164/584262) |
| Branch Coverage | 48.49% (128746/265503) |
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improvement](be) Drain queries and loads after reporting shutdown [doris]
Posted by "zhangstar333 (via GitHub)" <gi...@apache.org>.
zhangstar333 commented on PR #69422:
URL: https://github.com/apache/doris/pull/69422#issuecomment-6334797843
/review
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improvement](be) Drain queries and loads after reporting shutdown [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #69422:
URL: https://github.com/apache/doris/pull/69422#issuecomment-6367872642
<details>
<summary>TPC-H: <b>Total hot run time: 28573 ms</b></summary>
```
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit 7874e7c1f7fb863d2ca235e474dde0238647c55e, data reload: false
------ Round 1 ----------------------------------
============================================
q1 17600 5253 5078 5078
q2 2103 346 250 250
q3 10253 1436 823 823
q4 4694 464 338 338
q5 7440 797 535 535
q6 180 186 154 154
q7 743 794 604 604
q8 9273 1352 1144 1144
q9 5537 4208 4222 4208
q10 6832 1295 1048 1048
q11 437 248 224 224
q12 641 391 273 273
q13 18111 3648 3061 3061
q14 276 283 239 239
q15 q16 720 732 666 666
q17 1206 753 600 600
q18 7011 6217 6113 6113
q19 1250 968 601 601
q20 393 348 224 224
q21 5634 2136 2285 2136
q22 347 295 254 254
Total cold run time: 100681 ms
Total hot run time: 28573 ms
----- Round 2, with runtime_filter_mode=off -----
============================================
q1 5701 5609 5595 5595
q2 673 577 526 526
q3 5248 5415 4959 4959
q4 2004 2163 1542 1542
q5 5313 5178 5885 5178
q6 247 193 148 148
q7 2317 2095 1888 1888
q8 3074 2958 2692 2692
q9 8016 8133 8009 8009
q10 3760 3653 3262 3262
q11 600 451 428 428
q12 701 682 519 519
q13 3508 4036 3281 3281
q14 322 312 278 278
q15 q16 701 705 635 635
q17 8446 7539 7311 7311
q18 13013 12213 12887 12213
q19 916 786 789 786
q20 2130 2151 1934 1934
q21 5630 4606 4680 4606
q22 497 432 390 390
Total cold run time: 72817 ms
Total hot run time: 66180 ms
```
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improvement](be) Drain queries and loads after reporting shutdown [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #69422:
URL: https://github.com/apache/doris/pull/69422#issuecomment-6344180679
<details>
<summary>ClickBench: <b>Total hot run time: 24.54 s</b></summary>
```
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit af5fe8d0fc16d9c260018bde325dc314b28bae15, data reload: false
query1 0.01 0.01 0.00
query2 0.10 0.05 0.04
query3 0.26 0.09 0.09
query4 1.61 0.10 0.10
query5 0.24 0.25 0.25
query6 1.16 0.70 0.67
query7 0.04 0.01 0.01
query8 0.06 0.04 0.04
query9 0.39 0.35 0.34
query10 0.61 0.59 0.59
query11 0.20 0.15 0.14
query12 0.19 0.15 0.15
query13 0.48 0.50 0.50
query14 1.03 1.01 1.01
query15 0.63 0.60 0.62
query16 0.36 0.34 0.34
query17 1.12 1.13 1.14
query18 0.22 0.19 0.19
query19 2.12 1.99 2.01
query20 0.02 0.02 0.01
query21 15.49 0.19 0.13
query22 4.84 0.05 0.05
query23 16.28 0.32 0.12
query24 2.84 0.58 0.48
query25 0.14 0.07 0.06
query26 1.52 0.21 0.16
query27 0.05 0.03 0.04
query28 3.43 0.73 0.40
query29 12.50 4.15 3.25
query30 0.28 0.13 0.12
query31 2.77 0.56 0.32
query32 3.22 0.60 0.50
query33 3.14 3.29 3.20
query34 15.29 4.00 3.39
query35 3.35 3.38 3.35
query36 0.55 0.46 0.43
query37 0.10 0.06 0.06
query38 0.06 0.04 0.04
query39 0.03 0.03 0.04
query40 0.16 0.14 0.15
query41 0.09 0.03 0.03
query42 0.04 0.03 0.03
query43 0.04 0.03 0.04
Total cold run time: 97.06 s
Total hot run time: 24.54 s
```
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improvement](be) Drain queries and loads after reporting shutdown [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #69422:
URL: https://github.com/apache/doris/pull/69422#issuecomment-6343859475
<details>
<summary>TPC-H: <b>Total hot run time: 28643 ms</b></summary>
```
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit af5fe8d0fc16d9c260018bde325dc314b28bae15, data reload: false
------ Round 1 ----------------------------------
============================================
q1 17642 5225 5069 5069
q2 2134 341 262 262
q3 10187 1382 817 817
q4 4695 460 341 341
q5 7440 805 520 520
q6 187 205 154 154
q7 774 804 614 614
q8 9256 1349 1074 1074
q9 5478 4232 4181 4181
q10 6806 1293 1028 1028
q11 443 248 236 236
q12 656 410 283 283
q13 18061 3581 3012 3012
q14 276 267 242 242
q15 q16 741 728 655 655
q17 1189 755 576 576
q18 6989 6185 6100 6100
q19 1100 970 597 597
q20 389 334 222 222
q21 4645 2382 2537 2382
q22 390 335 278 278
Total cold run time: 99478 ms
Total hot run time: 28643 ms
----- Round 2, with runtime_filter_mode=off -----
============================================
q1 5913 5937 5902 5902
q2 756 618 593 593
q3 5636 5665 5218 5218
q4 2051 2155 1578 1578
q5 5815 5743 5605 5605
q6 243 185 148 148
q7 2314 2030 1826 1826
q8 3023 2678 2709 2678
q9 7844 7501 7523 7501
q10 3624 3560 3146 3146
q11 596 415 387 387
q12 683 658 526 526
q13 3305 3604 3032 3032
q14 290 296 266 266
q15 q16 657 713 609 609
q17 7768 7069 6950 6950
q18 13036 12214 12859 12214
q19 931 789 803 789
q20 2153 2154 1924 1924
q21 5684 4768 4891 4768
q22 512 468 423 423
Total cold run time: 72834 ms
Total hot run time: 66083 ms
```
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improvement](be) Drain queries and loads after reporting shutdown [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #69422:
URL: https://github.com/apache/doris/pull/69422#discussion_r4344942386
##########
be/src/runtime/exec_env.cpp:
##########
@@ -191,20 +194,41 @@ std::map<TNetworkAddress, FrontendInfo> ExecEnv::get_running_frontends() {
return res;
}
+void ExecEnv::wait_for_shutdown_heartbeat() {
+ int32_t elapsed = 0;
+ while (!k_shutdown_heartbeat_sent && elapsed < config::grace_shutdown_heartbeat_wait_seconds) {
+ sleep(1);
+ ++elapsed;
+ }
+ LOG(INFO) << "shutdown heartbeat prepared: " << k_shutdown_heartbeat_sent.load() << ", waited "
+ << elapsed << " seconds";
+ // The response may still be in transit. This buffer also gives followers time
+ // to replay shutdown state. It does not prove that every FE has applied it.
+ sleep(std::max(0, config::grace_shutdown_heartbeat_propagation_seconds));
+}
+
void ExecEnv::wait_for_all_tasks_done() {
- // For graceful shutdown, need to wait for all running queries to stop
+ // Both receiver implementations and the load coordinator must finish. For example,
+ // an HTTP upload or transaction commit can outlive all of its pipeline fragments.
int32_t wait_seconds_passed = 0;
while (true) {
int num_queries = _fragment_mgr->running_query_num();
- if (num_queries < 1) {
+ size_t num_channels = _load_channel_mgr->num_active_channels();
+ size_t num_streams = _load_stream_mgr->get_load_stream_num();
+ size_t num_loads = StreamLoadContext::num_active_contexts();
Review Comment:
[P2] Release failed transactional contexts before counting them as active
A transactional INSERT can leave this count positive until process teardown after setup has already failed. FragmentMgr::exec_plan_fragment() puts its StreamLoadContext in NewLoadStreamMgr before execute_plan_fragment(), but an early _get_or_create_query_ctx() failure (for example, a backend tag change causes an older workload-group publication to shut down the selected normal group before set_workload_group()) returns before any PipelineFragmentContext and completion callback exist. The manager retains a shared owner with no timeout cleanup; the rollback RPC only cancels its pipe. The new lifetime census therefore makes a later, otherwise idle shutdown wait all of grace_shutdown_wait_seconds (120 seconds by default) for a failed load that cannot make progress. Release the registered context and finish rollback on these early setup failures, and cover failure before fragment creation in the shutdown tests.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improvement](be) Drain queries and loads after reporting shutdown [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #69422:
URL: https://github.com/apache/doris/pull/69422#discussion_r4345545128
##########
be/src/runtime/exec_env.cpp:
##########
@@ -191,20 +194,44 @@ std::map<TNetworkAddress, FrontendInfo> ExecEnv::get_running_frontends() {
return res;
}
+void ExecEnv::wait_for_shutdown_heartbeat() {
+ int32_t elapsed = 0;
+ while (!k_shutdown_heartbeat_sent && elapsed < config::grace_shutdown_heartbeat_wait_seconds) {
+ sleep(1);
+ ++elapsed;
+ }
+ LOG(INFO) << "shutdown heartbeat prepared: " << k_shutdown_heartbeat_sent.load() << ", waited "
+ << elapsed << " seconds";
+ // The response may still be in transit. This buffer also gives followers time
+ // to replay shutdown state. It does not prove that every FE has applied it.
+ sleep(std::max(0, config::grace_shutdown_heartbeat_propagation_seconds));
+}
+
void ExecEnv::wait_for_all_tasks_done() {
- // For graceful shutdown, need to wait for all running queries to stop
+ // Both receiver implementations and the load coordinator must finish. For example,
+ // an HTTP upload or transaction commit can outlive all of its pipeline fragments.
int32_t wait_seconds_passed = 0;
while (true) {
int num_queries = _fragment_mgr->running_query_num();
- if (num_queries < 1) {
+ size_t num_channels = _load_channel_mgr->num_active_channels();
+ // TODO: Fix partial-open receiver cleanup (see LoadStream::on_closed()) so
+ // abandoned entries do not consume the full grace_shutdown_wait_seconds.
+ // Keep counting receivers until then to avoid exiting during active loads.
+ size_t num_streams = _load_stream_mgr->get_load_stream_num();
+ size_t num_loads = StreamLoadContext::num_active_contexts();
Review Comment:
[P2] Release failed multi-table routine-load contexts before draining
This lifetime census also counts abandoned multi-table routine loads. If `_get_or_create_query_ctx()` fails before creating a child fragment (for example, the selected workload group is shut down between lookup and registration), `MultiTablePipe::exec_plans()` has already incremented `_inflight_cnt` and registered the child contexts. Its callback-taking `FragmentMgr` overload returns without a completion callback, so `handle_consume_finished()` cannot reach zero. `HANDLE_MULTI_TABLE_ERROR` then waits forever at `load_status_future.get()`, and `_handle_consumer_finished()` never unregisters the children. With no live query/channel, `num_active_contexts()` nevertheless stays positive indefinitely, making a later idle shutdown consume the full `grace_shutdown_wait_seconds` (120 seconds by default). This is a separate instance from the existing transactional-INSERT thread: this direct callback overload bypasses the new `need_txn` cleanup. Balance the failed child-plan reservation and
release its registrations on the pre-callback error path, and cover that multi-table failure before using all context lifetimes as drainable work.
##########
be/src/load/channel/load_channel_mgr.cpp:
##########
@@ -64,6 +64,11 @@ LoadChannelMgr::LoadChannelMgr() : _stop_background_threads_latch(1) {
});
}
+size_t LoadChannelMgr::num_active_channels() {
+ std::lock_guard<std::mutex> lock(_lock);
Review Comment:
[P1] Keep the channel census off blocking query cleanup
This mutex can also cover synchronous filesystem cleanup. `LoadChannelMgr::open()` holds `_lock` while constructing `LoadChannel`; the constructor acquires a `shared_ptr<QueryContext>` for the load ID, keeps only its `ResourceContext`, then releases the temporary owner. If a late receiver open races completion/cancellation of a locally spilling INSERT SELECT, that temporary can become the last owner: the query registry and resource controller hold weak references. `QueryContext::~QueryContext()` then calls `delete_query_spill_directory()` -> `LocalFileSystem::delete_directory_impl()` -> `std::filesystem::remove_all()` under this manager lock. A stalled spill filesystem therefore blocks the new shutdown poll before it can check `grace_shutdown_wait_seconds`, even when the limit is zero. The atomic stream-manager fix covers a different manager and does not cover this constructor path. Make this census nonblocking, or move construction and possible last-owner cleanup outside the mute
x.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improvement](be) Drain queries and loads after reporting shutdown [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #69422:
URL: https://github.com/apache/doris/pull/69422#discussion_r4374287393
##########
be/src/runtime/exec_env.cpp:
##########
@@ -191,20 +194,44 @@ std::map<TNetworkAddress, FrontendInfo> ExecEnv::get_running_frontends() {
return res;
}
+void ExecEnv::wait_for_shutdown_heartbeat() {
+ int32_t elapsed = 0;
+ while (!k_shutdown_heartbeat_sent && elapsed < config::grace_shutdown_heartbeat_wait_seconds) {
+ sleep(1);
+ ++elapsed;
+ }
+ LOG(INFO) << "shutdown heartbeat prepared: " << k_shutdown_heartbeat_sent.load() << ", waited "
+ << elapsed << " seconds";
+ // The response may still be in transit. This buffer also gives followers time
+ // to replay shutdown state. It does not prove that every FE has applied it.
+ sleep(std::max(0, config::grace_shutdown_heartbeat_propagation_seconds));
+}
+
void ExecEnv::wait_for_all_tasks_done() {
- // For graceful shutdown, need to wait for all running queries to stop
+ // Both receiver implementations and the load coordinator must finish. For example,
+ // an HTTP upload or transaction commit can outlive all of its pipeline fragments.
int32_t wait_seconds_passed = 0;
while (true) {
int num_queries = _fragment_mgr->running_query_num();
- if (num_queries < 1) {
+ size_t num_channels = _load_channel_mgr->num_active_channels();
+ // TODO: Fix partial-open receiver cleanup (see LoadStream::on_closed()) so
+ // abandoned entries do not consume the full grace_shutdown_wait_seconds.
+ // Keep counting receivers until then to avoid exiting during active loads.
+ size_t num_streams = _load_stream_mgr->get_load_stream_num();
+ size_t num_loads = StreamLoadContext::num_active_contexts();
+ if (num_queries == 0 && num_channels == 0 && num_streams == 0 && num_loads == 0) {
Review Comment:
[P2] Count accepted receiver opens before declaring shutdown idle
An accepted `PInternalService::open_load_stream()` RPC is not counted until after its tablet lookups and `LoadStreamMgr` registration. In the supported shared-nothing V2 path, `ExecEnv::get_tablet()` can block on the tablet-shard shared lock: a concurrent CREATE_TABLE for another tablet in that shard holds the exclusive lock across filesystem `exists`/`create_directory` calls (`TabletManager::create_tablet()`). If this BE only receives the load and its coordinator runs elsewhere, all four counts here remain zero during that wait. With a prompt shutdown heartbeat, a filesystem stall longer than the propagation/post-delay allowances but shorter than the sender's 60-second open timeout can therefore cause this BE to exit without using its 120-second work-drain budget, failing a load that could still finish. Reserve accepted opens before queuing/blocking setup and retain that reservation until registration takes over or the open fails. This differs from the existing partial-open threa
d, which concerns an overcount after registration.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improvement](be) Drain queries and loads after reporting shutdown [doris]
Posted by "zhangstar333 (via GitHub)" <gi...@apache.org>.
zhangstar333 commented on PR #69422:
URL: https://github.com/apache/doris/pull/69422#issuecomment-6332320126
/review
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improvement](be) Drain queries and loads after reporting shutdown [doris]
Posted by "zhangstar333 (via GitHub)" <gi...@apache.org>.
zhangstar333 commented on PR #69422:
URL: https://github.com/apache/doris/pull/69422#issuecomment-6333375448
/review
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improvement](be) Drain queries and loads after reporting shutdown [doris]
Posted by "zhangstar333 (via GitHub)" <gi...@apache.org>.
zhangstar333 commented on PR #69422:
URL: https://github.com/apache/doris/pull/69422#issuecomment-6408933911
/review
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [improvement](be) Drain queries and loads after reporting shutdown [doris]
Posted by "zhangstar333 (via GitHub)" <gi...@apache.org>.
zhangstar333 commented on PR #69422:
URL: https://github.com/apache/doris/pull/69422#issuecomment-6333951009
/review
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org