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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
