Skip to content

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

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

[refactor](storage) Unify BE and Recycler object clients#66350
sollhui wants to merge 7 commits 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/cpp/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/cpp/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.

Before the refactor, BE and Recycler reached the same provider SDKs through parallel implementations, with cross-cutting behavior duplicated on both sides:

                                     BEFORE

  +---------------------- BE ----------------------+     +------------------- Recycler -------------------+
  |                                                |     |                                                |
  |  BE call sites -> S3ClientFactory              |     |  Recycler call sites -> S3Accessor            |
  |                       |                        |     |                       |                        |
  |                       v                        |     |                       v                        |
  |  RateLimitedObjStorageClient                   |     |  Recycler limiter / fault injection           |
  |                       |                        |     |                       |                        |
  |                       v                        |     |                       v                        |
  |  Separate BE S3 / Azure clients                |     |  Separate Recycler S3 / Azure clients          |
  |  credentials / eager listing / recursive delete|     |  credentials / lazy listing / recursive delete |
  |  error conversion / metrics                    |     |  error conversion / metrics                    |
  +------------------------+-----------------------+     +------------------------+-----------------------+
                           |                                                        |
                           +---------------------------+----------------------------+
                                                       |
                                                       v
                                              AWS SDK / Azure SDK

After the refactor, BE and Recycler remain separate policy owners at the top, while both depend on one shared provider layer centered below them:

                                      AFTER

  +---------------------- BE ----------------------+     +------------------- Recycler -------------------+
  |                                                |     |                                                |
  |  BE call sites                                 |     |  Recycler call sites                           |
  |       |                                        |     |       |                                        |
  |       v                                        |     |       v                                        |
  |  ObjClientHolder / S3ClientFactory             |     |  S3Accessor / ListIterator adapter             |
  |       |                                        |     |       |                                        |
  |       v                                        |     |       v                                        |
  |  BE runtime hooks                              |     |  Recycler runtime hooks                        |
  |  - QPS/bytes limiter                           |     |  - existing limiter                           |
  |  - bucket-selection policy                     |     |  - fault injection                            |
  |                                                |     |  - recursive-delete executor                  |
  +------------------------+-----------------------+     +------------------------+-----------------------+
                           |                                                        |
                           +---------------------------+----------------------------+
                                                       |
                                                       v
                      +---------------- common/cpp/client ----------------+
                      |                                                   |
                      |  ObjStorageClient + common request/response       |
                      |                         |                         |
                      |              +----------+----------+              |
                      |              |                     |              |
                      |              v                     v              |
                      |     S3ObjStorageClient    AzureObjStorageClient   |
                      |                                                   |
                      |  paginated iterator / recursive-delete engine    |
                      |  provider batch limits / error and HTTP metadata  |
                      |  latency and failure metrics                      |
                      |              |                     |              |
                      |              v                     v              |
                      |           AWS SDK              Azure SDK          |
                      +---------------------------------------------------+

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?

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/cpp/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 it remains distinct from a provider HTTP 429.
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 BE admitted client operations through its limiter, with different bucket-selection policy in cloud and non-cloud modes Recycler invoked its own limiter and 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 and Recycler inject their existing limiter and environment-specific policy independently.
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.

@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.

Comment thread common/cpp/client/obj_storage_client.cpp
@sollhui

sollhui commented Aug 3, 2026

Copy link
Copy Markdown
Contributor Author

run buildall

@sollhui

sollhui commented Aug 3, 2026

Copy link
Copy Markdown
Contributor Author

/review

@sollhui

sollhui commented Aug 3, 2026

Copy link
Copy Markdown
Contributor Author

run buildall

@sollhui

sollhui commented Aug 3, 2026

Copy link
Copy Markdown
Contributor Author

/review

@github-actions

github-actions Bot commented Aug 3, 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/30794530392

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

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