[refactor](storage) Unify BE and Recycler object clients - #66350
Conversation
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
run buildall |
|
/review |
Move the shared S3/Azure clients, credential factories, pagination, and recursive deletion orchestration into common/client. Keep BE and Recycler policies in runtime hooks and adapters. Reuse the CPU-aware limiter manager and token-bucket fixes needed from apache#65420, but apply admission before each real provider SDK request instead of wrapping logical operations. Test: Not run (per request).
f30abce to
dc25d3e
Compare
|
Codex automated review failed and did not complete. Error: Codex completed, but no new pull request review was submitted for the current head SHA. Please inspect the workflow logs and rerun the review after the underlying issue is resolved. |
|
run buildall |
|
/review |
|
run buildall |
|
/review |
|
Codex automated review failed and did not complete. Error: Codex completed, but no new pull request review was submitted for the current head SHA. Please inspect the workflow logs and rerun the review after the underlying issue is resolved. |
1. What does this PR do?
BE and Cloud Recycler currently maintain separate object-storage abstractions and separate S3/Azure client implementations. Although they call the same provider SDKs, they duplicate credential construction, error conversion, metrics, pagination, batch deletion, and provider-specific compatibility logic. The two implementations have also evolved different APIs and behavior, which makes fixes easy to apply to only one side.
This PR consolidates the provider-independent interface and the S3/Azure implementations under
common/cpp/client:ObjStorageClientinterface and shared request/response types.common/cpp/client.ObjectListIterator.ObjectStoreInfoPBwith the optional AWS session token so AK/SK/token credentials follow the same path in BE and Recycler; redact the token from logs and debug output.Before the refactor, BE and Recycler reached the same provider SDKs through parallel implementations, with cross-cutting behavior duplicated on both sides:
After the refactor, BE and Recycler remain separate policy owners at the top, while both depend on one shared provider layer centered below them:
The key boundary is that provider mechanics are shared, while environment-specific decisions remain in BE and Recycler adapters. This prevents the S3/Azure implementations from depending on BE or Recycler configuration and keeps future provider fixes in one place.
Validation: local compilation and tests were not run as requested. The changed C++ files were formatted, and the final patch passed static whitespace, stale-include, and conflict-marker checks.
2. How are the different behaviors unified?
doris::io::ObjStorageClient, using BEStatus-style codes and eager list resultsdoris::cloud::ObjStorageClient, using integer return codes and a separate iterator APIdoris::ObjStorageClientand one set of request/response types.doris::ioaliases keep BE call sites source-compatible, while Recycler adapters convert the shared response back to its existing integer-facing API.common/cpp/client; both BE and Recycler call the same provider code.retand error messageObjectStorageResponsealways carries a Doris status code, HTTP code, and request ID. Provider failures continue updatingrecord_object_request_failed. Local BE limiter rejection keeps HTTP code0, so it remains distinct from a provider HTTP 429.false/empty to indicate completionEND_OF_FILEis an internal iterator sentinel and is converted to a successful empty result bynext(). It is distinct from a real providerNOT_FOUND, so a provider 404 is not silently swallowed.NoSuchKeywas treated as an empty list for S3-compatible providers such as TOSNoSuchKey-as-empty behavior once for both callers.recycler_max_tasks_per_batch.1000for S3 and256for Azure). Shared recursive deletion uses that limit, and direct oversized delete calls are still split defensively by the provider client.S3ClientFactory; empty credentials meant anonymous accessAwsCredentialFactoryimplements static AK/SK/session-token credentials, provider chains, role ARN, and external ID once. The caller explicitly selects the previous empty-credential behavior:ANONYMOUSfor BE andDEFAULT_CHAINfor Recycler.AzureAuthFactorycreates the container client and shared-key credential for both. BE's TLS CA diagnostic context remains attached to Azure errors.S3ClientConfsupported a token, butObjectStoreInfoPBdid not carry it through the storage-vault pathObjectStoreInfoPBObjectStoreInfoPB -> S3Conf -> AwsCredentialFactorynow carries AK/SK/token consistently. Token values are cleared or masked before logging.client_bvar::ScopedLatencyRAII timer. Public/common stopwatch utilities are unchanged.