You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by "suxiaogang223 (via GitHub)" <gi...@apache.org> on 2026/10/23 07:08:23 UTC
[PR] [fix](be) Bound object storage client cache across credential rotations [doris]
suxiaogang223 opened a new pull request, #69511:
URL: https://github.com/apache/doris/pull/69511
### What problem does this PR solve?
Issue Number: N/A
Problem Summary:
When an external catalog repeatedly supplies new temporary S3 credentials, each AK/SK/session-token combination produces a different client cache key. The process-wide cache retains old clients after their readers finish, retaining SDK connection pools and TLS resources throughout the BE lifetime. Concurrent misses for a newly rotated credential can also construct multiple clients before publishing one cached entry.
- Replace unlimited retention with a bounded LRU and an idle TTL while retaining complete configuration equality, including credentials and session tokens.
- Share one in-flight creation for the same key; construct clients and wait outside the cache mutex. Propagate Status failures and exceptions to all waiters, remove failed flights, and allow later retries. Different keys continue independently.
- Register the cache with CacheManager for idle sweeps and memory-pressure clearing. Eviction releases only the cache reference, outside the cache mutex; active readers/writers keep independent shared ownership. Unregister before CacheManager teardown and release cached clients before AWS SDK shutdown.
- Account cache container allocations under a cache MemTracker, and expose entry, hit, miss, wait, creation, failure and eviction counters without credential labels.
### Release note
Object-storage client retention is bounded by two startup-only BE settings:
- `s3_client_cache_capacity`: default `128`; `0` disables retention while preserving sharing of in-flight creation.
- `s3_client_cache_idle_ttl_seconds`: default `600`; expired entries are pruned on access and by the existing CacheManager periodic sweep.
Active readers and writers retain their clients after eviction. The bound applies to cached references, not clients still owned by active operations. The shared factory applies this retention policy to S3 and Azure clients.
### Check List (For Author)
- Test:
- [ ] Unit Test execution (pending CI)
- Added nine cache unit tests covering 1,000 explicit token rotations with live/destruction assertions, LRU ordering, idle TTL and hit refresh, active-client ownership, 16-way same-key creation, unrelated-key progress, shared failures and exceptions with retry, pruning during creation, and zero-capacity retention.
- Added a factory integration unit test checking that rotated explicit tokens produce separate clients/providers and that the previous provider keeps its original token. Existing full-key/hash-collision coverage remains applicable.
- **Local build/tests were not completed.** A build attempt stopped in third-party setup because LLVM 20 was missing; the unit-test entry point was stopped during submodule preparation. Per request, no further local compilation or test execution was performed. clang-format 16 was run only on the 10 changed C++ files; formatting check, header/build-hygiene text checks and `git diff --check` passed.
- Suggested CI command: `./run-be-ut.sh --run --filter='S3ClientCacheTest.*:S3ClientFactoryTest.*'`.
- Real S3/TLS soak validation over sustained credential rotations is still pending; fake-client unit coverage does not establish the production heap improvement.
- Behavior changed: Yes, cache retention is bounded and same-key concurrent creation is shared.
- Does this need documentation: No separate documentation change included; settings and defaults are documented in source comments and this release note.
Self-review:
- Goal/scope: the retained-client growth mechanism is addressed in the shared factory; dedicated cache ownership tests are added but have not been executed locally.
- Concurrency: one cache mutex protects LRU/index/flights. SDK construction, waits and retired-client destruction happen outside it; cache operations do not acquire CacheManager's lock while holding it.
- Lifecycle/memory: eviction drops only cache ownership; no new background thread or reference cycle is introduced. Container allocation/free uses the same cache tracker across query and GC threads. SDK allocation size is not inferred from cache entry counts.
- Configuration: both new settings are immutable at runtime; changing them requires BE restart.
- Parallel paths: the common S3/Azure factory and rate-limiter wrapping are preserved; explicit credential changes remain distinct keys.
- Errors/observability: failure and throwing-creator paths complete waiters and permit retry; aggregate metrics contain no credentials. No persistence, wire/schema or transaction changes are involved.
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases and CI results
- [ ] Confirm document
- [ ] Add branch pick label
--
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] [fix](be) Bound object storage client cache across credential rotations [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #69511:
URL: https://github.com/apache/doris/pull/69511#issuecomment-6414237411
<details>
<summary>ClickBench: <b>Total hot run time: 24.47 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 095468ae39cf58828a1492cc0b644521784033a8, data reload: false
query1 0.00 0.00 0.00
query2 0.09 0.05 0.05
query3 0.26 0.08 0.09
query4 1.61 0.11 0.10
query5 0.23 0.25 0.24
query6 1.16 0.68 0.68
query7 0.04 0.01 0.01
query8 0.06 0.04 0.04
query9 0.40 0.35 0.32
query10 0.57 0.57 0.62
query11 0.20 0.15 0.15
query12 0.19 0.15 0.15
query13 0.49 0.50 0.49
query14 1.03 1.01 1.00
query15 0.63 0.60 0.61
query16 0.35 0.35 0.35
query17 1.15 1.19 1.12
query18 0.22 0.20 0.21
query19 2.15 1.95 2.02
query20 0.02 0.01 0.01
query21 15.49 0.20 0.13
query22 4.79 0.06 0.05
query23 16.27 0.32 0.13
query24 2.87 0.47 0.48
query25 0.09 0.06 0.06
query26 1.26 0.22 0.17
query27 0.05 0.05 0.05
query28 3.58 0.74 0.37
query29 12.53 4.10 3.27
query30 0.30 0.13 0.09
query31 2.77 0.55 0.32
query32 3.22 0.60 0.50
query33 3.20 3.21 3.21
query34 15.44 4.06 3.39
query35 3.36 3.34 3.34
query36 0.57 0.44 0.46
query37 0.10 0.06 0.07
query38 0.05 0.04 0.04
query39 0.05 0.03 0.03
query40 0.17 0.16 0.15
query41 0.09 0.03 0.03
query42 0.04 0.03 0.03
query43 0.04 0.04 0.03
Total cold run time: 97.18 s
Total hot run time: 24.47 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] [fix](be) Bound object storage client cache across credential rotations [doris]
Posted by "suxiaogang223 (via GitHub)" <gi...@apache.org>.
suxiaogang223 commented on PR #69511:
URL: https://github.com/apache/doris/pull/69511#issuecomment-6359606765
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] [fix](be) Bound object storage client cache across credential rotations [doris]
Posted by "suxiaogang223 (via GitHub)" <gi...@apache.org>.
suxiaogang223 commented on PR #69511:
URL: https://github.com/apache/doris/pull/69511#issuecomment-6411663457
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] [fix](be) Bound object storage client cache across credential rotations [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #69511:
URL: https://github.com/apache/doris/pull/69511#issuecomment-6359512131
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] [fix](be) Bound object storage client cache across credential rotations [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #69511:
URL: https://github.com/apache/doris/pull/69511#issuecomment-6413965576
<details>
<summary>TPC-H: <b>Total hot run time: 28739 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 095468ae39cf58828a1492cc0b644521784033a8, data reload: false
------ Round 1 ----------------------------------
============================================
q1 17575 5209 5073 5073
q2 2180 314 267 267
q3 10217 1409 824 824
q4 4684 457 344 344
q5 7446 812 529 529
q6 177 183 148 148
q7 762 808 591 591
q8 9279 1377 1099 1099
q9 5490 4220 4165 4165
q10 6830 1292 1026 1026
q11 431 242 227 227
q12 637 396 285 285
q13 18118 3679 3068 3068
q14 279 278 248 248
q15 q16 730 716 643 643
q17 1192 754 571 571
q18 6986 6203 6138 6138
q19 1245 967 603 603
q20 383 328 219 219
q21 6047 2423 2403 2403
q22 384 326 268 268
Total cold run time: 101072 ms
Total hot run time: 28739 ms
----- Round 2, with runtime_filter_mode=off -----
============================================
q1 5987 5898 6111 5898
q2 728 597 552 552
q3 5687 5811 5262 5262
q4 2013 2159 1542 1542
q5 5827 5814 5621 5621
q6 238 186 137 137
q7 2230 2010 1816 1816
q8 2977 2559 2639 2559
q9 8057 7870 7512 7512
q10 3613 3561 3193 3193
q11 590 404 379 379
q12 652 691 500 500
q13 3276 3676 2978 2978
q14 288 282 274 274
q15 q16 672 697 620 620
q17 7784 7018 6899 6899
q18 12930 12137 13037 12137
q19 898 774 765 765
q20 2141 2129 1915 1915
q21 5573 4608 4688 4608
q22 490 476 420 420
Total cold run time: 72651 ms
Total hot run time: 65587 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] [fix](be) Bound object storage client cache across credential rotations [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #69511:
URL: https://github.com/apache/doris/pull/69511#issuecomment-6414145168
<details>
<summary>TPC-DS: <b>Total hot run time: 150954 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 095468ae39cf58828a1492cc0b644521784033a8, data reload: false
query5 4313 573 424 424
query6 441 217 198 198
query7 4812 446 224 224
query8 321 180 167 167
query9 8782 3108 3130 3108
query10 481 342 257 257
query11 6033 3867 3575 3575
query12 146 92 87 87
query13 1258 438 311 311
query14 6111 3968 3676 3676
query14_1 3425 3463 3462 3462
query15 207 198 178 178
query16 950 465 427 427
query17 871 662 543 543
query18 2421 432 312 312
query19 199 164 139 139
query20 87 85 88 85
query21 223 142 114 114
query22 13059 13056 12723 12723
query23 13450 12875 12471 12471
query23_1 12474 12531 12570 12531
query24 7374 813 459 459
query24_1 482 468 469 468
query25 543 428 366 366
query26 1266 259 138 138
query27 2780 443 270 270
query28 4631 1932 1935 1932
query29 1556 584 422 422
query30 300 220 182 182
query31 1007 911 812 812
query32 157 91 83 83
query33 510 295 235 235
query34 906 840 486 486
query35 740 780 630 630
query36 836 802 718 718
query37 126 99 80 80
query38 1774 1727 1667 1667
query39 791 790 748 748
query39_1 724 730 738 730
query40 215 112 93 93
query41 67 66 64 64
query42 86 81 83 81
query43 340 364 308 308
query44 1293 677 682 677
query45 174 181 175 175
query46 824 958 554 554
query47 2928 2938 2771 2771
query48 290 306 216 216
query49 578 397 301 301
query50 658 269 198 198
query51 10639 10448 10274 10274
query52 78 86 76 76
query53 184 226 149 149
query54 252 200 191 191
query55 71 64 69 64
query56 228 222 207 207
query57 1660 1602 1566 1566
query58 291 267 255 255
query59 2138 2135 1930 1930
query60 278 241 222 222
query61 147 149 145 145
query62 384 333 303 303
query63 182 158 159 158
query64 2737 951 773 773
query65 3324 3268 3264 3264
query66 1785 407 298 298
query67 20622 20140 19986 19986
query68 3242 984 600 600
query69 385 284 240 240
query70 848 781 763 763
query71 303 237 207 207
query72 2840 2602 2367 2367
query73 527 523 298 298
query74 4653 4507 4402 4402
query75 2362 2302 1928 1928
query76 2419 1006 591 591
query77 355 387 282 282
query78 9753 9776 9283 9283
query79 1036 846 540 540
query80 567 443 405 405
query81 539 321 266 266
query82 828 142 112 112
query83 312 211 183 183
query84 365 128 111 111
query85 818 419 382 382
query86 317 243 232 232
query87 1997 1940 1806 1806
query88 3716 2710 2683 2683
query89 329 287 243 243
query90 1906 164 176 164
query91 164 161 128 128
query92 103 90 91 90
query93 1009 1019 576 576
query94 460 308 268 268
query95 593 391 309 309
query96 661 495 233 233
query97 2465 2420 2320 2320
query98 165 154 150 150
query99 775 760 662 662
Total cold run time: 235654 ms
Total hot run time: 150954 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] [fix](be) Bound object storage client cache across credential rotations [doris]
Posted by "suxiaogang223 (via GitHub)" <gi...@apache.org>.
suxiaogang223 closed pull request #69511: [fix](be) Bound object storage client cache across credential rotations
URL: https://github.com/apache/doris/pull/69511
--
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