wenzhenghu commented on PR #65126:
URL: https://github.com/apache/doris/pull/65126#issuecomment-4910589157
workbuddy + glm 5.2：
### 一、总体评价
这是一个**高质量的外部元数据缓存重构**。将旧的 `MetaCache<T>`（包装两个 `LoadingCache`）拆分为独立的
`MetaCacheEntry<K,V>`，配合不可变的 `NameCacheValue` 快照，架构清晰，并发控制严谨。**未发现 blocking
issue**，可合入。
### 二、架构与优点
**1. 缓存条目独立化**
每个逻辑数据集（databaseNames / databases / tableNames / tables）成为独立
`MetaCacheEntry`，职责单一，TTL/容量/失效策略可独立配置。旧的 `MetaCache<T>` 双 `LoadingCache` 耦合被消除。
**2. 慢 I/O 不阻塞 Caffeine 同步 load 路径**
`getWithManualLoad` 将慢 miss load 移出 Caffeine 的 `CacheLoader.load`，改用 striped
`loadLock` 去重 + 手动 `data.put`。这使得 `REFRESH CATALOG` 不再被同 key 的慢 schema load
阻塞——回归测试 `test_jdbc_refresh_catalog_schema_refresh_non_blocking` 精确验证了这一点。
**3. 三层并发协调机制设计正确**
`loadLock`（去重）→ `publishLock`（序列化发布）→ `generation`（版本校验）三层配合：
- 锁顺序全局一致（`loadLock → publishLock` 嵌套，无反向获取），**无死锁风险**
- load 在 `publishLock` 之外执行，不阻塞 `put`/`invalidate`
- `generation` 快照保证 invalidate 后 in-flight load 的 stale 结果不写回
- `invalidateAll`/`invalidateIf` 先 `bumpAllGenerations` 覆盖尚未入缓存的 in-flight
load
**4. replay 非阻塞语义**
`getDbForReplay` / `getTableForReplay` 仅走 `getIfPresent`，miss 时不触发同步 load，避免
replayer 线程被远程 I/O 阻塞。
**5. lower-case mode 0/1/2 行为正确**
mode 0（原样）、mode 1（存储小写）、mode 2（比较不敏感）三条路径互斥，mode 2 统一走
`resolveXxxNameFromSnapshot` 做大小写索引查找。
**6. NameCacheValue 不可变 + copy-on-write**
names 列表与 lower-case 索引在同一快照中原子发布，读写无锁竞争。`withName`/`withoutLocalName`
生成新快照，读者始终看到一致状态。
**7. 旧代码完全清理**
`MetaCache.java`、`LegacyMetaCacheFactory.java` 删除后无遗留引用（已全局 grep 确认）。
### 三、问题与建议
**[Minor] 第二次 generation 检查为 test-only 残留**
`MetaCacheEntry.java:332-334`：
```java
putLoadedValueWithoutGenerationBump(key, loaded);
if (generation != generationOf(key)) { // publishLock 持有期间不可能变
removeLoadedValueWithoutGenerationBump(key, loaded);
}
```
此时已持有 `publishLock`，`generation` 不可能被其他线程修改，第二次检查在生产环境恒为 false。该分支仅为配合
`beforeManualCachePutForTest` 测试钩子注入并发 invalidate 而保留。
建议：加注释说明此分支为 test-only 竞态模拟支持，避免被误判为防御式编程。或考虑将竞态验证收敛到测试子类内部，移除生产路径的冗余检查（符合
AGENTS.md "不做防御式编程"原则）。
**[Nit] NameCacheValue.names() 每次深拷贝**
`names()` 每次调用执行 `copyPairs` + `ImmutableList.copyOf`，分配新对象。`ImmutableList`
本身已不可变，若 `Pair` 调用方只读不写，可直接返回内部 `ImmutableList` 避免重复分配。属安全与性能的
trade-off，当前实现可接受。
**[Nit] stats() 混合统计源**
`hitRate`/`evictionCount` 来自 Caffeine（基于 `getIfPresent`
请求），`loadSuccessCount`/`loadFailureCount` 来自手动跟踪（manual miss load + auto
refresh）。两者统计口径不同，展示时可能造成困惑。建议在 `MetaCacheEntryStats` 或注释中说明口径。
**[Nit] Constructor 过载较多**
`MetaCacheEntry` 有 6 个 public constructor + 2 个 static factory。可考虑 Builder
模式统一，但不影响正确性。
### 四、测试覆盖评估
测试覆盖**非常充分**，共 69 个单测 + 2 个回归测试：
| 测试文件 | 数量 | 覆盖重点 |
|---------|------|---------|
| `MetaCacheEntryTest` | 29 | generation 竞态、striped lock 去重、invalidate
后不写回、refresh cancel、compute/put 并发发布 |
| `NameCacheValueTest` | 7 | copy-on-write 不可变、case-insensitive 索引、冲突拒绝 |
| `MetaCacheDeadlockTest` | 1 | 嵌套 invalidateAll + bounded executor 死锁 |
| `ExternalCatalogTest` | 18 | 增量注册/注销、cold/hot miss、name miss refresh
可配置、replay 非阻塞 |
| `ExternalDatabaseTest` | 14 | table 级对称覆盖、mode 0/1/2、replay hot snapshot
miss |
| 回归测试 | 2 | JDBC refresh 非阻塞（debug point 注入慢 load） |
核心竞态场景（invalidate vs in-flight load、put vs concurrent miss、refresh vs
invalidate）均有对应单测。
### 五、核心结论：
- **Approve**，架构重构质量高，并发控制设计正确，无死锁风险
- 唯一值得跟进的是 `MetaCacheEntry.getWithManualLoad` 第二次 generation
检查的注释澄清（Minor，不影响正确性）
- 测试覆盖全面（69 单测 + 2 回归测试），核心竞态场景均有验证
--
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]
