Skip to content

[refactor](storage) Unify BE and Recycler object clients - #66350

Open
sollhui wants to merge 1 commit into
apache:masterfrom
sollhui:agent/unify-be-recycler-obj-client
Open

[refactor](storage) Unify BE and Recycler object clients#66350
sollhui wants to merge 1 commit into
apache:masterfrom
sollhui:agent/unify-be-recycler-obj-client

Conversation

@sollhui

@sollhui sollhui commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

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/client:

  • Add one shared ObjStorageClient interface and shared request/response types.
  • Move the S3 and Azure provider clients, common stream helpers, latency metrics, and credential factories into common/client.
  • Replace BE's eager list API and Recycler's separate iterator with one paginated ObjectListIterator.
  • Move recursive-delete orchestration into the shared client and expose provider capabilities such as maximum delete batch size.
  • Keep BE- and Recycler-specific policy outside the provider clients through runtime hooks and thin adapters.
  • Extend ObjectStoreInfoPB with 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.
  • Remove the duplicated BE clients, Recycler clients, and the BE rate-limiting decorator.

The branch is based on master after #65420. It keeps the CPU-aware limiter configuration and token-bucket behavior from that PR, while moving admission to the shared client immediately before each real provider SDK request.

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?

Behavior BE before Recycler before Unified behavior
Client API doris::io::ObjStorageClient, using BE Status-style codes and eager list results doris::cloud::ObjStorageClient, using integer return codes and a separate iterator API One doris::ObjStorageClient and one set of request/response types. doris::io aliases keep BE call sites source-compatible, while Recycler adapters convert the shared response back to its existing integer-facing API.
Provider implementation Separate BE S3/Azure clients Separate Recycler S3/Azure clients One S3 client and one Azure client in common/client; both BE and Recycler call the same provider code.
Error model BE status code plus HTTP code/request ID in selected paths Recycler-specific ret and error message ObjectStorageResponse always carries a Doris status code, HTTP code, and request ID. Provider failures continue updating record_object_request_failed. Local BE limiter rejection keeps HTTP code 0, so #65420's provider-429 retry distinction is preserved.
End of listing Eager list returned a completed vector Iterator used false/empty to indicate completion END_OF_FILE is an internal iterator sentinel and is converted to a successful empty result by next(). It is distinct from a real provider NOT_FOUND, so a provider 404 is not silently swallowed.
Pagination BE fetched all S3/Azure pages inside one logical call Recycler fetched pages lazily Both use the same lazy iterator. Every page is one real provider request, with its own latency measurement and rate-limit admission.
Missing S3 prefix NoSuchKey was treated as an empty list for S3-compatible providers such as TOS Same compatibility was maintained separately The shared S3 iterator preserves the NoSuchKey-as-empty behavior once for both callers.
Recursive deletion BE and Recycler had different recursive-delete implementations Recycler supported expiration filtering, a worker pool, and configurable tasks per batch Shared orchestration performs listing, expiration filtering, task grouping, and error propagation. BE uses the synchronous defaults; Recycler injects its existing executor and recycler_max_tasks_per_batch.
Delete batch size S3 and Azure batching was embedded in each BE client Separate provider batching existed in Recycler Provider capability reports the limit (1000 for S3 and 256 for Azure). Shared recursive deletion uses that limit, and direct oversized delete calls are still split defensively by the provider client.
AWS credentials BE constructed v1/v2 providers in S3ClientFactory; empty credentials meant anonymous access Recycler maintained another provider-construction path; empty credentials used the default chain AwsCredentialFactory implements static AK/SK/session-token credentials, provider chains, role ARN, and external ID once. The caller explicitly selects the previous empty-credential behavior: ANONYMOUS for BE and DEFAULT_CHAIN for Recycler.
Azure credentials BE created a shared-key credential and kept it for presigned URLs Recycler created its own shared-key credential AzureAuthFactory creates the container client and shared-key credential for both. BE's TLS CA diagnostic context remains attached to Azure errors.
Session token S3ClientConf supported a token, but ObjectStoreInfoPB did not carry it through the storage-vault path Recycler could not consume an AK/SK session token from ObjectStoreInfoPB ObjectStoreInfoPB -> S3Conf -> AwsCredentialFactory now carries AK/SK/token consistently. Token values are cleared or masked before logging.
Rate limiting After #65420, a BE decorator admitted one logical client operation; cloud mode limited only internal buckets Recycler invoked its own limiter/fault injection around provider operations The shared clients expose runtime hooks and acquire admission immediately before each SDK call, including every list page and provider-sized delete batch. BE supplies the #65420 manager (all buckets in non-cloud mode, only internal vault buckets in cloud mode); Recycler supplies its existing limiter and fault-injection policy.
Latency metrics BE and Recycler each used their own stopwatch macro around duplicated calls Same metrics were recorded through separate helpers Provider clients use a private client_bvar::ScopedLatency RAII timer. Public/common stopwatch utilities are unchanged.
Recycler-only control APIs BE implementations and tests needed unrelated stubs when sharing an interface S3 lifecycle, versioning, and multipart-abort APIs were Recycler-facing These APIs are part of the shared interface with a default not-supported response. Provider clients implement the supported behavior, while unrelated BE mocks no longer need dummy methods.

