Repository navigation
Conversation
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
run buildall |
2 similar comments
|
run buildall |
|
run buildall |
TPC-H: Total hot run time: 28494 ms |
TPC-DS: Total hot run time: 151321 ms |
ClickBench: Total hot run time: 24.36 s |
|
Review of commit F-01 · Blocker: the new holder tests call an undefined helper At Please add the missing helper in the test's anonymous namespace, or access the typed S3 configuration directly. For example: const std::string& secret_key_of(const ObjClientConfig& config) {
return std::get<S3ClientConfig>(config.backend).auth.secret_key;
}This is a F-02 · Minor: narrow the Azure region-removal claim in the PR description Behavior-change item 4 suggests that a flat Azure configuration no longer needs a region. However, This preserves the existing property-path behavior, so it is a wording issue rather than a runtime regression. Please limit the description to the paths that implement the relaxation, for example:
Validation: source review covered all 58 changed files and converged after two rounds. No builds or tests were run locally; the compilation finding is based on symbol lookup and the CMake source-collection rules. |
|
Architecture follow-up for commit Main relationships after the refactor 1. Consolidate the common creation flow and keep providers focused on SDK construction. The S3 provider and Azure provider both follow essentially the same sequence: They also call the factory's rate-limit helper, creating a dependency that goes Consider having the factory own the common creation flow and cache, while each provider only builds its backend client. The existing cache key already includes the backend, so it can distinguish clients for different backends. This would reduce duplication and make the dependency direction more direct. The tradeoff is sharing one short cache-lock critical section across the two backends. If separate caches are preferable, extracting a small common creation helper would still remove the duplicated sequence. A complex provider inheritance hierarchy is unnecessary here. 2. Keep one implementation of typed configuration validation. The resolver's validate_flat_fields and the factory's S3 validation repeat checks for the endpoint, region, credential combinations, and access/secret keys. It would help to distinguish:
The second group should have one implementation, always called by the factory and reused by the resolver when early validation is needed. Adding an authentication mode would then be less likely to leave two sets of rules inconsistent. Preserve existing compatibility rules during this cleanup. For example, Azure properties still require a region today; reorganizing validation should not silently change that behavior. 3. Extend compile-time variant checking to configuration merging and authentication. The factory's backend dispatch already uses
Adding a third backend or an Azure OAuth alternative would not necessarily expose every missing case at compile time. Some omissions could instead become a runtime Consider explicit per-alternative visitor overloads for these paths too. Pure configuration merging could also live in the configuration layer, so the holder's refresh logic does not need to depend on the input resolver. Both changes would make extension points more explicit. 4. Make the scope of shared connection options match their actual effect. ObjStorageConnectionOptions appears to apply to every backend. However, Azure construction receives only This inherits the old behavior and is not a new regression. The new model nevertheless gives callers a broader impression of support than the implementation provides. A small first step would be to document that these options currently apply only to S3. A later change could move them into S3 configuration or implement explicit Azure equivalents. Adding Azure support should be treated as a behavior change with corresponding tests. 5. As a lower-priority cleanup, align the remaining S3ClientFactory name with its responsibilities. The retained S3ClientFactory no longer creates object-storage clients. Its main responsibilities are SDK lifetime, AWS helpers, and CA handling; Azure still obtains CA information through it. At minimum, a more accurate name would avoid suggesting that the code now has two client factories. If CA handling is later extracted into a backend-neutral utility, keep SDK lifetime ownership explicit and preserve the guarantee that clients are destroyed before SDK shutdown. I would retain the typed configuration, centralized input conversion, and independent holder snapshots. Prioritizing points 1-3 would further reduce duplicated rules and dependency cycles, while concentrating the changes required to add a backend and making omissions easier for the compiler to detect. Validation was source-based; no builds or tests were run for this architecture analysis. |
### What problem does this PR solve? Issue Number: None Related PR: apache#69605 Problem Summary: The holder tests in obj_client_factory_test call secret_key_of, but the helper was dropped together with an adjacent test when this PR was split from apache#69082, so the BE unit tests did not compile. The helper is restored. ### Release note None ### Check List (For Author) - Test: Unit Test (compiled and run in CI) - Behavior changed: No - Does this need documentation: No
|
run buildall |
TPC-H: Total hot run time: 28906 ms |
TPC-DS: Total hot run time: 151230 ms |
ClickBench: Total hot run time: 24.33 s |
…jClientFactory ### What problem does this PR solve? Issue Number: None Related PR: apache#69605 Problem Summary: apply_rate_limit_policy was declared in obj_client_factory.h and defined in obj_client_factory.cpp. ObjClientFactory holds the client providers, and both providers called back into the factory to wrap a new client with the BE rate limiter, so the factory and the providers depended on each other. The policy (BeObjStorageRateLimitPolicy and apply_rate_limit_policy) moves to io/obj_storage/obj_client_rate_limit.{h,cpp}, and the providers include that instead. The dependency is now one way: factory -> providers -> rate limit policy. No behavior change. ### Release note None ### Check List (For Author) - Test: No need to test (code moved without change; existing rate limit tests in s3_client_factory_test cover it, run in CI) - Behavior changed: No - Does this need documentation: No
|
run buildall |
…jClientFactory ### What problem does this PR solve? Issue Number: None Related PR: apache#69605 Problem Summary: Same change as in apache#69605. apply_rate_limit_policy lived in ObjClientFactory while both client providers, which the factory holds, called back into it, so the factory and the providers depended on each other. The policy moves to io/obj_storage/obj_client_rate_limit.{h,cpp}; the dependency is now factory -> providers -> rate limit policy. No behavior change. ### Release note None ### Check List (For Author) - Test: No need to test (code moved without change; run in CI) - Behavior changed: No - Does this need documentation: No
…layer ### What problem does this PR solve? Issue Number: None Related PR: apache#69605 Problem Summary: The typed configuration rules were split by where the old code lived rather than by which module owns them. Validation was implemented twice, as `validate_flat_fields` in the resolver and `validate_obj_client_backend` in the factory. The refresh merge `merge_update` lived in the resolver, so the holder depended on the input adapter, and it used `else` to mean Azure. Azure validation and construction read `AzureSharedKeyConfig` with `std::get`, so a new authentication alternative would fail at runtime with `bad_variant_access` instead of at compile time. - `validate_obj_client_config` and `merge_obj_client_config` are now part of the config layer (`obj_storage_config.{h,cpp}`) and are written as one overload per backend and per Azure authentication alternative, so a missing case fails to compile. The factory validates every request with it; the holder merges with it and no longer depends on the resolver. - The resolver keeps only the rules of FE properties that the typed configuration does not cover: a region for every provider, Azure included, and no provider-native credential for Azure. It then checks the converted configuration with the same `validate_obj_client_config`. - `AzureClientProvider` builds the credential options and the endpoint info through per-alternative overloads. - `ObjStorageConnectionOptions` documents that only S3 clients apply it today. Behavior is unchanged except that an invalid FE property map now fails at conversion instead of at the first client request, and when several fields are missing the reported one may differ. ### Release note None ### Check List (For Author) - Test: Unit Test (obj_client_factory_test covers typed Azure validation and the Azure provider-native credential rule on the property path; existing merge tests moved to merge_obj_client_config). Run in CI. - Behavior changed: No (see above) - Does this need documentation: No
|
/review |
|
Codex automated review failed and did not complete. Error: Selected model is at capacity. Please try a different model. Please inspect the workflow logs and rerun the review after the underlying issue is resolved. |
…rage-rename Bring in the latest apache#69605 (BE-1) and apply the renames of this PR to the code it added since: refresh_resource / refresh_vault, the invalidation tests, and the master changes it carries (S3FileReaderTest, the peer race skip counter, the split spill writers).
…mment ### What problem does this PR solve? Issue Number: None Related PR: apache#69605, apache#69082 Problem Summary: The bulk rename of S3ClientFactory to AwsSdkManager also rewrote a comment in obj_storage_config.cpp that names the function the config checks came from, so it read "the former AwsSdkManager::create". That function was S3ClientFactory::create; AwsSdkManager never had a create method. Restore the original name. A scan of comments that name a method of AwsSdkManager or the ObjStorage* classes found no other method that does not exist. ### Release note None ### Check List (For Author) - Test: No need to test (comment only) - Behavior changed: No - Does this need documentation: No
### What problem does this PR solve? Related PR: apache#69605 Problem Summary: The typed configuration omitted the bucket from the S3 client cache key. Buckets with otherwise identical settings would share a client and its connection pool. Carry the bucket from legacy inputs through cache lookup and reset. S3 requests still supply their target bucket individually. ### Release note None ### Check List (For Author) - Test: 69 ASAN BE unit tests passed across ObjClientFactoryTest, ObjStorageUriS3RulesTest and S3ClientFactoryTest. Header hygiene, formatting and clang-tidy passed. - Behavior changed: No (preserves the pre-refactor S3 cache behavior) - Does this need documentation: No
### What problem does this PR solve? Related PR: apache#69082, apache#69605 Problem Summary: Keep the integration branch aligned with the split BE refactor. Include the bucket supplied by the legacy configuration sources in the S3 client cache key and preserve it through refresh. This keeps separate connection pools for otherwise identical configurations that name different buckets. ### Release note None ### Check List (For Author) - Test: FE and ASAN BE builds passed. A local two-bucket S3 regression passed with cold and warm caches. BE logs confirmed two client builds, and SQL and an independent S3 client verified the exported data. Header hygiene, formatting and clang-tidy passed. - Behavior changed: No (preserves the pre-refactor S3 cache behavior) - Does this need documentation: No
### What problem does this PR solve? Related PR: apache#69605 Problem Summary: Public URL generation assumed an S3 client whenever an endpoint matched the OSS private endpoint suffix. A custom Azure endpoint with that suffix would fail before signing. Rewrite the endpoint in a copy of the configured backend so its own client signs the public URL. ### Release note None ### Check List (For Author) - Test: FE and ASAN BE builds passed. All 74 selected BE unit tests passed with Azure enabled, including public and private signing. Header hygiene, clang-format 16 and clang-tidy passed. - Behavior changed: No (restores pre-refactor public URL generation). - Does this need documentation: No
### What problem does this PR solve? Related PR: apache#69605 Problem Summary: When credentials were refreshed while a writer waited for its parts, CompleteMultipartUpload used the replacement client. Remember the holder state before waiting. After all upload callbacks finish, reject an invalidated holder and keep the original client when only authentication changed. Other configuration updates and resource backend rebinds still use the current client. ### Release note None ### Check List (For Author) - Test: ASAN BE and FE builds passed. The new regression failed on the previous close implementation and passed with this change. All 75 selected BE unit tests passed with Azure enabled. Header hygiene, clang-format 16 and clang-tidy passed. - Behavior changed: No (restores pre-refactor credential rotation). - Does this need documentation: No
### What problem does this PR solve? Related PR: apache#69605 Problem Summary: A holder refresh that changed a timeout or S3 addressing during multipart close routed CompleteMultipartUpload to the replacement client. Keep the client selected before waiting when only credentials, request or connection timeouts, or S3 addressing change. Check the live holder after all uploads finish and continue to honor vault invalidation and resource backend changes. ### Release note None ### Check List (For Author) - Test: Production FE and ASAN BE builds passed. The regression failed on the previous implementation for four refresh scenarios and passed with this change. Of 138 selected BE UTs, 104 passed and 34 external storage tests skipped. Formatting, header hygiene and clang-tidy passed. - Behavior changed: No (restores pre-refactor multipart close behavior for addressing and timeout updates). - Does this need documentation: No
### What problem does this PR solve? Related PR: apache#69082 Problem Summary: A holder refresh that changed a timeout or S3 addressing during multipart close routed CompleteMultipartUpload to the replacement client. Keep the client selected before waiting when only credentials, request or connection timeouts, or S3 addressing change. Check the live holder after all uploads finish and continue to honor vault invalidation and resource backend changes. ### Release note None ### Check List (For Author) - Test: Production FE and ASAN BE builds, formatting, header hygiene and clang-tidy passed. The equivalent writer regression passed on apache#69605. This branch's macOS BE UT build is blocked by the existing size_t / ordinal_t std::min mismatch in binary_column_extract_iterator_test.cpp. - Behavior changed: No (restores pre-refactor multipart close behavior for addressing and timeout updates). - Does this need documentation: No
### What problem does this PR solve? Related PR: apache#69605 Problem Summary: During a multipart close, the client selector compared individual configuration fields after waiting for the parts. A change of S3 vendor or Azure container switched the pending Complete to the refreshed client, even though the backend type stayed the same. Use the client captured before the wait for same-type refreshes, as on master. Recheck the live holder after draining callbacks so invalidation still blocks Complete, and use the current client after a cross-backend resource rebind. The test clients keep separate parts to check both selection and storage error propagation. ### Release note None ### Check List (For Author) - Test: FE and ASAN BE build, clang-format 16, clang-tidy and build hygiene. ASAN_UT: 104 selected tests passed and 34 external-storage tests skipped. The regression failed against the old selector for both an S3 vendor refresh and an Azure container change. - Behavior changed: Yes. Same-backend multipart completion preserves master's choice of client. - Does this need documentation: No
### What problem does this PR solve? Related PR: apache#69605 Problem Summary: With both AWS_ENDPOINT and AWS_REGION missing, flat FE property resolution reported the region first for S3 and Azure. Restore master's endpoint-first diagnostic at the properties entry point and cover both providers. Move the stateless key parser back into the source file's anonymous namespace, remove three unused headers and correct comments left behind by the holder migration. ### Release note When BE receives flat object storage properties without an endpoint or a region, it again reports the missing endpoint first. ### Check List (For Author) - Test: Built FE and ASAN BE with Azure enabled, and passed build hygiene, clang-format 16 and clang-tidy on the changed BE files. Under ASAN_UT the new test failed on the previous implementation for both S3 and Azure, then passed along with 240 other selected tests. 34 external-storage cases skipped with --enable_s3_client off. - Behavior changed: Yes. The diagnostic order for incomplete FE properties matches master. - Does this need documentation: No
### What problem does this PR solve? Related PR: apache#69605 Problem Summary: Migrating the BE storage binding from S3Conf dropped the sse_enabled value carried by ObjectStoreInfoPB. The previous BE included that value in vault creation and error diagnostics. Carry it on ObjStorageConfig, keep the original false default and print it in the storage and flat-source diagnostics. Client construction, caching and the upload path continue to use their existing inputs. ### Release note BE vault diagnostics retain the sse_enabled value from storage metadata. ### Check List (For Author) - Test: Built FE and the ASAN BE with Azure enabled. Build hygiene, clang-format 16 and clang-tidy passed. The new test failed on the previous code for both missing log fields and passed after the fix. 242 selected ASAN_UT cases passed, with 34 external-storage cases skipped. - Behavior changed: No relative to master. The refactored BE reports the vault SSE value again. - Does this need documentation: No
### What problem does this PR solve? Related PR: apache#69605 Problem Summary: The benchmark writer sets a 3000 ms S3 connection timeout. The reader at the merge base passed a property that the resolver ignored and inherited the SDK setting. Omit the reader override when converting the benchmark to typed client configuration so its read workload uses the same settings as before. ### Release note None ### Check List (For Author) - Test: Built FE and ASAN BE on macOS arm64 with Azure enabled and PCH disabled. Ran 72 object client factory, URI and S3 factory unit tests under ASAN. All passed. clang-format 16 and BE header hygiene passed. With the optional microbenchmark enabled, clang-tidy reports an existing RapidJSON overload ambiguity in the released_elements response on macOS. - Behavior changed: No relative to the merge base. - Does this need documentation: No
### What problem does this PR solve? Related PR: apache#69082, apache#69605 Problem Summary: The benchmark writer sets a 3000 ms S3 connection timeout. The reader at the merge base passed a property that the resolver ignored and inherited the SDK setting. Omit the reader override when converting the benchmark to typed client configuration so its read workload uses the same settings as before. ### Release note None ### Check List (For Author) - Test: clang-format 16, BE header hygiene and changed-file clang-tidy passed on this branch. The identical change on apache#69605 passed the default FE and ASAN BE build and 72 related BE tests. The optional tool on macOS still encounters the existing RapidJSON overload ambiguity when serializing released_elements. - Behavior changed: No relative to the merge base. - Does this need documentation: No
### What problem does this PR solve? Related PR: apache#69605 Problem Summary: Keep the behavior of the merge base when migrating object storage clients. Leave the file cache benchmark's read-side connection timeout property spelling and the vault sync warning's status argument as they were. Enable RFC 3986 encoding in the reader and writer constructors and remove the test for eager SDK encoding. ### Release note None ### Check List (For Author) - Test: Built FE and the Azure-enabled ASAN BE on macOS arm64 with PCH disabled. In the related BE suites, 241 tests passed and 34 external S3FileSystemTest cases skipped. The two Hive writer cases also passed after the cache-disabling fixture. clang-format 16, clang-tidy and BE build hygiene passed. - Behavior changed: No relative to the master merge base for these three paths. - Does this need documentation: No
### What problem does this PR solve? Related PR: apache#69605 Problem Summary: ObjClientHolder published a new state only when no other reset had published since the caller took its snapshot, and otherwise returned Aborted. Before the refactor the holder built the client outside the lock and the last reset to publish won, with both calls succeeding. The generation check did not add correctness: the merge keeps only the storage target, which no reset changes, so the published configuration is the same whichever state it was merged with, and the holder cannot tell which caller carries the newer configuration either way. Its only effect was a different arbitrary winner and a failed refresh for the other caller. Restore the previous behavior: _publish no longer takes the expected state, and it assigns the next generation under the lock. The check for an invalidated holder stays, so a client that was being built during invalidate() still cannot bring the holder back. A reset cannot overlap a rebind: resource updates run on a single worker thread, and vault syncs only reset or invalidate. ### Release note None ### Check List (For Author) - Test: Not compiled and not run; this machine has no BE build. The unit test for overlapping resets was rewritten to expect both resets to succeed and the last one to stay, and needs a CI run. Build hygiene passed. Formatting of the changed lines was checked with clang-format 20 only, because clang-format 16 is not installed here. - Behavior changed: No relative to master. Overlapping resets both succeed and the last one wins again. - Does this need documentation: No
|
run buildall |
…coding ### What problem does this PR solve? Related PR: apache#69605 Problem Summary: The previous commit "Preserve existing object storage behavior" returned three places to their state on master in order to keep this change a pure refactor. All three had been changed at a reviewer's request, so this commit restores them. - A failed vault sync logs the status of the failing create or refresh again. On master the warning received the status of get_storage_vault_info, which is OK at that point, so a create failure, including the new configuration errors of this change, was logged with err OK. - The AWS SDK enables RFC 3986 path encoding through SDKOptions before InitAPI again, the reader and writer constructors no longer set the process-wide flag, and the test that asserts the flag after initialization is back. - The file cache microbench reader passes AWS_CONNECTION_TIMEOUT_MS again. The key on master, AWS_CONNECT_TIMEOUT_MS, is one that nothing reads, so the read client ran without the 3000 ms connection timeout of the write client. The six files are identical to commit d3b60b3 again. ### Release note None ### Check List (For Author) - Test: Not compiled and not run in this commit; this machine has no BE build. The six files are byte-identical to d3b60b3, which was built and unit tested earlier. Build hygiene passed. - Behavior changed: Yes. The vault sync warning shows the failing status, and strict RFC 3986 key encoding is on from AWS SDK initialization instead of from the first reader or writer. - Does this need documentation: No
|
run buildall |
|
Could we rename I'm happy for the rename to be a follow-up if it would add too much churn here. |
…obench key as on master ### What problem does this PR solve? Issue Number: None Related PR: apache#69605 Problem Summary: Review asked to keep this refactor free of behavior changes and bug fixes. Three places still differed from master and are returned to it: - A failed vault create or refresh in `CloudStorageEngine::sync_storage_vault` logs the status of `get_storage_vault_info` again, as on master. - The S3 reader and writer constructors set `Aws::Http::SetCompliantRfc3986Encoding(true)` again, instead of `SDKOptions` before `InitAPI`. The test that asserted the flag after SDK initialization is removed with it. - The file cache microbench reader passes `AWS_CONNECT_TIMEOUT_MS` again, the key master uses. The fixes themselves will be sent separately. The six files are identical to `e0caadd6ac8`. ### Release note None ### Check List (For Author) - Test: No need to test; the six files are identical to e0caadd. Build hygiene and clang-format 16 passed. Not compiled locally; CI compiles and runs the unit tests. - Behavior changed: No (returns these three places to master behavior) - Does this need documentation: No
|
run buildall |
…zure parts out of it ### What problem does this PR solve? Issue Number: None Related PR: apache#69605 Problem Summary: After client construction moved to `ObjClientFactory`, `S3ClientFactory` no longer built clients, yet its name still said so, and it still carried two Azure parts: its constructor set up the Azure SDK log, and `AzureClientProvider` called `S3ClientFactory::instance().ca_cert_file_path()` for its CA bundle. Review pointed out both. - `S3ClientFactory` becomes `AwsSdkManager`. It keeps only the AWS SDK lifetime (`InitAPI` / `ShutdownAPI`), `getClientConfiguration` and `create_aws_credentials_provider`, which the S3 provider and Kinesis use. gtest suite and file names stay. - The Azure SDK log setup moves unchanged into `init_azure_sdk_logging()` in `azure_client_provider`. `ObjClientFactory` calls it on construction, right after initializing the AWS SDK, and BE startup now constructs `ObjClientFactory` instead of `S3ClientFactory`, so both SDKs are still set up at startup in the same thread, and the AWS SDK is still shut down after the providers release their clients. - The CA bundle cache moves unchanged into `io/obj_storage/obj_client_ca.{h,cpp}` (`obj_client_ca_cert_file_path()`): the first valid path of `ca_cert_file_paths`, cached, looked up again while empty. Both providers and the AWS credentials provider use it, so the Azure provider no longer depends on an AWS class. `AwsSdkManager` still resolves it once on construction. The CA test uses a test hook instead of the removed private members. ### Release note None ### Check List (For Author) - Test: Unit Test (`S3ClientFactoryTest.RefreshCaCertForCredentialsProvider` now goes through the new hook). Not compiled locally; CI compiles and runs the unit tests. Build hygiene and clang-format 16 passed. - Behavior changed: No - Does this need documentation: No
|
run buildall |
### What problem does this PR solve? Related PR: apache#69605, apache#69082 Problem Summary: Splitting the S3 and Azure SDK configurations changed how resource and vault updates retained the original target, how legacy Azure metadata was validated and which client completed an in-flight multipart upload. Retain the fields carried by flat Azure inputs for validation and cache identity. Merge the effective target before building the replacement client, publish successful updates on the existing holder and finish BE multipart uploads with the client acquired before waiting for parts. ### Release note None ### Check List (For Author) - Test: Local macOS arm64 FE and ASAN BE build, Azure enabled, PCH disabled. Object storage, URI, multipart, CSV and embed UT passed. The file-system group passed 60 tests and skipped 34 gated external-storage cases. The standalone Hive group reproduced its file-cache setup dependency. Its two cases passed after the cache-disabled fixture. These checks preceded the independent SDK plumbing commit. The PR checklist tracks head runs. - Behavior changed: No. Existing flat inputs follow the master merge, validation, client identity and multipart completion rules. - Does this need documentation: No
What problem does this PR solve?
Issue Number: None
Related PR: #69082
Problem Summary: BE accepted FE properties, cold-storage resources and storage vaults through one flat
S3Conf/S3ClientConf. Azure reused the access key, secret key and bucket for its account, key and container. The same structure also supplied cache identity, validation and refresh history. Splitting the SDK inputs by backend lets the factory construct each client from its own type while preserving the rules that existing inputs use.This is the first step of splitting #69082 (BE object storage refactor and native Azure SAS / OAuth2 access). It restructures BE object storage and adapts the shared C++ authentication factory and its Recycler caller. The FE and Thrift interfaces stay the same. Azure continues to use SharedKey.
io/obj_storage/obj_storage_config.{h,cpp}).io::ObjClientConfigholds the shared connection options, the client usage (internal storage / external access) and exactly one backend,std::variant<S3ClientConfig, AzureClientConfig>. The Azure authentication is theAzureAuthConfigshared with Recycler (see below).io::ObjStorageConfigreplacesS3Confand carries the bucket, prefix, client configuration and the vault metadata flagsse_enabled. BE reports the flag in storage diagnostics, as on master.io::ObjClientConfigreplacesS3ClientConf. The rules of the typed configuration live in this layer, written per backend and per authentication throughstd::visit, so a missing branch fails to compile:validate_obj_client_config(whether a configuration is valid) andmerge_obj_client_config(how a refreshed resource or vault configuration merges into the current one, with the same field rules as the flat merge).ObjStorageConfigResolver::from_properties,from_storage_paramandfrom_object_store_infoconvert the three flat sources. FE properties validate during conversion. Resource and vault metadata retain their raw fields until client creation, so an update is validated against the effective endpoint and region after merging. Azure retains the legacy fields used for validation, refresh and cache identity alongside its SharedKey SDK configuration. AI embed, Kinesis and the file cache microbenchmark writer build typed configurations directly.ObjClientFactoryvalidates every request withvalidate_obj_client_config(the checksS3ClientFactory::createapplied), owns the test hookset_client_creator_for_test, and dispatches throughstd::visit.S3ClientProviderandAzureClientProviderlook up and publish entries under the cache lock. They construct the SDK client outside the lock on a miss. The BE rate limit policy (obj_client_rate_limit) and the CA bundle cache (obj_client_ca) are their own modules shared by both providers.S3ClientFactorybecomesAwsSdkManagerand keeps only the AWS SDK lifetime and the AWS helpers (client configuration, credentials provider), which Kinesis also uses. The Azure SDK log setup that its constructor carried moves unchanged toinit_azure_sdk_logging()next toAzureClientProvider;ObjClientFactoryruns both SDK setups on construction, and BE startup constructs it in place ofS3ClientFactory.AzureAuthFactory, and both passed it the flatAzureCredentialOptions(a type enum plus fields). The model now lives once in common, next to the GCP credential, undercommon/cpp/obj-client/auth/azure/:azure_auth.h(SDK-free, always built) definesAzureSharedKeyConfigand theAzureAuthConfigvariant, with the rules every method shares (validate_azure_authfor required fields,hash_combine_azure_auth,azure_auth_type_namewithout secrets).AzureAuthFactorytakesAzureAuthConfigdirectly, with onestd::visitbranch per method, andAzureCredentialType/AzureCredentialOptionsare removed; transport, CA, retries and policies still come from each process throughBlobClientOptions. The storage account name stays outside the variant, because it names the storage target. BE’sAzureClientConfigholds the common auth config. Flat inputs retain the legacy validation order and role ARN exception. Recycler builds anAzureSharedKeyConfigfrom its storage metadata and passes it to the same factory; it does not call the check, becauseS3Conf::from_obj_store_infoloads the ak/sk pair complete or not at all (the meta service requires the pair when an Azure entry is created, but not on every later update), so Recycler builds the client exactly as before. A new method (SAS, OAuth2, Managed Identity) is then added once in common, and each entry only adapts its input or rejects the method.ObjClientHoldermoves toio/obj_storage/obj_client_holder.{h,cpp}. Its immutable snapshot contains a configuration, client and generation. A successful reset publishes all three together. If building the client fails, the previous snapshot remains available.ObjStorageUri::parse_s3replacesS3URIwith the same rules.FileFactoryresolves FILE_S3 locations through one function for the reader,create_fsandcreate_file_writer.S3AuthConfig::credentialcarries the provider-nativeCredentialConfig, the resolver parses and converts it for all three sources,ObjClientFactoryvalidates it on every request, andS3ClientProviderbuilds the client throughmake_s3_clientwith the GCS V4 signed URL generator. A reset takes the vendor and credential from the update, as the flat merge does.Kept as on master
These were checked against the merge base one by one, because the refactor touches them:
-internal.aliyuncs.comuses the configured backend with-internalremoved from a copy of the endpoint. Presigning testsse_enabledof a vault is carried and printed in diagnostics; nothing else reads it.Scope and SDK lifetime
Vault ID allocation and handling a deleted vault's existing references are left to a separate change. This refactor keeps the legacy refresh semantics.
At process exit the provider caches now release their clients before
AwsSdkManagercallsAws::ShutdownAPI.Release note
None.
Check List (For Author)
Test
11b7821a5don macOS arm64 with./build.sh --be --fe, Azure enabled and PCH disabled.build-support/check-build-hygiene.sh.--file-cache-microbenchtarget fails on macOS arm64 at the existing RapidJSONAddMember("released_elements", released, allocator)call. RapidJSON cannot select a numeric overload forsize_ton this platform. The ordinary BE build excludes this tool. Upstream callBehavior changed: Existing storage inputs retain their request, refresh and validation behavior. The SDK shutdown order is described above.
Does this need documentation: No