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 d3a2abdf376dc9887a536c089fd4de83bc02fa6a Author: morningman <[email protected]> AuthorDate: Tue Jul 28 14:53:03 2026 +0800 [doc](catalog) record that the dead AWS provider arm is live upstream The task space assumed StorageAdapter.getAwsCredentialsProvider() was simply dead code and planned to delete it, gated on first checking apache/doris master. That check now ran, and the assumption was wrong: upstream master (2faf819fa89) has two live callers, connectivity/AbstractS3CompatibleConnectivityTester and property/common/IcebergAwsClientCredentialsProperties. It reads as dead here only because this branch's migration already deleted both consumers along with the whole datasource/connectivity package. Since StorageAdapter itself exists on both sides and is three-way merged on every rebase, removing the method would turn every upstream edit to that region into a manual conflict, in exchange for 146 lines that never execute. The recommendation is flipped to "do not delete" and the task is left blocked on the owner rather than carried out. No code changed. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> --- plan-doc/fecore-property-cleanup/HANDOFF.md | 27 ++++++++-------- plan-doc/fecore-property-cleanup/open-decisions.md | 36 ++++++++++++++++------ plan-doc/fecore-property-cleanup/progress.md | 29 +++++++++++++++++ plan-doc/fecore-property-cleanup/tasklist.md | 16 +++++++--- 4 files changed, 80 insertions(+), 28 deletions(-) diff --git a/plan-doc/fecore-property-cleanup/HANDOFF.md b/plan-doc/fecore-property-cleanup/HANDOFF.md index 677dfdb52bb..ecd800ecfa8 100644 --- a/plan-doc/fecore-property-cleanup/HANDOFF.md +++ b/plan-doc/fecore-property-cleanup/HANDOFF.md @@ -7,7 +7,7 @@ --- -# 🆕 下一个 session = **FPC-02**（删 AWS 死构造臂），先过 OD-2 +# 🆕 下一个 session = **拿 OD-1 追认 + OD-2 拍板**（都不需要重新论证，直接问） ## 状态：**主删除 FPC-03 已完成并验证通过。`metastore/` 目录已不存在。** @@ -33,21 +33,20 @@ --- -## 📋 下一步：FPC-02（`tasklist.md` 阶段 2） +## ⛔ FPC-02 已停手（OD-2 前置条件不成立） -删 `StorageAdapter.getAwsCredentialsProvider()` + 两个私有 helper， -以及 `AwsCredentialsProviderFactory` 的 `createV2` / `createDefaultV2` / 单参 `getV2ClassName`， -共 **~146 行零调用者代码，零行为变更**。 +我按指示先跑了 OD-2 的前置检查，**结果与预设相反**： +`upstream-apache/master` @ `2faf819fa89` 上 `StorageAdapter.getAwsCredentialsProvider()` +**有两个活调用者**（`connectivity/AbstractS3CompatibleConnectivityTester.java:71`、 +`property/common/IcebergAwsClientCredentialsProperties.java:84`）。 +本分支判它「零调用者」，是因为迁移已把这两个消费者连同整个 `datasource/connectivity/` +包删光了 ⇒ **上游活、本分支死**。 -**动手前先过 OD-2**：grep 一次上游 `apache/doris` master 看有没有 -`getAwsCredentialsProvider()` 的调用者（这段是上游 `f499c78c67c` / #66004 整体带进来的， -若上游有调用者，下次 rebase 会 modify/delete 冲突）。 +`StorageAdapter.java` 两边都在、走 rebase 三方合并 ⇒ 删掉方法会把上游对该区域的每次改动 +变成人工冲突，换来的只是 146 行本就不执行的代码。**推荐值已翻转为 B（不做）**， +等用户拍板。若用户判断 `StorageAdapter` 后续要整体退役，则 A（删）更好。 -⚠️ `tasklist.md` FPC-02 里的**「必须保留」清单要逐条对**—— -`getAwsCredentialsProviderMode()` 和 `s3CredentialsMode` 字段是**活的**（喂 BE 的 -`AWS_CREDENTIALS_PROVIDER_TYPE`，且被 `AzureGuessRoutingParityTest` 钉着），删了会炸。 - -🟢 FPC-02 **可以整项丢弃**，不影响已完成的 FPC-03。 +**未执行任何 FPC-02 代码改动。** --- @@ -79,5 +78,5 @@ - **没跑 e2e**（需要集群）。FPC-03 是纯删除不可达代码 + Gson 回放测试已过，风险低； 但真正的存储绑定路径（iceberg hadoop `warehouse → fs.defaultFS`）只有单测覆盖。 -- **没查 apache/doris master** 是否有 `getAwsCredentialsProvider()` 调用者 → 这正是 OD-2。 +- ~~没查 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 7775b83bc67..c2acd16de3f 100644 --- a/plan-doc/fecore-property-cleanup/open-decisions.md +++ b/plan-doc/fecore-property-cleanup/open-decisions.md @@ -93,23 +93,41 @@ FPC-03 要删掉路 B。问题是：**路 A 的 `pluginSupplier` 为 null 时怎 `AwsCredentialsProviderFactory` 的 `createV2` / `createDefaultV2` / 单参 `getV2ClassName` 加起来约 **146 行，零调用者**（我复核过：全仓只有它自己的声明和 javadoc）。 -### 唯一的顾虑 +### 🔴 2026-07-28 已查上游：**前置条件不成立，推荐值翻转为 B** -这段代码是从上游 `f499c78c67c`（#66004）**整体带进来**的。如果 apache/doris master 上有、 -或将来加了调用者，下次 rebase 就会撞 modify/delete 冲突。 +原推荐是「先 grep 一次上游 master；**无调用者**就删」。**查了，有调用者** +（`upstream-apache/master` @ `2faf819fa89`）： -**我没有查上游 master**（`design.md` §8 已如实声明）。 +``` +fe/fe-core/.../datasource/connectivity/AbstractS3CompatibleConnectivityTester.java:71 + adapter.getAwsCredentialsProvider() ← 就是 StorageAdapter 的这个方法 +fe/fe-core/.../datasource/property/common/IcebergAwsClientCredentialsProperties.java:84 + s3Adapter.getAwsCredentialsProvider() ← 同上 +``` + +**它在上游是活的，在本分支才是死的** —— 因为本分支的迁移已经把这两个消费者 +（连同整个 `datasource/connectivity/` 包）删光了。 + +**这就改变了性价比**：`StorageAdapter.java` 本身**两边都在**、会被 rebase 三方合并。 +今天上游对 `getAwsCredentialsProvider()` 或其邻近代码的任何改动都能干净合入； +一旦我把方法删掉，这些改动就变成**每次 rebase 都要人工处理的冲突 hunk** —— +而换来的只是 146 行**本来就没人执行**的代码，功能收益为零。 + +（`AwsCredentialsProviderFactory` 的 `createV2`/`createDefaultV2` 是同一处的下游， +删了方法才轮得到它们，所以一并搁置。） -### 选项 +### 选项（更新后） | | 做法 | |---|---| -| **A（推荐）** | 先 grep 一次上游 master；无调用者就删。删了 146 行死代码，且让 `common/` 的存活理由更清晰（剩下的都是真在用的） | -| **B** | 不做。反正是死代码，留着不碍事，省掉一次潜在的 rebase 冲突 | +| **B（现推荐）** | **不做 FPC-02。** 死代码留着零成本，避免给一个高频 rebase 的分支平添长期冲突面 | +| **A** | 仍然删。若判断本分支终局是「不再跟随上游 `StorageAdapter`」（例如该文件本就要整体重写/删除），那冲突面是虚的，删掉更干净 | -**我的推荐：A**，但**低优先级**——它和主线（FPC-03）完全解耦，什么时候做都行。 +**我的推荐：B。** 判据是 memory 里那条「上游定期 rebase + force-push」—— +**为零功能收益长期承担 rebase 摩擦不划算**。 +若你认为 `StorageAdapter` 本来就要在后续阶段整体退役，那 A 更好，请直接说。 -> **拍板结果**：（待填） +> **拍板结果**：（待填 —— 已停在此处，未擅自执行） > **日期**：（待填） --- diff --git a/plan-doc/fecore-property-cleanup/progress.md b/plan-doc/fecore-property-cleanup/progress.md index 9eac420834a..6a3af2a584f 100644 --- a/plan-doc/fecore-property-cleanup/progress.md +++ b/plan-doc/fecore-property-cleanup/progress.md @@ -145,3 +145,32 @@ paimon `TcclPinningConnectorContext:49`、`DatasourcePrintableMap:69` - `metastore/` 目录已不存在；`property/` 下只剩 `common` / `constants` / `fileformat`。 - 下一步：**FPC-02**（删 AWS 死构造臂）——先按 OD-2 grep 一次上游 master。 + +--- + +## 2026-07-28（三）— FPC-02 停手：OD-2 前置条件不成立 + +按 HANDOFF 的指示，动 FPC-02 前先执行 OD-2 的前置检查「grep 一次上游 master」。 + +**结果与预设相反。** `upstream-apache/master` @ `2faf819fa89` 上 +`StorageAdapter.getAwsCredentialsProvider()` **有两个活调用者**： + +``` +datasource/connectivity/AbstractS3CompatibleConnectivityTester.java:71 adapter.getAwsCredentialsProvider() +datasource/property/common/IcebergAwsClientCredentialsProperties.java:84 s3Adapter.getAwsCredentialsProvider() +``` + +本分支之所以判它「零调用者」，是因为迁移**已经把这两个消费者连同整个 +`datasource/connectivity/` 包删光了** —— 即 **上游活、本分支死**。 + +**为什么这翻转了结论**：`StorageAdapter.java` 本身两边都在，会走 rebase 三方合并。 +今天上游改动该区域能干净合入；删掉方法后，这些改动就变成**每次 rebase 的人工冲突 hunk**， +而收益只是 146 行本就不执行的代码 —— 对一个**定期 rebase 到 force-push 上游**的分支， +这笔账不划算。 + +⇒ **已停手，未执行 FPC-02**，`open-decisions.md` OD-2 推荐值改为 **B（不做）**，等用户拍板。 +若用户判断 `StorageAdapter` 本身后续要整体退役，则冲突面是虚的，A（删）更好。 + +**通用教训（补强坑 1）**：判「死代码」必须**声明口径是哪个 ref**。 +「本分支零调用者」和「上游零调用者」是两件事；对**长期 rebase 型分支**， +删除上游仍在用的代码是在**给自己制造持续的合并债**，不是在清理。 diff --git a/plan-doc/fecore-property-cleanup/tasklist.md b/plan-doc/fecore-property-cleanup/tasklist.md index c3356f00ada..39cba194dab 100644 --- a/plan-doc/fecore-property-cleanup/tasklist.md +++ b/plan-doc/fecore-property-cleanup/tasklist.md @@ -21,8 +21,9 @@ 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/ 只剩活代码 -grep -rn 'createV2\|createDefaultV2\|getAwsCredentialsProvider()' $R/fe --exclude-dir=target # → 空 +# ③ common/ 只剩活代码 —— ⚠️ 仅当 OD-2 拍板为 A（做 FPC-02）时才是判据； +# OD-2 现推荐 B（不做），此时本条**不适用**，`common/` 保留死构造臂是有意为之 +grep -rn 'createV2\|createDefaultV2\|getAwsCredentialsProvider()' $R/fe --exclude-dir=target # ④ 编译 + 门禁全绿（每步都要，不只最后一次） mvn -f $R/fe/pom.xml -T 1C clean test-compile -Dcheckstyle.skip=true @@ -69,7 +70,13 @@ mvn -f $R/fe/pom.xml -pl fe-core checkstyle:check ## 阶段 2 — 删死代码（独立，可先做，也可整项丢弃） -- [ ] **FPC-02** ⬜ 删 AWS provider 的死构造臂（**~146 行，零行为变更**） +- [ ] **FPC-02** ⛔ **BLOCKED on OD-2 —— 现推荐「不做」** 删 AWS provider 的死构造臂（**~146 行，零行为变更**） + > 🔴 **2026-07-28 查上游后推荐值翻转**：`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。** - **文件**： - `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` @@ -101,8 +108,7 @@ 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 是否有 - `getAwsCredentialsProvider()` 调用者（rebase 冲突风险，`design.md` §5） + - 🟢 **可整项丢弃**：不影响 FPC-03。~~落地前 grep 一次上游 master~~ **已 grep，见上方红框**。 --- --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
