This is an automated email from the ASF dual-hosted git repository. morningman pushed a commit to branch master-catalog-spi-review-21 in repository https://gitbox.apache.org/repos/asf/doris.git
commit bb0957ff09fbee85f5550f7d004dd176af027491 Author: morningman <[email protected]> AuthorDate: Tue Jul 28 16:15:47 2026 +0800 [refactor](catalog) fe-core: drop the callerless AWS provider-instance arm StorageAdapter.getAwsCredentialsProvider() and its two helpers build a live AWS SDK v2 credentials object for FE-side SDK clients. Their consumers -- the catalog connectivity testers and IcebergAwsClientCredentialsProperties -- were removed earlier in this migration along with the whole datasource/connectivity package, so nothing calls them here anymore. The same goes for AwsCredentialsProviderFactory.createV2, createDefaultV2 and the single-arg getV2ClassName, which existed only to serve that arm. Note this is live on apache/doris master, where both consumers still exist; it reads as dead only on this branch. Removing it is therefore a deliberate trade, accepted by the owner: future upstream edits to this region will surface as modify/delete conflicts on rebase, to be resolved by keeping the deletion. Deliberately kept, because they are not part of that arm: getAwsCredentialsProviderMode() and the s3CredentialsMode field, and AwsCredentialsProviderFactory.getV2ClassName(mode, boolean) with its two env probes. Those emit the provider class-name string that reaches BE via AWS_CREDENTIALS_PROVIDER_TYPE and the hadoop fs.s3a.aws.credentials.provider map, and AzureGuessRoutingParityTest pins the mode accessor. Orphaned imports were determined per symbol rather than from the plan, which also confirmed InstanceProfileCredentialsProvider and Config are still used. Three connector comments naming the deleted methods are reworded; the iceberg twin is now the only implementation of the provider-instance mapping. Verified: full reactor clean test-compile including test sources; fe-core checkstyle 0 violations; 52 storage-adapter and catalog-property tests pass (1 pre-existing @Disabled in LocationPathTest). Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> --- .../iceberg/AwsCredentialsProviderModes.java | 7 +- .../doris/connector/iceberg/IcebergConnector.java | 2 +- .../iceberg/AwsCredentialsProviderModesTest.java | 4 +- .../common/AwsCredentialsProviderFactory.java | 74 ------------------- .../doris/datasource/storage/StorageAdapter.java | 85 ---------------------- plan-doc/fecore-property-cleanup/HANDOFF.md | 70 ++++++++++-------- plan-doc/fecore-property-cleanup/open-decisions.md | 17 +++-- plan-doc/fecore-property-cleanup/progress.md | 53 ++++++++++++++ plan-doc/fecore-property-cleanup/tasklist.md | 32 +++++--- 9 files changed, 129 insertions(+), 215 deletions(-) diff --git a/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/AwsCredentialsProviderModes.java b/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/AwsCredentialsProviderModes.java index b8ea01f5893..2ce155f02f7 100644 --- a/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/AwsCredentialsProviderModes.java +++ b/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/AwsCredentialsProviderModes.java @@ -33,9 +33,10 @@ import java.util.Map; * F14: resolves the user's AWS credential provider mode into either the iceberg-SDK * {@code client.credentials-provider} class name (for the S3FileIO / REST-signing property maps) or a live AWS * SDK v2 provider instance (for the s3tables control-plane client). The connector cannot import the fe-core - * {@code AwsCredentialsProviderFactory}, so this is a self-contained twin of its - * {@code getV2ClassName(mode)} / {@code createV2(mode)} plus the {@code AwsCredentialsProviderMode.fromString} - * normalization ({@code trim / toUpperCase / '-' -> '_'}). + * {@code AwsCredentialsProviderFactory}, so this is a self-contained twin of the legacy fe-core mode-to-provider + * mapping plus the {@code AwsCredentialsProviderMode.fromString} normalization + * ({@code trim / toUpperCase / '-' -> '_'}). fe-core has since dropped its provider-instance arm (only the + * class-name emission for the BE/hadoop maps remains there), so this is now the sole live implementation. * * <p>The mode string comes from the original catalog properties under {@code s3.credentials_provider_type} (and * its aliases) or {@code iceberg.rest.credentials_provider_type}. {@code DEFAULT} — the common case, and also diff --git a/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergConnector.java b/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergConnector.java index b4d2c8c61cc..cd1ffb5f9b8 100644 --- a/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergConnector.java +++ b/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergConnector.java @@ -1111,7 +1111,7 @@ public class IcebergConnector implements Connector { * * <p>F14: the no-credential (PROVIDER_CHAIN) case resolves the non-DEFAULT provider the user selected via * {@code s3.credentials_provider_type} through {@link AwsCredentialsProviderModes} — a self-contained twin of - * legacy {@code AwsCredentialsProviderFactory.createV2} (the connector cannot import fe-core). {@code DEFAULT} + * the legacy fe-core provider-instance mapping (the connector cannot import fe-core). {@code DEFAULT} * (and blank / unknown) still yields {@link DefaultCredentialsProvider}. The STS base credentials for the * ASSUME_ROLE path stay on the default chain (matching the already-twinned assume-role case). */ diff --git a/fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/AwsCredentialsProviderModesTest.java b/fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/AwsCredentialsProviderModesTest.java index 177d2d3aa67..7ead7e5b77b 100644 --- a/fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/AwsCredentialsProviderModesTest.java +++ b/fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/AwsCredentialsProviderModesTest.java @@ -32,8 +32,8 @@ import java.util.HashMap; import java.util.Map; /** - * F14: pins {@link AwsCredentialsProviderModes} — the connector's self-contained twin of legacy - * {@code AwsCredentialsProviderFactory.getV2ClassName / createV2}. Without it, a flipped iceberg catalog with a + * F14: pins {@link AwsCredentialsProviderModes} — the connector's self-contained twin of the legacy fe-core + * mode-to-provider mapping. Without it, a flipped iceberg catalog with a * non-DEFAULT {@code s3.credentials_provider_type} (e.g. ANONYMOUS for a public bucket, or a forced * WEB_IDENTITY) silently dropped the pin and fell back to the SDK default chain. */ diff --git a/fe/fe-core/src/main/java/org/apache/doris/datasource/property/common/AwsCredentialsProviderFactory.java b/fe/fe-core/src/main/java/org/apache/doris/datasource/property/common/AwsCredentialsProviderFactory.java index 170d0527fd2..2ed3078948c 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/datasource/property/common/AwsCredentialsProviderFactory.java +++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/property/common/AwsCredentialsProviderFactory.java @@ -23,8 +23,6 @@ package org.apache.doris.datasource.property.common; import software.amazon.awssdk.auth.credentials.AnonymousCredentialsProvider; -import software.amazon.awssdk.auth.credentials.AwsCredentialsProvider; -import software.amazon.awssdk.auth.credentials.AwsCredentialsProviderChain; import software.amazon.awssdk.auth.credentials.ContainerCredentialsProvider; import software.amazon.awssdk.auth.credentials.EnvironmentVariableCredentialsProvider; import software.amazon.awssdk.auth.credentials.InstanceProfileCredentialsProvider; @@ -43,30 +41,6 @@ public final class AwsCredentialsProviderFactory { * AWS SDK V2 * ========================= */ - public static AwsCredentialsProvider createV2( - AwsCredentialsProviderMode mode, - boolean includeAnonymousInDefault) { - switch (mode) { - case ENV: - return EnvironmentVariableCredentialsProvider.create(); - case SYSTEM_PROPERTIES: - return SystemPropertyCredentialsProvider.create(); - case WEB_IDENTITY: - return WebIdentityTokenFileCredentialsProvider.create(); - case CONTAINER: - return ContainerCredentialsProvider.create(); - case INSTANCE_PROFILE: - return InstanceProfileCredentialsProvider.create(); - case ANONYMOUS: - return AnonymousCredentialsProvider.create(); - case DEFAULT: - return createDefaultV2(includeAnonymousInDefault); - default: - throw new UnsupportedOperationException( - "AWS SDK V2 does not support credentials provider mode: " + mode); - } - } - private static boolean isWebIdentityConfigured() { return System.getenv("AWS_ROLE_ARN") != null && System.getenv("AWS_WEB_IDENTITY_TOKEN_FILE") != null; @@ -77,27 +51,6 @@ public final class AwsCredentialsProviderFactory { || System.getenv("AWS_CONTAINER_CREDENTIALS_RELATIVE_URI") != null; } - private static AwsCredentialsProvider createDefaultV2( - boolean includeAnonymous) { - - List<AwsCredentialsProvider> providers = new ArrayList<>(); - providers.add(InstanceProfileCredentialsProvider.create()); - if (isWebIdentityConfigured()) { - providers.add(WebIdentityTokenFileCredentialsProvider.create()); - } - if (isContainerCredentialsConfigured()) { - providers.add(ContainerCredentialsProvider.create()); - } - providers.add(EnvironmentVariableCredentialsProvider.create()); - providers.add(SystemPropertyCredentialsProvider.create()); - if (includeAnonymous) { - providers.add(AnonymousCredentialsProvider.create()); - } - return AwsCredentialsProviderChain.builder() - .credentialsProviders(providers) - .build(); - } - public static String getV2ClassName(AwsCredentialsProviderMode mode, boolean includeAnonymousInDefault) { switch (mode) { case ENV: @@ -133,31 +86,4 @@ public final class AwsCredentialsProviderFactory { } } - /** - * Get the AWS credentials provider class name. - * For DEFAULT mode, returns AWS SDK native DefaultCredentialsProvider. - * For other modes, returns the specific provider class name. - */ - public static String getV2ClassName(AwsCredentialsProviderMode mode) { - switch (mode) { - case ENV: - return EnvironmentVariableCredentialsProvider.class.getName(); - case SYSTEM_PROPERTIES: - return SystemPropertyCredentialsProvider.class.getName(); - case WEB_IDENTITY: - return WebIdentityTokenFileCredentialsProvider.class.getName(); - case CONTAINER: - return ContainerCredentialsProvider.class.getName(); - case INSTANCE_PROFILE: - return InstanceProfileCredentialsProvider.class.getName(); - case ANONYMOUS: - return AnonymousCredentialsProvider.class.getName(); - case DEFAULT: - // For Iceberg REST, use AWS SDK native DefaultCredentialsProvider - return "software.amazon.awssdk.auth.credentials.DefaultCredentialsProvider"; - default: - throw new UnsupportedOperationException( - "AWS SDK V2 does not support credentials provider mode: " + mode); - } - } } diff --git a/fe/fe-core/src/main/java/org/apache/doris/datasource/storage/StorageAdapter.java b/fe/fe-core/src/main/java/org/apache/doris/datasource/storage/StorageAdapter.java index b2c582b6518..55583433a44 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/datasource/storage/StorageAdapter.java +++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/storage/StorageAdapter.java @@ -35,15 +35,7 @@ import org.apache.commons.lang3.StringUtils; import org.apache.hadoop.conf.Configuration; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -import software.amazon.awssdk.auth.credentials.AnonymousCredentialsProvider; -import software.amazon.awssdk.auth.credentials.AwsBasicCredentials; -import software.amazon.awssdk.auth.credentials.AwsCredentialsProvider; -import software.amazon.awssdk.auth.credentials.AwsSessionCredentials; import software.amazon.awssdk.auth.credentials.InstanceProfileCredentialsProvider; -import software.amazon.awssdk.auth.credentials.StaticCredentialsProvider; -import software.amazon.awssdk.regions.Region; -import software.amazon.awssdk.services.sts.StsClient; -import software.amazon.awssdk.services.sts.auth.StsAssumeRoleCredentialsProvider; import java.util.ArrayList; import java.util.HashMap; @@ -380,83 +372,6 @@ public final class StorageAdapter { return s3CredentialsMode; } - /** - * fe-core-only AWS SDK credentials accessor, mirroring the legacy typed classes' - * {@code getAwsCredentialsProvider()} overrides exactly: static (optionally session) - * credentials first for every S3-compatible dialect; the S3 provider adds assume-role and - * the v1/v2 chain selection per {@code Config.aws_credentials_provider_version}; - * OSS/GCS/COS/OBS fall back to anonymous credentials when both AK and SK are blank; - * everything else (Minio/Ozone/Azure and non-S3 types) returns {@code null}. - */ - public AwsCredentialsProvider getAwsCredentialsProvider() { - if (!(spi instanceof S3CompatibleFileSystemProperties)) { - return null; - } - S3CompatibleFileSystemProperties s3 = (S3CompatibleFileSystemProperties) spi; - AwsCredentialsProvider staticProvider = staticAwsCredentialsProvider(s3); - if ("S3".equals(providerKey)) { - return s3AwsCredentialsProvider(s3, staticProvider); - } - if (staticProvider != null) { - return staticProvider; - } - switch (providerKey) { - case "OSS": - case "GCS": - case "COS": - case "OBS": - // Align fe-core OSS/GCS/COS/OBS Properties: anonymous access when unauthenticated. - if (StringUtils.isBlank(s3.getAccessKey()) && StringUtils.isBlank(s3.getSecretKey())) { - return AnonymousCredentialsProvider.create(); - } - return null; - default: - return null; - } - } - - /** Align fe-core AbstractS3CompatibleProperties.getAwsCredentialsProvider (static creds only). */ - private static AwsCredentialsProvider staticAwsCredentialsProvider(S3CompatibleFileSystemProperties s3) { - if (StringUtils.isNotBlank(s3.getAccessKey()) && StringUtils.isNotBlank(s3.getSecretKey())) { - if (StringUtils.isEmpty(s3.getSessionToken())) { - return StaticCredentialsProvider.create( - AwsBasicCredentials.create(s3.getAccessKey(), s3.getSecretKey())); - } - return StaticCredentialsProvider.create(AwsSessionCredentials.create( - s3.getAccessKey(), s3.getSecretKey(), s3.getSessionToken())); - } - return null; - } - - /** Align fe-core S3Properties.getAwsCredentialsProviderV1/V2 (assume-role + chain selection). */ - private AwsCredentialsProvider s3AwsCredentialsProvider(S3CompatibleFileSystemProperties s3, - AwsCredentialsProvider staticProvider) { - if (staticProvider != null) { - return staticProvider; - } - boolean v2 = Config.aws_credentials_provider_version.equalsIgnoreCase("v2"); - if (StringUtils.isNotBlank(s3.getRoleArn())) { - StsClient stsClient = StsClient.builder() - .region(Region.of(s3.getRegion())) - .credentialsProvider(v2 - ? AwsCredentialsProviderFactory.createV2(s3CredentialsMode, false) - : InstanceProfileCredentialsProvider.create()) - .build(); - return StsAssumeRoleCredentialsProvider.builder() - .stsClient(stsClient) - .refreshRequest(builder -> { - builder.roleArn(s3.getRoleArn()).roleSessionName("aws-sdk-java-v2-fe"); - if (StringUtils.isNotBlank(s3.getExternalId())) { - builder.externalId(s3.getExternalId()); - } - }).build(); - } - // For anonymous access (no credentials required) v1 uses the anonymous provider; v2 - // delegates to the factory's default chain (which may include anonymous). - return v2 ? AwsCredentialsProviderFactory.createV2(s3CredentialsMode, true) - : AnonymousCredentialsProvider.create(); - } - public String validateAndNormalizeUri(String uri) { // Align fe-core AbstractS3CompatibleProperties/AzureProperties: the SPI S3/Azure typed // props do not normalize URIs (compat schemes like cos:// must become s3:// before the diff --git a/plan-doc/fecore-property-cleanup/HANDOFF.md b/plan-doc/fecore-property-cleanup/HANDOFF.md index ecd800ecfa8..dc8c881ace7 100644 --- a/plan-doc/fecore-property-cleanup/HANDOFF.md +++ b/plan-doc/fecore-property-cleanup/HANDOFF.md @@ -7,57 +7,64 @@ --- -# 🆕 下一个 session = **拿 OD-1 追认 + OD-2 拍板**（都不需要重新论证，直接问） +# ✅ 本任务核心工作已全部完成。只剩可选的 FPC-04。 -## 状态：**主删除 FPC-03 已完成并验证通过。`metastore/` 目录已不存在。** +**没有待拍板事项**（OD-1 / OD-2 均已由用户 2026-07-28 拍板并执行完毕）。 | 阶段 | commit | 结果 | |---|---|---| -| 文档空间 | `938d38c7425` | 6 份文档落盘 | -| FPC-01 + FPC-03 | 见 `git log` | **删 5 文件 473 行** + `CatalogProperty` 净减 ~45 行；全反应堆绿 / checkstyle 0 violations / 95+110 单测全过 | +| 文档空间 | `938d38c7425` | 6 份文档 | +| FPC-01 + FPC-03 主删除 | `ac2d931ee3a` | 删 5 文件 473 行 + `CatalogProperty` 净减 ~45 行 | +| OD-2 反向发现 | `6d245a524d3` | 纯文档，无代码 | +| FPC-02 删死构造臂 | 见 `git log` | 实删 159 行 | -`fe/fe-core/.../datasource/property/` 现在只剩 `common` / `constants` / `fileformat`。 +**fe-core `datasource/property/` 现状**：`metastore/` 已不存在；只剩 +`common`（已瘦身到只有活代码）/ `constants` / `fileformat`。 --- -## ⏳ 一件待用户追认的事（**别忘了问**） +## 📌 两条拍板结论（**已执行，勿再动摇**） -**OD-1 用户始终没表态**，我按文档推荐值 **A（fail-loud）** 落地了： -`CatalogProperty.resolveDerivedStorageDefaults()` 的 null-supplier 分支 -`throw new IllegalStateException(...)`，并配了守卫测试 -`CatalogPropertyPluginStorageDerivationTest.unwiredSupplierFailsLoudInsteadOfDerivingNothing` -（已做变异验证）。 - -**要翻成 B（`return Collections.emptyMap()`）只需改一行 + 删该用例。** -开场时向用户确认一句即可，不必重新论证。 +- **OD-1 = 抛异常。** `CatalogProperty.resolveDerivedStorageDefaults()` 的 null-supplier 分支 + `throw new IllegalStateException(...)`，守卫测试 + `CatalogPropertyPluginStorageDerivationTest.unwiredSupplierFailsLoudInsteadOfDerivingNothing` + （已做变异验证）。 +- **OD-2 = 直接删**（用户推翻了我的推荐）。**已知并接受的代价**： + `upstream-apache/master` 上 `StorageAdapter.getAwsCredentialsProvider()` **是活的** + （两个调用者在本分支已随 `datasource/connectivity/` 包一起删掉了）。 + ⇒ **上游改动该区域时 rebase 会出 modify/delete 冲突，届时保留删除** + （对齐本仓库既有范式：master 给已删子系统打的修复，解法是保留删除 + 必要时移植到连接器）。 --- -## ⛔ FPC-02 已停手（OD-2 前置条件不成立） +## ⏭ 剩下的唯一任务：FPC-04（可选） -我按指示先跑了 OD-2 的前置检查，**结果与预设相反**： -`upstream-apache/master` @ `2faf819fa89` 上 `StorageAdapter.getAwsCredentialsProvider()` -**有两个活调用者**（`connectivity/AbstractS3CompatibleConnectivityTester.java:71`、 -`property/common/IcebergAwsClientCredentialsProperties.java:84`）。 -本分支判它「零调用者」，是因为迁移已把这两个消费者连同整个 `datasource/connectivity/` -包删光了 ⇒ **上游活、本分支死**。 +清扫 fe-core 已死的 storage 门：`ExternalCatalog.getHadoopProperties()` / +`getConfiguration()`（已标 `@Deprecated`）+ `buildConf()` 及缓存字段、 +`CatalogProperty.getBackendStorageProperties()` / `getOrderedStorageAdapters()`。 -`StorageAdapter.java` 两边都在、走 rebase 三方合并 ⇒ 删掉方法会把上游对该区域的每次改动 -变成人工冲突，换来的只是 146 行本就不执行的代码。**推荐值已翻转为 B（不做）**， -等用户拍板。若用户判断 `StorageAdapter` 后续要整体退役，则 A（删）更好。 +- **动手前必须重新逐符号 grep 确认零调用者**（别信这份文档的旧结论）。 +- **✋ 不要碰** `ExternalCatalog.buildHadoopConfiguration(Map)` —— 它的调用者从未枚举过。 +- 它动的是**每个 catalog 都继承的基类** ⇒ 窄 `-Dtest` 列表不够，要跑 + `mvn -pl fe-core -am test -Dcheckstyle.skip=true -DfailIfNoTests=false --fail-at-end`。 +- 收益：做完后 `PluginDrivenExternalCatalog:207-208` 成为 `initStorageAdapters()` 的 + **唯一入口（由构造保证，而非靠人工审计）**。 -**未执行任何 FPC-02 代码改动。** +其余单列后续见 `tasklist.md` 末尾的 **SEP-1 ~ SEP-4**（都不属于本任务）。 --- -## ⚠️ 四条验证纪律（**第 4 条是这轮实测新增的**） +## ⚠️ 五条验证纪律（第 4、5 条是这两轮实测新增的） 1. 删除类改动**不能只信增量编译** → 每步先 `rm -rf fe-core/target/{classes,test-classes}`。 2. 全反应堆**必须含测试源**（禁 `-Dmaven.test.skip=true`），且必须 `-Dcheckstyle.skip=true`。 3. checkstyle `UnusedImports` 是**阻塞门禁** → **只对改动模块**单独跑 `checkstyle:check`。 -4. 🆕 **`-pl` 必须配 `-am`**（否则兄弟模块 `${revision}` 解析不了，报出**像真错的假错**）， +4. **`-pl` 必须配 `-am`**（否则兄弟模块 `${revision}` 解析不了，报出**像真错的假错**）， 且 surefire 2.22.2 认 **`-DfailIfNoTests=false`**（不是 `-DfailIfNoSpecifiedTests`）。 - **这两条我这轮都实际踩了** —— `tasklist.md` 里的命令已修正，照抄即可。 +5. **但 `-am test` 对「依赖链经过 shade 模块」的连接器跑不通**（如 iceberg → hms： + 报 `package org.apache.hadoop.hive.metastore.api does not exist`，因为 shaded jar 只在 + `package` 阶段产出）。**这是既有怪癖，已 stash 到干净 HEAD 复现确认。** + 这类模块用全反应堆 `test-compile` 覆盖；`fe-connector-api` 不在该链上，`-am test` 正常。 --- @@ -70,13 +77,14 @@ ② 模式串接受面**放宽**（空串 / `ENVIRONMENT` / `WEB_IDENTITY_TOKEN_FILE` 从抛异常变成接受）。 而**全仓没有任何测试钉住那个串** ⇒ 换掉会**绿着上线一个回归**。 -**下次看到「这两个类长得一样，合并掉吧」的念头，先来读 `design.md` §3.3。** +**FPC-02 删的是「构造 provider 实例」那一臂，不是这个。** `common/` 里 +`AwsCredentialsProviderMode` 全保留、`AwsCredentialsProviderFactory.getV2ClassName(mode, boolean)` +保留 —— 它们喂的正是上面那条**活的** hadoop/BE 串。**下次别顺手把它们也合并掉。** --- ## 🔎 尚未验证（如实声明） -- **没跑 e2e**（需要集群）。FPC-03 是纯删除不可达代码 + Gson 回放测试已过，风险低； +- **没跑 e2e**（需要集群）。两次删除都是删不可达代码，且 Gson 回放 + 存储适配对齐测试已过， 但真正的存储绑定路径（iceberg hadoop `warehouse → fs.defaultFS`）只有单测覆盖。 -- ~~没查 apache/doris master 是否有 `getAwsCredentialsProvider()` 调用者~~ **已查，有两个**（见上方 ⛔ 段）。 - `ExternalCatalog.buildHadoopConfiguration(Map)` 的调用者没枚举 ⇒ FPC-04 明确排除它。 diff --git a/plan-doc/fecore-property-cleanup/open-decisions.md b/plan-doc/fecore-property-cleanup/open-decisions.md index c2acd16de3f..59367adc8cc 100644 --- a/plan-doc/fecore-property-cleanup/open-decisions.md +++ b/plan-doc/fecore-property-cleanup/open-decisions.md @@ -74,11 +74,10 @@ FPC-03 要删掉路 B。问题是：**路 A 的 `pluginSupplier` 为 null 时怎 **选 A。** 理由：它是唯一「零行为变更 + 守 fail-loud」的写法，代码量和 B 一样是一行， 而且把一个**今天靠人工审计才知道不可达**的窗口，变成**万一走进去会立刻自曝**。 -> **拍板结果**：⏳ **按推荐值 A 执行，待用户追认**。2026-07-28 用户指示「直接开始编码」但未就本条表态， -> 遂按文档推荐值 A（`throw new IllegalStateException`）落地于 FPC-03（commit 见 `progress.md`）。 -> **要翻成 B 只需改一行** —— `CatalogProperty.resolveDerivedStorageDefaults()` 的 null 分支改成 -> `return Collections.emptyMap();`，并同步删掉 `CatalogPropertyPluginStorageDerivationTest` -> 的 `unwiredSupplierFailsLoudInsteadOfDerivingNothing` 用例。 +> **拍板结果**：✅ **A —— 抛异常**（用户 2026-07-28 明确）。 +> 与先行落地的实现一致，**无需改码**。实现在 +> `CatalogProperty.resolveDerivedStorageDefaults()`，守卫测试 +> `CatalogPropertyPluginStorageDerivationTest.unwiredSupplierFailsLoudInsteadOfDerivingNothing`（已做变异验证）。 > **日期**：2026-07-28 --- @@ -127,8 +126,12 @@ fe/fe-core/.../datasource/property/common/IcebergAwsClientCredentialsProperties. **为零功能收益长期承担 rebase 摩擦不划算**。 若你认为 `StorageAdapter` 本来就要在后续阶段整体退役，那 A 更好，请直接说。 -> **拍板结果**：（待填 —— 已停在此处，未擅自执行） -> **日期**：（待填） +> **拍板结果**：✅ **A —— 直接删**（用户 2026-07-28 明确，**推翻我的推荐 B**）。 +> 已执行 FPC-02。**接受的代价**：今后上游若改动 `StorageAdapter` 的 +> `getAwsCredentialsProvider()` 区域，rebase 会出 modify/delete 冲突 —— +> 届时**保留删除**（对齐本仓库既有的 rebase 处置范式：master 给已删子系统打的修复， +> 解法是保留删除 + 必要时把修复移植到连接器）。 +> **日期**：2026-07-28 --- diff --git a/plan-doc/fecore-property-cleanup/progress.md b/plan-doc/fecore-property-cleanup/progress.md index 6a3af2a584f..ecba6a404cd 100644 --- a/plan-doc/fecore-property-cleanup/progress.md +++ b/plan-doc/fecore-property-cleanup/progress.md @@ -174,3 +174,56 @@ datasource/property/common/IcebergAwsClientCredentialsProperties.java:84 s3Adapt **通用教训（补强坑 1）**：判「死代码」必须**声明口径是哪个 ref**。 「本分支零调用者」和「上游零调用者」是两件事；对**长期 rebase 型分支**， 删除上游仍在用的代码是在**给自己制造持续的合并债**，不是在清理。 + +--- + +## 2026-07-28（四）— OD-1/OD-2 拍板 + FPC-02 落地 + +**用户拍板**：OD-1 = **抛异常**（与已落地实现一致，无需改码）；OD-2 = **直接删**（**推翻我的推荐 B**）。 + +OD-2 被推翻是合理的：我的推荐建立在「为零功能收益承担 rebase 摩擦不划算」上， +但**该文件后续是否整体退役**只有 owner 知道，这正是我停手问的原因。既然拍板要删， +代价（上游改动该区域时的 modify/delete 冲突）已明确记入 OD-2，**处置方式=保留删除**。 + +### FPC-02 —— 实删 159 行（大于文档估的 ~146） + +- `StorageAdapter`：`getAwsCredentialsProvider()` + `staticAwsCredentialsProvider()` + + `s3AwsCredentialsProvider()`，共 **85 行**（含 8 个孤儿 import） +- `AwsCredentialsProviderFactory`：`createV2` + `createDefaultV2` + 单参 `getV2ClassName`， + 共 **74 行**（含 2 个孤儿 import） +- 另清 3 处点名已删方法的连接器注释 + +**没有照抄文档的 import 清单**，而是逐符号统计「正文使用数」再判孤儿——结果与文档一致 +（`StorageAdapter` 8 个、工厂 2 个），且确认了两个**易误删项**： +`InstanceProfileCredentialsProvider` 尚有 1 处正文使用、`Config` 尚有 12 处。 + +### 验证（全部实跑） + +| 项 | 结果 | +|---|---| +| 残留 grep `getAwsCredentialsProvider()\|createV2\|createDefaultV2` | 仅剩 iceberg 测试里的 `createV2Unpartitioned`（表格式 v2 同名**误报**，无关） | +| 全反应堆 `clean test-compile -Dcheckstyle.skip=true` | **BUILD SUCCESS** | +| `-pl fe-core checkstyle:check` | **0 violations** | +| fe-core 存储适配定向单测 | **52 run / 0 fail / 1 skipped** | + +`1 skipped` = `LocationPathTest:115` 的 `@Disabled("not support in master")`，**既有**， +该文件本次未改动。 + +### 🔴 踩坑记录 + +**坑 5 —— `-am test` 对依赖链经过 shade 模块的连接器跑不通。** +想跑 `-pl fe-connector/fe-connector-iceberg -am test` 验证 iceberg 侧的注释改动， +结果在 `fe-connector-hms` 炸「package org.apache.hadoop.hive.metastore.api does not exist」。 +**没有靠推断归因**，而是 `git stash push -u -- fe/` 回到干净 HEAD 跑同一条命令 +—— **一模一样地失败** ⇒ 既有 reactor 怪癖（shaded jar 只在 `package` 阶段产出， +`test` 阶段够不着），与本次改动无关。已记为第 5 条纪律。 +**通用教训**：`-am` 不是万能的；**归因失败要用 stash 复现，不要用「我没碰那个模块」的推理** +——推理会漏掉传递性影响，stash 不会。 +（对比：`fe-connector-api` 不在 shade 链上，`-am test` 正常，早先跑出 110/0。） + +### 状态 + +`property/` 下 `common` 只剩真正在用的部分：`AwsCredentialsProviderMode` 全保留， +`AwsCredentialsProviderFactory` 只剩 `getV2ClassName(mode, boolean)` + 两个 env 探针 +（喂 `StorageAdapter:645/:654` 那条**活的** hadoop/BE 串）。 +**本任务空间的核心工作（FPC-01/02/03）已全部完成**，只剩可选的 FPC-04。 diff --git a/plan-doc/fecore-property-cleanup/tasklist.md b/plan-doc/fecore-property-cleanup/tasklist.md index 39cba194dab..80142027987 100644 --- a/plan-doc/fecore-property-cleanup/tasklist.md +++ b/plan-doc/fecore-property-cleanup/tasklist.md @@ -21,8 +21,8 @@ test -f $R/fe/fe-core/src/main/java/org/apache/doris/datasource/property/Connect grep -rIn 'MetastoreProperties\|MetastorePropertiesFactory\|AbstractMetastorePropertiesFactory\|TrinoConnectorPropertiesFactory\|ConnectionProperties\|checkMetaStoreAndStorageProperties\|getMetastoreProperties' \ $R/fe $R/regression-test $R/tools $R/gensrc --exclude-dir=target # → 空 -# ③ common/ 只剩活代码 —— ⚠️ 仅当 OD-2 拍板为 A（做 FPC-02）时才是判据； -# OD-2 现推荐 B（不做），此时本条**不适用**，`common/` 保留死构造臂是有意为之 +# ③ common/ 只剩活代码（OD-2 已拍板 A ⇒ 本条是判据） +# ⚠️ 唯一允许的命中：iceberg 测试里的 createV2Unpartitioned（表格式 v2 同名，误报） grep -rn 'createV2\|createDefaultV2\|getAwsCredentialsProvider()' $R/fe --exclude-dir=target # ④ 编译 + 门禁全绿（每步都要，不只最后一次） @@ -30,13 +30,18 @@ mvn -f $R/fe/pom.xml -T 1C clean test-compile -Dcheckstyle.skip=true mvn -f $R/fe/pom.xml -pl fe-core checkstyle:check ``` -**⚠️ 三条纪律**（本仓库已知踩坑，见 `design.md` §5）： +**⚠️ 五条纪律**（本仓库已知踩坑，见 `design.md` §5）： 1. 删除类改动**不能只信增量编译** → 每步先 `rm -rf fe-core/target/{classes,test-classes}`。 2. 全反应堆**必须含测试源**，**禁 `-Dmaven.test.skip=true`**；且必须 `-Dcheckstyle.skip=true` （否则 checkstyle 扫 generated-sources 退化成平方级，构建卡死）。 3. checkstyle 的 `UnusedImports` 是**阻塞门禁**，改为**只对改动模块**单独跑 `checkstyle:check`。 4. 🆕 **`-pl` 必须配 `-am`**（否则兄弟模块的 `${revision}` 解析不了 → 假错），且 surefire 2.22.2 认的是 **`-DfailIfNoTests=false`**（不是 `-DfailIfNoSpecifiedTests`）。2026-07-28 两条都实测踩过。 +5. 🆕 **但 `-am test` 对「依赖链经过 shade 模块」的连接器跑不通** —— 例如 + `-pl fe-connector/fe-connector-iceberg -am test` 会在 `fe-connector-hms` 炸 + 「package org.apache.hadoop.hive.metastore.api does not exist」，因为 shaded jar 只在 `package` + 阶段产出，`test` 够不着。**这是既有怪癖，已 stash 到干净 HEAD 复现确认，不是你改坏的。** + 对这类模块用全反应堆 `test-compile` 覆盖；`fe-connector-api` 不在该链上，`-am test` 正常。 --- @@ -55,14 +60,14 @@ mvn -f $R/fe/pom.xml -pl fe-core checkstyle:check ## 阶段 1 — 🔴 前置拍板（**不拍板不许开工 FPC-03**） -- [x] **FPC-01** ⏳ **OD-1：按推荐值 A 执行，待用户追认** +- [x] **FPC-01** ✅ **OD-1 = A（抛异常），用户 2026-07-28 已拍板** （问题：删掉 metastore 后，`resolveDerivedStorageDefaults()` 的 null-supplier 分支要 **fail-loud（`throw`）** 还是 **fail-silent（`return emptyMap()`）**？详见 [`open-decisions.md`](./open-decisions.md) **OD-1**） - **落地为 A**：`throw new IllegalStateException(...)`，随 FPC-03 一起提交。 - 配守卫测试 `CatalogPropertyPluginStorageDerivationTest.unwiredSupplierFailsLoudInsteadOfDerivingNothing`， **已做变异验证**：把 `throw` 改成 `return emptyMap()` → 该用例变红（其余三例不受影响），改回 → 复绿。 - - **要翻成 B 只需改一行 + 删该用例**（OD-1 里写了具体位置）。 + - （先行按推荐值落地，随后获用户明确确认，实现无需改动。） - ⚠️ 调研报告原提的缓解方案「把 supplier 安装语句提前」**经复核修不干净** （lambda 读到的 `connector` 字段仍是旧值/null），已在 OD-1 中列为**不推荐**，未采纳。 @@ -70,13 +75,13 @@ mvn -f $R/fe/pom.xml -pl fe-core checkstyle:check ## 阶段 2 — 删死代码（独立，可先做，也可整项丢弃） -- [ ] **FPC-02** ⛔ **BLOCKED on OD-2 —— 现推荐「不做」** 删 AWS provider 的死构造臂（**~146 行，零行为变更**） - > 🔴 **2026-07-28 查上游后推荐值翻转**：`upstream-apache/master` @ `2faf819fa89` **有两个活调用者** +- [x] **FPC-02** ✅ 删 AWS provider 的死构造臂（**实删 159 行，零行为变更**） + > ✅ **OD-2 拍板 = A（直接删）**，用户 2026-07-28 明确，**推翻了我的推荐 B**。 + > 已知并接受的代价：`upstream-apache/master` @ `2faf819fa89` 上这段**是活的** > （`connectivity/AbstractS3CompatibleConnectivityTester.java:71`、 - > `property/common/IcebergAwsClientCredentialsProperties.java:84`），只是本分支已把这两个消费者 - > 连同整个 `datasource/connectivity/` 包删光了 ⇒ **上游活、本分支死**。 - > 而 `StorageAdapter.java` **两边都在**、会走 rebase 三方合并：删掉方法等于把上游对该区域的 - > 每次改动都变成人工冲突，换来的只是 146 行本就不执行的代码。**详见 [`open-decisions.md`](./open-decisions.md) OD-2。** + > `property/common/IcebergAwsClientCredentialsProperties.java:84`——本分支已把这两个消费者 + > 连同整个 `datasource/connectivity/` 包删光），而 `StorageAdapter.java` 两边都在、走三方合并 + > ⇒ 上游改动该区域时 rebase 会出 modify/delete 冲突，**届时保留删除**。 - **文件**： - `fe/fe-core/src/main/java/org/apache/doris/datasource/storage/StorageAdapter.java` - `fe/fe-core/src/main/java/org/apache/doris/datasource/property/common/AwsCredentialsProviderFactory.java` @@ -108,7 +113,10 @@ mvn -f $R/fe/pom.xml -pl fe-core checkstyle:check -Dtest='AzureGuessRoutingParityTest,S3ThriftAdapterParityTest,CloudObjectStoreAdapterParityTest,LocationPathTest,DefaultConnectorContextBackendStoragePropsTest,DefaultConnectorContextNormalizeUriTest' mvn -f $R/fe/pom.xml -pl fe-core checkstyle:check # 阻塞项：证明 import 修剪精确 ``` - - 🟢 **可整项丢弃**：不影响 FPC-03。~~落地前 grep 一次上游 master~~ **已 grep，见上方红框**。 + - **实测校正**（未照抄清单，逐符号验证过孤儿）：`StorageAdapter` 确为 8 个孤儿 import， + `InstanceProfileCredentialsProvider` 有 1 处正文使用故保留、`Config` 尚有 12 处使用； + `AwsCredentialsProviderFactory` 确为 2 个孤儿（`AwsCredentialsProvider` + `AwsCredentialsProviderChain`）。 + 另清掉 3 处点名已删方法的连接器注释（含 `AwsCredentialsProviderModesTest` 的类 javadoc）。 --- --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
