924060929 commented on PR #66729:
URL: https://github.com/apache/doris/pull/66729#issuecomment-5674132615
## FE 部分 Review
### 变更概览
PR 标题是 BE Java extensions 的插件化隔离重构，但 FE 侧变更量很大（230 个文件），涉及三个层面：
1. **SPI 层**：`ConnectorMetadata` 新增 `listsPartitionsAtSnapshot` 能力声明方法，API
major 版本从 6 升到 7
2. **Connector 实现层**：HiveConnector 生命周期加固、HudiConnector 引入 per-configuration
FileSystem scope 引用计数
3. **fe-core 层**：`PluginDrivenMvccExternalTable` 支持 snapshot-aware
分区枚举，`CreateFunctionCommand` 适配插件化 UDF 缓存
另外 `fe/be-java-extensions/` 下是 JNI 插件化的主体重构（205 个文件），引入 `DorisPlugin`
SPI、`PluginClassLoader`、`PluginRuntime` 等。
---
### 设计评价
**整体设计质量很高。** 几个核心机制的设计思路清晰、论证充分：
#### 1. HudiConnector FS_SCOPES 引用计数 — 解决真实的生产问题
核心洞察：Hadoop `FileSystem` 缓存是 static strong reference，key 是 UGI。两个 catalog
如果共享同一个 UGI，就会共享同一个 FileSystem，而 `close()` 只能关闭自己 UGI 下的。之前 scan planning 路径打开的
FileSystem 缓存在 FE login user 下，没有任何 connector 能关闭它，每个 catalog 配置来来去去都留下 S3
client 和 executor 线程，最终 OOM。
解决方案：per-configuration digest 生成唯一 UGI → 引用计数 → 最后一个 holder 负责
`FileSystem.closeAllForUGI` → 异步 teardown 不阻塞 journal replay 线程。
**这个设计是正确的。** 特别是 key 基于配置 digest 而非 catalog id，这意味着相同配置的两个 catalog 共享
scope（这是期望行为，因为 Hadoop 的 FileSystem 缓存也是按 UGI 共享的），而 ALTER CATALOG 重建的
connector 落在同一个 scope 上而不是创建新的。
#### 2. HiveConnector close() 的 "detach under monitor, close outside" 模式
解决了 ALTER CATALOG 后 statement 仍能触发 sibling 重建导致资源泄漏的问题。`closed` flag 在
synchronized block 内设置，所有 `getOrCreate*` 在 monitor 内 re-check，确保 close() 和 lazy
build 互斥。close 本身在 monitor 外执行（避免在 CatalogMgr write lock 下做网络
IO）。三个阶段（cache、iceberg sibling、hudi sibling）独立执行，每个失败都 suppress 到第一个异常上，保证 hudi
的 FS_SCOPES 引用计数释放不会被 iceberg 的 REST 连接超时跳过。
**这是正确的。** 特别是 "every stage runs even when an earlier one throws" 这个性质——之前
iceberg close 抛异常就直接 return 了，hudi 的 FS_SCOPES 释放永远不会执行。
#### 3. `listsPartitionsAtSnapshot` SPI — 正确且必要
time-travel 查询之前总是用空分区集（scan-all），因为 LATEST 的分区列表对 snapshot 来说在两个方向上都是错的。新增的
SPI 方法让 connector 声明"我的 listPartitions 在 snapshot-applied handle 上返回的是
AT-SNAPSHOT 的分区集"，fe-core 才敢把真实分区集 pin 上去。
Hudi 的实现 `!useHiveSyncPartition()` 也是正确的——HMS 持有的是 NOW 的分区，一个在 pin 之后被 drop
且 unsync 的分区不在 HMS 中，用它来 prune 会丢数据。
---
### 发现的问题
#### P1-1: `HudiConnector.FS_SCOPES` 的 `fileSystemScopeKey` 碰撞风险
```java
private static final Map<String, ScopeEntry> FS_SCOPES = new HashMap<>();
```
key 是配置属性的 digest。如果两个 catalog 的配置**不完全相同**但 digest 碰撞了，它们会共享同一个 UGI 和
FileSystem——这会导致一个 catalog close 时关闭另一个正在使用的 FileSystem。
虽然 SHA-256 碰撞概率极低，但代码中**没有看到 digest 算法的选择和碰撞处理的说明**。建议在 `fileSystemScopeKey`
的 javadoc 中明确说明使用的算法和碰撞后果，并在 `ScopeEntry` 中保存一份配置摘要用于 debug 时的区分。
**严重程度**：P1 — 碰撞后果是数据丢失（FileSystem 被关闭），而不仅仅是异常。
#### P2-1: `everySiblingDelegationInTheSourceIsClassified` 测试的脆弱性
```java
private static final Pattern METHOD_DECLARATION =
Pattern.compile("^
(?!@)(?!//)(?!\\*)[A-Za-z_].*?\\b(\\w+)\\s*\\(");
private static final Pattern SIBLING_RESOLVER = Pattern.compile(
"\\b(?:siblingMetadata|icebergSiblingMetadata|hudiSiblingMetadata|memoizedSiblingMetadata)\\s*\\(");
```
这个测试通过正则扫描源码来确保所有 sibling 转发方法都被测试覆盖。设计意图很好（completeness lock），但：
- 方法声明的正则只匹配 4 空格缩进的成员方法，如果代码格式变化（比如方法在 lambda 内部、或者缩进变化）就会漏检
- `Assertions.assertTrue(delegating.size() > 30)` 的 guard 只检查数量，不检查质量
这是一个**好的尝试但有维护风险**的测试。如果源码重构导致方法声明格式变化，测试会静默失效。
#### P2-2: `HudiConnector.close()` 签名变更
```java
public void close() throws IOException {
```
`Connector.close()` 现在声明 `throws IOException`。这是一个接口级别的 breaking change。所有实现
`Connector` 接口的类（包括测试中的 mock/fake）都需要更新。考虑到 `Connector` 接口在 `fe-connector-spi`
中且 SPI major 版本已经升到 7，这应该是预期的，但需要确认所有内部实现都已更新。
#### P2-3: `parseInstantOrZero` 的静默降级
```java
private static long parseInstantOrZero(String instant) {
try {
return Long.parseLong(instant);
} catch (NumberFormatException e) {
return 0L;
}
}
```
对于无法解析的 instant，返回 0 意味着 "no reliable change
signal"。这是安全的（降级到不刷新），但**没有任何日志**。如果用户传了一个格式错误的
instant，他们会看到一个永远不过期的缓存，且没有任何提示。建议至少加一个 `LOG.debug` 级别的日志。
#### P3-1: 注释长度
多处注释非常长（`HiveConnector.closed` 字段 12 行、`HudiConnector.FS_SCOPES` 30+
行、`FS_SCOPE_CLOSER` 20+ 行）。这些注释的质量很高，清楚地解释了 "why" 和 "what could go
wrong"。但从可维护性角度看，30 行的 inline 注释在代码演进时容易与实际代码脱节。建议对超过 10 行的设计说明，考虑提取到设计文档或 ADR
中，inline 注释保留核心结论和引用。
---
### 测试覆盖
**测试是这个 PR 最强的部分。** 几个亮点：
1. **`HudiConnectorFileSystemScopeTest`** — 覆盖了引用计数的所有边界：共享
scope、double-close、never-used connector、closed-then-asked。每个 case 用不同的 marker
property 避免静态 map 跨 case 污染。
2.
**`HiveConnectorSiblingTest.everyStageIsClosedEvenWhenAnEarlierOneThrows`** —
直接验证了之前 iceberg close 失败导致 hudi FS_SCOPES 泄漏的 bug 被修复。
3.
**`HiveConnectorMetadataSiblingDelegationTest.everySiblingDelegationInTheSourceIsClassified`**
— 源码扫描式 completeness lock，虽然脆弱但有效。
4.
**`PluginDrivenMvccExternalTableTest.testTimeTravelPinsTheRealPartitionSetOfASnapshotAwareConnector`**
— 验证了 pinned listing 被 PINNED schema 类型化（STRING dt），而非 latest schema（DATEV2
dt），通过 PartitionKey 中的类型差异来证明。
---
### 总结
| 类别 | 评价 |
|------|------|
| 架构设计 | 优秀。FS_SCOPES 引用计数、close() 生命周期加固、listsPartitionsAtSnapshot SPI
都解决了真实问题 |
| 正确性 | 高。并发控制（detach-under-monitor/close-outside）、引用计数的 acquire/release
配对、pinned listing 的类型安全都有充分论证 |
| 测试覆盖 | 优秀。mutation-style 测试、completeness lock、边界条件覆盖全面 |
| 注释质量 | 过高。信息密度高但长度也高，部分 inline 注释应提取到设计文档 |
| 风险点 | `fileSystemScopeKey` 碰撞后果严重（无 debug 手段）；`parseInstantOrZero` 静默降级无日志
|
**P1 需要关注**：`fileSystemScopeKey` 应在文档中明确算法和碰撞处理策略，建议在 `ScopeEntry`
中保存配置摘要用于故障排查。
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