3. Architecture before and after

Before this PR, BE and Recycler reached the same provider SDKs through parallel implementations. Cross-cutting behavior was duplicated on both sides:

                                      BEFORE

  +-------------------- BE --------------------+
  |                                            |
  |  BE call sites                             |
  |       |                                    |
  |       v                                    |
  |  S3ClientFactory                           |
  |       |                                    |
  |       v                                    |
  |  RateLimitedObjStorageClient               |
  |       |                                    |
  |       +----------------+----------------+   |
  |                        |                |   |
  |                        v                v   |
  |              BE S3 client       BE Azure client
  |                        |                |   |
  |  BE credentials / eager listing / recursive delete
  |  BE error conversion / bvar timing / limiter policy
  +------------------------|----------------|---+
                           |                |
                           v                v
                       AWS SDK          Azure SDK
                           ^                ^
                           |                |
  +------------------ Recycler -----------------+
  |                        |                |   |
  |              Recycler S3 client  Recycler Azure client
  |                        ^                ^   |
  |                        +--------+-------+   |
  |                                 |           |
  |                            S3Accessor       |
  |                                 ^           |
  |                                 |           |
  |                        Recycler call sites  |
  |                                             |
  |  Recycler credentials / lazy listing / recursive delete
  |  Recycler error model / bvar timing / limiter policy
  +---------------------------------------------+

After this PR, adapters retain deployment-specific policy, while all provider behavior is implemented once in common/client:

                                       AFTER

  +-------------------- BE --------------------+
  |                                            |
  |  BE call sites                             |
  |       |                                    |
  |       v                                    |
  |  ObjClientHolder / S3ClientFactory         |
  |       |                                    |
  |       +-- BE runtime hooks --------------------+
  |           - #65420 QPS/bytes limiter            |
  |           - non-cloud: all buckets               |
  |           - cloud: internal vaults only          |
  +--------------------------------------------------|--+
                                                     |
                                                     v
                  +--------- common/client -------------------------+
                  |                                                  |
                  |  ObjStorageClient + common request/response      |
                  |                  |                               |
                  |        +---------+----------+                    |
                  |        |                    |                    |
                  |        v                    v                    |
                  |  S3ObjStorageClient   AzureObjStorageClient      |
                  |        |                    |                    |
                  |  paginated iterator / recursive-delete engine   |
                  |  provider batch limits / error and HTTP metadata |
                  |  client_bvar::ScopedLatency / failure metrics    |
                  |        ^                    ^                    |
                  |        +---------+----------+                    |
                  |                  |                               |
                  |   AwsCredentialFactory / AzureAuthFactory        |
                  +------------------|-------------------------------+
                                     |                    |
                                     v                    v
                                  AWS SDK             Azure SDK
                                     ^                    ^
                                     |                    |
  +---------------- Recycler --------|--------------------|--+
  |                                  |                    |  |
  |  Recycler call sites -> S3Accessor / ListIterator adapter       |
  |                                  |                               |
  |       +-- Recycler runtime hooks +                               |
  |           - existing Recycler limiter                            |
  |           - rate-limit fault injection                           |
  |           - worker-pool recursive-delete executor                |
  +------------------------------------------------------------------+

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.

@hello-stephen

Copy link
Copy Markdown
Contributor

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

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?

@sollhui

sollhui commented Aug 1, 2026

Copy link
Copy Markdown
Contributor Author

run buildall

@sollhui

sollhui commented Aug 1, 2026

Copy link
Copy Markdown
Contributor Author

/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).
@sollhui
sollhui force-pushed the agent/unify-be-recycler-obj-client branch from f30abce to dc25d3e Compare August 1, 2026 09:44
@github-actions

github-actions Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Codex automated review failed and did not complete.

Error: Codex completed, but no new pull request review was submitted for the current head SHA.
Workflow run: https://github.com/apache/doris/actions/runs/30693205073

Please inspect the workflow logs and rerun the review after the underlying issue is resolved.

@@ -0,0 +1,103 @@
// Licensed to the Apache Software Foundation (ASF) under one

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

put client folder under common/cpp

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants