Skip to content

[refactor](be) Build object storage clients from typed, per-backend configuration - #69605

Open
xylaaaaa wants to merge 33 commits into
apache:masterfrom
xylaaaaa:refactor/be-obj-client-config
Open

xylaaaaa wants to merge 33 commits into
apache:masterfrom
xylaaaaa:refactor/be-obj-client-config

Conversation

@xylaaaaa

@xylaaaaa xylaaaaa commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

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.

  • Typed configuration (io/obj_storage/obj_storage_config.{h,cpp}). io::ObjClientConfig holds the shared connection options, the client usage (internal storage / external access) and exactly one backend, std::variant<S3ClientConfig, AzureClientConfig>. The Azure authentication is the AzureAuthConfig shared with Recycler (see below). io::ObjStorageConfig replaces S3Conf and carries the bucket, prefix, client configuration and the vault metadata flag sse_enabled. BE reports the flag in storage diagnostics, as on master. io::ObjClientConfig replaces S3ClientConf. The rules of the typed configuration live in this layer, written per backend and per authentication through std::visit, so a missing branch fails to compile: validate_obj_client_config (whether a configuration is valid) and merge_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_param and from_object_store_info convert 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.
  • Client construction by backend. ObjClientFactory validates every request with validate_obj_client_config (the checks S3ClientFactory::create applied), owns the test hook set_client_creator_for_test, and dispatches through std::visit. S3ClientProvider and AzureClientProvider look 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. S3ClientFactory becomes AwsSdkManager and 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 to init_azure_sdk_logging() next to AzureClientProvider; ObjClientFactory runs both SDK setups on construction, and BE startup constructs it in place of S3ClientFactory.
  • Azure authentication shared with Recycler. BE and Recycler already built Azure clients through the common AzureAuthFactory, and both passed it the flat AzureCredentialOptions (a type enum plus fields). The model now lives once in common, next to the GCP credential, under common/cpp/obj-client/auth/azure/: azure_auth.h (SDK-free, always built) defines AzureSharedKeyConfig and the AzureAuthConfig variant, with the rules every method shares (validate_azure_auth for required fields, hash_combine_azure_auth, azure_auth_type_name without secrets). AzureAuthFactory takes AzureAuthConfig directly, with one std::visit branch per method, and AzureCredentialType / AzureCredentialOptions are removed; transport, CA, retries and policies still come from each process through BlobClientOptions. The storage account name stays outside the variant, because it names the storage target. BE’s AzureClientConfig holds the common auth config. Flat inputs retain the legacy validation order and role ARN exception. Recycler builds an AzureSharedKeyConfig from its storage metadata and passes it to the same factory; it does not call the check, because S3Conf::from_obj_store_info loads 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.
  • ObjClientHolder moves to io/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.
  • Locations. ObjStorageUri::parse_s3 replaces S3URI with the same rules. FileFactory resolves FILE_S3 locations through one function for the reader, create_fs and create_file_writer.
  • GCP IAM credentials ([feature](storage) Support native GCP IAM authentication #68540) move with the S3 fields: S3AuthConfig::credential carries the provider-native CredentialConfig, the resolver parses and converts it for all three sources, ObjClientFactory validates it on every request, and S3ClientProvider builds the client through make_s3_client with the GCS V4 signed URL generator. A reset takes the vendor and credential from the update, as the flat merge does.
  • The reader no longer uses AWS types for HTTP 429.

Kept as on master

These were checked against the merge base one by one, because the refactor touches them:

  • The S3 cache key includes the bucket. Identical settings for different buckets produce separate clients and connection pools. Two prefixes in the same bucket can reuse a client. Legacy Azure cache keys retain all flat identity fields, including the container. S3 cache test · Legacy cache identity test
  • Resource and vault refreshes reuse the existing file system and holder, including across S3/Azure switches. The merged client keeps the original endpoint, region and endpoint-override flag. Credentials, client bucket/container, vendor and connection settings follow the update. The file system keeps its original bucket and prefix. A failed build leaves the previous client in place. Among successful concurrent resets, the last publication wins. Cross-backend refresh test
  • A BE writer acquires its multipart completion client before waiting for pending uploads and uses it for Complete, even if the holder switches backend during the wait. Multipart refresh test
  • A public presigned URL for an endpoint ending in -internal.aliyuncs.com uses the configured backend with -internal removed from a copy of the endpoint. Presigning test
  • Flat configurations check the effective endpoint, region and native credential before the key-pair rule. The legacy role ARN exemption and the optional vault connectivity probe are preserved for Azure metadata. A GCP credential paired with Azure is rejected. Metadata validation test
  • sse_enabled of a vault is carried and printed in diagnostics; nothing else reads it.
  • The S3 reader and writer constructors set the AWS SDK RFC 3986 encoding flag, a failed vault create or refresh is logged with the same status, and the file cache microbench passes the same connection keys, as on master. Fixes for these three are kept for a separate PR.
  • The rate limit policy, the SDK client construction options for equivalent backend configurations, the location parsing rules and the Recycler's Azure client are unchanged.

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 AwsSdkManager calls Aws::ShutdownAPI.

Release note

None.

Check List (For Author)

  • Test

    • Built FE and the ASAN BE locally at 11b7821a5d on macOS arm64 with ./build.sh --be --fe, Azure enabled and PCH disabled.
    • The factory, URI and multipart-refresh group passed all 68 tests. The file system, reader, writer, spill and merge group passed 60 and skipped 34 gated external-storage tests, including six Azure cases. Both processes exited 0.
    • CSV and EMBED passed all 95 cases. With default file-cache settings the combined consumer filter aborts in the first Hive reporting test while constructing a writer (ASAN read at 0x48 in the file-cache factory). In a separate process the cache-disabled fixture and both Hive reporting tests passed, 3/3.
    • Ran clang-format 16, clang-tidy on the changed lines, and build-support/check-build-hygiene.sh.
    • In an earlier local run, the standalone --file-cache-microbench target fails on macOS arm64 at the existing RapidJSON AddMember("released_elements", released, allocator) call. RapidJSON cannot select a numeric overload for size_t on this platform. The ordinary BE build excludes this tool. Upstream call
  • Behavior changed: Existing storage inputs retain their request, refresh and validation behavior. The SDK shutdown order is described above.

  • Does this need documentation: No

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

@xylaaaaa

Copy link
Copy Markdown
Contributor Author

run buildall

2 similar comments
@shuke987

Copy link
Copy Markdown
Contributor

run buildall

@xylaaaaa

Copy link
Copy Markdown
Contributor Author

run buildall

@xylaaaaa
xylaaaaa marked this pull request as ready for review October 26, 2026 07:11
Copilot AI balanced review requested due to automatic review settings October 26, 2026 07:11

Copilot AI left a comment

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.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

@hello-stephen

Copy link
Copy Markdown
Contributor
TPC-H: Total hot run time: 28494 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit 719de6ac81f0abe4889f8b43d8752edc4f927ebd, data reload: false

------ Round 1 ----------------------------------
============================================
q1	17576	5093	5284	5093
q2	2141	320	269	269
q3	10196	1396	818	818
q4	4688	465	339	339
q5	7449	804	530	530
q6	189	207	152	152
q7	767	802	610	610
q8	9252	1322	1130	1130
q9	5549	4188	4188	4188
q10	6841	1360	1013	1013
q11	427	244	229	229
q12	634	407	283	283
q13	18098	3669	3006	3006
q14	277	282	239	239
q15	q16	736	706	657	657
q17	1217	750	583	583
q18	7013	6215	6119	6119
q19	1164	969	636	636
q20	373	335	222	222
q21	5253	2123	2316	2123
q22	357	297	255	255
Total cold run time: 100197 ms
Total hot run time: 28494 ms

----- Round 2, with runtime_filter_mode=off -----
============================================
q1	5614	5617	5600	5600
q2	697	597	537	537
q3	5227	5417	4885	4885
q4	1981	2135	1542	1542
q5	5303	5127	5711	5127
q6	257	199	153	153
q7	2330	2109	1921	1921
q8	3102	2901	2609	2609
q9	8064	7953	8009	7953
q10	3734	3676	3251	3251
q11	612	448	405	405
q12	697	724	524	524
q13	3553	3907	3233	3233
q14	317	309	294	294
q15	q16	689	708	632	632
q17	8214	7492	7279	7279
q18	12942	12185	12893	12185
q19	900	774	782	774
q20	2134	2142	1974	1974
q21	5525	4747	4639	4639
q22	477	467	438	438
Total cold run time: 72369 ms
Total hot run time: 65955 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
TPC-DS: Total hot run time: 151321 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpcds-tools
TPC-DS sf100 test result on commit 719de6ac81f0abe4889f8b43d8752edc4f927ebd, data reload: false

query5	4311	607	455	455
query6	436	211	193	193
query7	4800	440	241	241
query8	321	165	151	151
query9	8796	3113	3132	3113
query10	487	303	262	262
query11	5987	3856	3570	3570
query12	144	91	86	86
query13	1266	459	351	351
query14	6129	3947	3646	3646
query14_1	3433	3394	3450	3394
query15	208	208	184	184
query16	957	441	411	411
query17	855	666	538	538
query18	2416	421	308	308
query19	195	166	138	138
query20	86	85	85	85
query21	215	128	107	107
query22	13010	12950	12828	12828
query23	13456	12767	12377	12377
query23_1	12503	12571	12558	12558
query24	7377	838	476	476
query24_1	478	482	490	482
query25	571	393	355	355
query26	1261	257	137	137
query27	2770	430	273	273
query28	4613	1943	1940	1940
query29	1524	577	422	422
query30	299	216	186	186
query31	1004	908	822	822
query32	143	97	90	90
query33	516	282	235	235
query34	906	835	486	486
query35	726	744	634	634
query36	819	793	754	754
query37	128	98	81	81
query38	1796	1718	1649	1649
query39	801	803	747	747
query39_1	727	728	738	728
query40	212	113	91	91
query41	66	65	65	65
query42	86	83	84	83
query43	341	354	307	307
query44	1328	693	697	693
query45	173	185	175	175
query46	836	955	559	559
query47	2707	3025	2813	2813
query48	291	305	211	211
query49	595	394	290	290
query50	693	276	212	212
query51	10712	10699	10579	10579
query52	81	82	70	70
query53	190	210	154	154
query54	258	190	178	178
query55	70	65	70	65
query56	250	236	212	212
query57	1657	1607	1553	1553
query58	280	274	259	259
query59	2158	2157	1999	1999
query60	297	225	222	222
query61	172	146	147	146
query62	383	327	282	282
query63	185	155	160	155
query64	2720	934	741	741
query65	3324	3253	3315	3253
query66	1768	409	301	301
query67	20213	20119	20098	20098
query68	3005	1006	605	605
query69	391	285	241	241
query70	834	772	768	768
query71	293	239	238	238
query72	2686	2589	2433	2433
query73	518	522	305	305
query74	4645	4490	4351	4351
query75	2341	2289	1912	1912
query76	2336	1003	601	601
query77	352	375	269	269
query78	9732	9687	9220	9220
query79	982	861	530	530
query80	562	452	362	362
query81	536	324	274	274
query82	841	137	109	109
query83	363	194	182	182
query84	311	123	109	109
query85	822	422	373	373
query86	323	236	232	232
query87	1997	1934	1808	1808
query88	3655	2665	2658	2658
query89	329	281	235	235
query90	1890	168	174	168
query91	164	155	127	127
query92	99	85	82	82
query93	999	1011	599	599
query94	445	297	269	269
query95	582	331	314	314
query96	661	480	234	234
query97	2435	2426	2310	2310
query98	162	171	149	149
query99	765	766	638	638
Total cold run time: 234342 ms
Total hot run time: 151321 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
ClickBench: Total hot run time: 24.36 s
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit 719de6ac81f0abe4889f8b43d8752edc4f927ebd, data reload: false

query1	0.01	0.00	0.01
query2	0.09	0.05	0.05
query3	0.26	0.09	0.09
query4	1.61	0.12	0.08
query5	0.23	0.24	0.24
query6	1.18	0.68	0.68
query7	0.04	0.00	0.00
query8	0.06	0.04	0.04
query9	0.39	0.35	0.35
query10	0.61	0.60	0.62
query11	0.19	0.14	0.14
query12	0.18	0.15	0.15
query13	0.51	0.49	0.50
query14	1.04	1.01	1.01
query15	0.64	0.61	0.60
query16	0.33	0.31	0.34
query17	1.13	1.16	1.12
query18	0.21	0.20	0.19
query19	2.12	2.02	1.94
query20	0.01	0.01	0.02
query21	15.49	0.21	0.14
query22	4.81	0.06	0.05
query23	16.26	0.32	0.12
query24	2.91	0.58	0.40
query25	0.10	0.06	0.05
query26	0.86	0.22	0.15
query27	0.06	0.05	0.03
query28	2.75	0.75	0.36
query29	12.50	4.13	3.21
query30	0.29	0.13	0.10
query31	2.77	0.56	0.33
query32	3.22	0.60	0.51
query33	3.19	3.33	3.32
query34	15.43	3.94	3.38
query35	3.35	3.34	3.36
query36	0.55	0.45	0.42
query37	0.10	0.07	0.06
query38	0.05	0.04	0.04
query39	0.03	0.03	0.03
query40	0.18	0.15	0.14
query41	0.09	0.03	0.02
query42	0.03	0.04	0.04
query43	0.04	0.03	0.04
Total cold run time: 95.9 s
Total hot run time: 24.36 s

@morningman

Copy link
Copy Markdown
Contributor

Review of commit 719de6ac81f0abe4889f8b43d8752edc4f927ebd: REQUEST_CHANGES (1 Blocker, 1 Minor).

F-01 · Blocker: the new holder tests call an undefined helper

At obj_client_factory_test.cpp:281-287, and again at line 314, the tests call secret_key_of(...), but the helper has no declaration or definition. Repository symbol search finds only these three calls. The recursive source collection in be/test/CMakeLists.txt includes this file in the normal BE UT target, so this causes a compilation failure before any test can run.

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 functional-bug regression: the merge base contains neither the new test file nor the unresolved calls. Please build and run the affected BE unit tests after fixing it.

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, ObjStorageConfigResolver::from_properties still rejects an empty region before selecting the Azure backend. The new test at lines 386-388 explicitly expects an Azure property map without AWS_REGION to fail.

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:

The typed Azure client factory no longer requires a region. Azure configurations converted from Thrift/PB metadata inherit this relaxation; property conversion retains its existing AWS_REGION requirement.

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.

@morningman

Copy link
Copy Markdown
Contributor

Architecture follow-up for commit 719de6ac81f0abe4889f8b43d8752edc4f927ebd. The typed configuration, centralized input conversion, and holder snapshots improve the structure. The following suggestions focus on making the result clearer, simpler, and easier to extend.

Main relationships after the refactor

properties / Thrift / PB
          |
       Resolver
          |
   ObjStorageConfig
     +-- bucket / prefix: storage target
     +-- ObjClientConfig: configuration needed to create a client
                |
             Factory
                |
        S3 / Azure Provider
                |
           Cache / SDK
                |
        ObjStorageClient

Configuration refresh -> merge update -> Holder publishes configuration and client together

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:

cache lookup -> build -> rate-limit wrapping -> cache publication

They also call the factory's rate-limit helper, creating a dependency that goes Factory -> Provider -> Factory.

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:

  • Input handling: parsing strings, extracting URI/bucket information, and preserving legacy properties requirements.
  • Configuration semantics: whether a typed S3ClientConfig or AzureClientConfig is valid.

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 std::visit, which is useful. Some other paths still assume there are only two alternatives:

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 std::bad_variant_access.

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 AzureClientConfig: the three connection options currently have no effect there, while still participating in cache identity.

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.

xylaaaaa added a commit to xylaaaaa/doris that referenced this pull request Oct 26, 2026
### 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
@xylaaaaa

Copy link
Copy Markdown
Contributor Author

run buildall

@hello-stephen

Copy link
Copy Markdown
Contributor
TPC-H: Total hot run time: 28906 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit 7d30bee1d9d149b76841f08d635320254e866bee, data reload: false

------ Round 1 ----------------------------------
============================================
q1	17600	5229	5064	5064
q2	2118	349	252	252
q3	10214	1409	845	845
q4	4678	468	342	342
q5	7437	808	539	539
q6	182	187	156	156
q7	753	814	612	612
q8	9262	1350	1152	1152
q9	5553	4211	4196	4196
q10	6826	1293	1024	1024
q11	429	259	234	234
q12	628	395	289	289
q13	18127	3693	3044	3044
q14	274	273	253	253
q15	q16	742	719	680	680
q17	1199	744	574	574
q18	7039	6162	6109	6109
q19	1257	966	603	603
q20	388	337	227	227
q21	6063	2425	2613	2425
q22	389	339	286	286
Total cold run time: 101158 ms
Total hot run time: 28906 ms

----- Round 2, with runtime_filter_mode=off -----
============================================
q1	5926	5926	6202	5926
q2	717	560	591	560
q3	5592	5747	5336	5336
q4	2025	2144	1555	1555
q5	5838	5770	5667	5667
q6	237	183	143	143
q7	2285	2036	1863	1863
q8	2976	2610	2645	2610
q9	8000	7617	7465	7465
q10	3604	3584	3100	3100
q11	579	419	382	382
q12	671	656	502	502
q13	3313	3611	3026	3026
q14	284	286	261	261
q15	q16	657	672	598	598
q17	7835	7005	6886	6886
q18	12985	12206	12923	12206
q19	904	782	784	782
q20	2158	2138	1908	1908
q21	5733	4669	4840	4669
q22	514	443	413	413
Total cold run time: 72833 ms
Total hot run time: 65858 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
TPC-DS: Total hot run time: 151230 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpcds-tools
TPC-DS sf100 test result on commit 7d30bee1d9d149b76841f08d635320254e866bee, data reload: false

query5	4335	619	476	476
query6	429	213	186	186
query7	4809	487	231	231
query8	326	170	155	155
query9	8777	3067	3022	3022
query10	496	311	258	258
query11	6049	3871	3586	3586
query12	159	91	87	87
query13	1273	456	320	320
query14	6127	3929	3622	3622
query14_1	3437	3456	3416	3416
query15	209	203	182	182
query16	946	449	356	356
query17	850	685	537	537
query18	2420	434	313	313
query19	208	177	145	145
query20	85	94	84	84
query21	213	137	111	111
query22	12950	13414	12828	12828
query23	13509	12856	12514	12514
query23_1	12496	12541	12615	12541
query24	7397	867	474	474
query24_1	487	492	488	488
query25	568	412	342	342
query26	1256	270	139	139
query27	2759	462	269	269
query28	4613	1948	1921	1921
query29	1533	624	427	427
query30	299	222	181	181
query31	1013	936	814	814
query32	137	95	91	91
query33	512	301	237	237
query34	907	908	494	494
query35	754	769	652	652
query36	786	793	734	734
query37	128	97	83	83
query38	1781	1747	1647	1647
query39	786	801	751	751
query39_1	723	728	750	728
query40	238	130	97	97
query41	69	65	64	64
query42	86	89	82	82
query43	353	376	313	313
query44	1383	685	704	685
query45	193	177	164	164
query46	917	1032	580	580
query47	2733	2905	2906	2905
query48	308	321	219	219
query49	573	418	296	296
query50	728	286	212	212
query51	10452	10560	10492	10492
query52	88	87	69	69
query53	193	221	150	150
query54	253	210	173	173
query55	80	69	64	64
query56	228	209	214	209
query57	1639	1613	1572	1572
query58	292	259	264	259
query59	2135	2132	1937	1937
query60	290	247	227	227
query61	147	146	147	146
query62	394	337	286	286
query63	201	170	157	157
query64	2726	960	775	775
query65	3327	3250	3254	3250
query66	1796	448	308	308
query67	20129	19990	20010	19990
query68	3224	1032	620	620
query69	394	300	253	253
query70	875	809	757	757
query71	296	241	201	201
query72	2762	2721	2460	2460
query73	553	564	298	298
query74	4681	4543	4325	4325
query75	2483	2361	1912	1912
query76	2412	1133	614	614
query77	358	407	288	288
query78	9752	9641	9199	9199
query79	1002	913	509	509
query80	563	497	435	435
query81	529	327	268	268
query82	574	136	111	111
query83	299	199	183	183
query84	305	122	97	97
query85	807	445	365	365
query86	323	239	235	235
query87	2000	1954	1807	1807
query88	3693	2646	2634	2634
query89	358	290	242	242
query90	1892	173	172	172
query91	165	153	128	128
query92	92	89	88	88
query93	1043	1034	590	590
query94	436	286	260	260
query95	583	390	299	299
query96	673	509	231	231
query97	2446	2432	2316	2316
query98	163	155	148	148
query99	769	759	653	653
Total cold run time: 234713 ms
Total hot run time: 151230 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
ClickBench: Total hot run time: 24.33 s
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit 7d30bee1d9d149b76841f08d635320254e866bee, data reload: false

query1	0.01	0.01	0.01
query2	0.09	0.05	0.05
query3	0.26	0.09	0.08
query4	1.61	0.11	0.11
query5	0.23	0.24	0.24
query6	1.17	0.67	0.66
query7	0.04	0.01	0.01
query8	0.06	0.04	0.04
query9	0.40	0.34	0.35
query10	0.58	0.60	0.56
query11	0.20	0.14	0.14
query12	0.18	0.15	0.16
query13	0.50	0.49	0.49
query14	1.02	1.00	1.02
query15	0.63	0.61	0.62
query16	0.36	0.35	0.36
query17	1.17	1.12	1.12
query18	0.21	0.19	0.20
query19	2.04	2.00	1.99
query20	0.02	0.02	0.01
query21	15.50	0.20	0.13
query22	4.89	0.05	0.05
query23	16.35	0.33	0.12
query24	3.01	0.57	0.30
query25	0.12	0.07	0.06
query26	0.95	0.22	0.16
query27	0.04	0.04	0.03
query28	3.52	0.77	0.36
query29	12.51	4.13	3.29
query30	0.27	0.13	0.10
query31	2.77	0.56	0.32
query32	3.22	0.60	0.50
query33	3.14	3.26	3.30
query34	15.41	4.02	3.40
query35	3.35	3.34	3.35
query36	0.56	0.43	0.42
query37	0.09	0.06	0.06
query38	0.05	0.04	0.03
query39	0.04	0.03	0.03
query40	0.17	0.14	0.15
query41	0.08	0.03	0.02
query42	0.04	0.03	0.03
query43	0.04	0.03	0.03
Total cold run time: 96.9 s
Total hot run time: 24.33 s

xylaaaaa added a commit to xylaaaaa/doris that referenced this pull request Oct 26, 2026
…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
@xylaaaaa

Copy link
Copy Markdown
Contributor Author

run buildall

xylaaaaa added a commit to xylaaaaa/doris that referenced this pull request Oct 26, 2026
…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
xylaaaaa added a commit to xylaaaaa/doris that referenced this pull request Oct 26, 2026
…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
@morningman

Copy link
Copy Markdown
Contributor

/review

@github-actions

github-actions Bot commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

Codex automated review failed and did not complete.

Error: Selected model is at capacity. Please try a different model.
Workflow run: https://github.com/apache/doris/actions/runs/42145826183

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

xylaaaaa added a commit to xylaaaaa/doris that referenced this pull request Nov 9, 2026
…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).
xylaaaaa added a commit to xylaaaaa/doris that referenced this pull request Nov 9, 2026
…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
xylaaaaa pushed a commit to xylaaaaa/doris that referenced this pull request Nov 9, 2026
### 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
chenjunwei added 3 commits November 9, 2026 17:09
### 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
xylaaaaa pushed a commit to xylaaaaa/doris that referenced this pull request Nov 9, 2026
### 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
chenjunwei added 3 commits November 10, 2026 11:27
### 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
xylaaaaa pushed a commit to xylaaaaa/doris that referenced this pull request Nov 10, 2026
### 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
chenjunwei and others added 2 commits November 10, 2026 14:51
### 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
@xylaaaaa

Copy link
Copy Markdown
Contributor Author

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

Copy link
Copy Markdown
Contributor Author

run buildall

@sollhui

sollhui commented Nov 10, 2026

Copy link
Copy Markdown
Contributor

Could we rename S3FileSystem to ObjStorageFileSystem? With refresh_resource(), the same file system instance can switch from an S3 client to a native Azure Blob client. Seeing S3FileSystem at a call site suggests S3 protocol semantics that the class doesn't always provide. ObjStorageFileSystem would also match ObjStorageClient and ObjStorageConfig.

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

Copy link
Copy Markdown
Contributor Author

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

Copy link
Copy Markdown
Contributor Author

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

6 participants