From 4c5b89e5427f16586f4548b699b1e081a00e08e0 Mon Sep 17 00:00:00 2001 From: morningman Date: Mon, 20 Jul 2026 23:55:49 +0800 Subject: [PATCH 01/19] [doc](catalog) design: external partition derived-view second-level cache (connector-view cache + NereidsSortedPartitionsCacheManager wiring) Two-layer cache to restore the partition derived-view caching CACHE-P1 dropped, without breaking SPI isolation or re-introducing the #65659 TOCTOU: - Cache A (below SPI line): ConnectorPartitionViewCache keyed by (db,table,snapshotId,schemaId), kills the remote enumeration. - Cache B (existing fe-core NereidsSortedPartitionsCacheManager): wire external tables in via SupportBinarySearchFilteringPartitions, kills per-query SortedPartitionRanges rebuild = full legacy parity. Version token carried on ConnectorMvccSnapshot.metaVersion (snapshotId/schemaId for MVCC, connector generation for hive). Consulted only at the per-statement freeze / prune; both layers stay TOCTOU-free. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_014ZngdXGBAXNDRe7jA42mAS --- ...external-partition-derived-cache-design.md | 179 ++++++++++++++++++ 1 file changed, 179 insertions(+) create mode 100644 plan-doc/designs/2026-07-20-external-partition-derived-cache-design.md diff --git a/plan-doc/designs/2026-07-20-external-partition-derived-cache-design.md b/plan-doc/designs/2026-07-20-external-partition-derived-cache-design.md new file mode 100644 index 00000000000000..b222a580b39953 --- /dev/null +++ b/plan-doc/designs/2026-07-20-external-partition-derived-cache-design.md @@ -0,0 +1,179 @@ +# 外表分区"派生视图"二级缓存设计（连接器视图缓存 + 接入 NereidsSortedPartitionsCacheManager） + +- 日期：2026-07-20 +- 分支：`branch-catalog-spi` +- 状态：设计已评审通过，待实现计划 +- 关联：CACHE-P1（翻闸弃二级缓存决策）、#65659（分区裁剪 TOCTOU freeze） + +## 1. 问题背景 + +catalog-SPI 翻闸（CACHE-P1 决策）把 legacy fe-core 里引擎特定的"分区派生视图"二级缓存整层删除，改为**每查询直连列举分区**。代价（已登记为 CACHE-P1 偏差、known-degradation）分两块，按引擎轻重不同： + +1. **远程枚举成本**：iceberg 分区表每查询一次 `PARTITIONS` 元数据表扫描（loadTable + 遍历 manifest-list + 全 manifest + avro 解码）；maxcompute 每查询一次 ODPS `getPartitions()`；hive 每查询把 HMS 名单解析成 `ConnectorPartitionInfo`（原始名单已被 `CachingHmsClient` 缓存，此处是解析 CPU）。 +2. **派生视图重建成本**：fe-core 每查询把分区名单构建成 `nameToPartitionItem`（`PartitionItem`），并在 `PruneFileScanPartition` 里用 `SortedPartitionRanges.build()` 重建二分查找结构（O(n) 建 range + O(n log n) 排序）。legacy 里这份 `SortedPartitionRanges` 是缓存复用的（`HiveExternalMetaCache.HivePartitionValues` 构造函数里建一次、跨查询复用），翻闸后变成每查询重建。 + +对**大量分区**表，这两块都显著。本设计恢复这两层缓存，同时满足 SPI 隔离、且不复发 #65659 的 TOCTOU。 + +## 2. 目标 / 非目标 + +**目标** +- 恢复跨查询的分区派生视图缓存，消除上述两块重复成本。 +- 严格满足 SPI 隔离：fe-core 保持连接器无关；连接器侧缓存建在 SPI 线以下。 +- 不复发 #65659 的分区视图查询内分叉（TOCTOU）。 +- 通用框架，全 SPI 引擎（`SPI_READY_TYPES = jdbc, es, trino-connector, max_compute, paimon, iceberg, hms`）可受益；按收益接线（iceberg/paimon/hive/maxcompute 先接，jdbc/es 分区退化可暂 no-op）。 + +**非目标** +- 不引入全局 as-of 快照语义；跨查询沿用 `use_meta_cache` 的 bounded-staleness 契约。 +- 不改变 `PruneFileScanPartition` 的二分裁剪算法（`PartitionPruner.binarySearchFiltering` 保持原样）。 +- 不为带谓词的 `listPartitions`（当前不存在此调用）设计缓存分桶。 + +## 3. 现状梳理（设计所依赖的事实） + +- **每语句冻结层已存在（相当于 Trino 的 per-transaction memoizer）**：hive/iceberg/paimon 都是 `PluginDrivenMvccExternalTable`；分区视图在 `materializeLatest()`（`PluginDrivenMvccExternalTable.java:127`）**每语句只物化一次**，冻结进 `PluginDrivenMvccSnapshot`（存于 `StatementContext.snapshots`，per-statement 实例字段）。`PruneFileScanPartition` 读的是冻结的 `SelectedPartitions`，绝不回连缓存——这是 #65659 TOCTOU 不复发的结构性原因。 +- **裁剪路径 `listPartitions` 永远不带 filter**：全部 4 个调用点（`PluginDrivenExternalTable.java:831/:887`、`PluginDrivenMvccExternalTable.java:271`、`ShowPartitionsCommand.java:300`）传 `Optional.empty()`。 +- **二分裁剪机制在分支中完好且已在用**：`PartitionPruner.pruneWithResult`（`PartitionPruner.java:163`）在 `sortedPartitionRanges.isPresent()` 时走 `binarySearchFiltering`（`:204-209, :255`），由 session 变量 `enableBinarySearchFilteringPartitions` 控制；#65659 合并后 `PruneFileScanPartition` 每查询用 `.or(() -> SortedPartitionRanges.build(nameToPartitionItem))`（`PruneFileScanPartition.java:101-102`）现建 ranges 后照常二分——功能与 legacy 一致，唯缺缓存复用。 +- **fe-core 已有引擎无关、按版本 keyed 的 ranges 缓存**：`NereidsSortedPartitionsCacheManager`（`common/cache/NereidsSortedPartitionsCacheManager.java`），key=`TableIdentifier(catalog,db,table)`，`PartitionCacheContext(tableId, partitionMetaVersion, sortedPartitionRanges)`，`get()`（`:74`）用 `getPartitionMetaVersion(scan)` 校验版本、变更即重建（`:98-102`），`loadCache()`（`:117`）带"插入过频跳过排序"启发式（`:127-131`），软值 + FE 配置 TTL。**OLAP 表已通过 `PruneOlapScanPartition.java:161-168` 在用**。其接口 `SupportBinarySearchFilteringPartitions`（`catalog/SupportBinarySearchFilteringPartitions.java`）javadoc 明言"you can save the partition's meta snapshot id in the CatalogRelation and get the partitions by the snapshot id"——为外表 MVCC 快照场景预留。 +- **外表当前完全绕过它**：`ExternalTable.getSortedPartitionRanges(scan)`（`ExternalTable.java:527`）返回 `Optional.empty()` 空桩，且 #65659 合并后**已无任何调用者**（死代码）。 +- **已有连接器侧缓存基建**：`fe-connector-cache` 的 `CacheSpec`（属性模型 `meta.cache...(enable|ttl-second|capacity)`）+ `MetaCacheEntry`（Caffeine，contextual-only + manual-miss，generation 计数失效）。iceberg 已有 `IcebergPartitionCache`（key=`(TableIdentifier, snapshotId)` → `List` **原始行**）、`IcebergLatestSnapshotCache`（`(snapshotId, schemaId)` 快照钉子）；hive 有 `CachingHmsClient`（缓存 `listPartitionNames`/`getPartitions` 原始 HMS 元数据）。**但派生视图（`ConnectorMvccPartitionView` / `List`）每次重算**。 +- **连接器失效缝**：`Connector.invalidateTable/invalidateDb/invalidateAll/invalidatePartition`（`fe-connector-api/.../Connector.java:312/316/324/336`，默认 no-op，引擎 override）。REFRESH TABLE（`RefreshManager.refreshTableInternal` → `connector.invalidateTable`）/CATALOG（`PluginDrivenExternalCatalog.onRefreshCache` → `connector.invalidateAll`）/DB + HMS 事件（ADD/DROP/REFRESH_PARTITIONS → `connector.invalidatePartition`）已全部 fan-out 到这 4 个钩子。 +- **session=user 凭证隔离**：iceberg 连接器缓存在 `iceberg.rest.session=user` 下被置 `null` 禁用（`check-authz-cache-sharding.sh` 标记守护）。 + +## 4. 方案总览：两层互补缓存 + +``` +Nereids 裁剪 (PruneFileScanPartition.pruneExternalPartitions) + │ nameToPartitionItem = scan.getSelectedPartitions().selectedPartitions // 冻结 (T1) + │ ranges = scan.getSelectedPartitions().sortedPartitionRanges // 冻结 (#65659), 分支恒空 + │ .or(() -> externalTable.getSortedPartitionRanges(scan)) // ← 复活死桩 → 缓存 B + │ .or(() -> Optional.ofNullable(SortedPartitionRanges.build(nameToPartitionItem))) // 最后兜底 + │ → PartitionPruner.pruneWithResult(..., ranges) → binarySearchFiltering // 机制不变 + ▼ +[缓存 B · fe-core 已存在] NereidsSortedPartitionsCacheManager + key = (catalog, db, table) + validity = getPartitionMetaVersion(scan) // = pinned metaVersion, 见 §6 + value = 预建 SortedPartitionRanges + origin = getOriginPartitions(scan) → pinned 快照的冻结 nameToPartitionItem + ── 消除: 每查询重建 SortedPartitionRanges (大量分区 CPU) + ▲ +[Tier-B 冻结 · 已存在] PluginDrivenMvccSnapshot (每语句物化一次, materializeLatest) + ▲ ── 唯一读下层缓存处 + │ +[缓存 A · 新增, SPI 线以下] ConnectorPartitionViewCache (各连接器 final 字段) + key = (db, table, snapshotId, schemaId) + value = List / ConnectorMvccPartitionView + ── 消除: 远程枚举 (iceberg PARTITIONS 扫描 / ODPS getPartitions / hive 名单解析) + ▲ +[remote] iceberg catalog / hive HMS / odps +``` + +对照 Trino：缓存 A = Trino 的"共享 TTL 层"（`SharedHiveMetastoreCache`，跨查询、可陈旧、SPI 线以下）；Tier-B 冻结层 = Trino 的"per-transaction memoizer"（首次触碰即冻结、查询内一致）。区别是 Doris 的 per-statement 冻结层已存在，故只需补共享层 A + 复用已有 ranges 缓存 B。 + +## 5. 缓存 A：`ConnectorPartitionViewCache`（SPI 线以下） + +**放置**：`fe/fe-connector/fe-connector-cache/.../cache/ConnectorPartitionViewCache.java`（共享工具，非 SPI 接口；与 `CachingHmsClient`/`IcebergPartitionCache` 同一构建方式）。各连接器持有一个 final 字段实例。 + +**形态**：内部一个 `MetaCacheEntry`，沿用 contextual-only + manual-miss 并发模型。 +- key `PartitionViewCacheKey`：`(db, table, snapshotId, schemaId)`。**不含 filter**（裁剪路径恒空；带 filter 的 `listPartitions` 由 loader 一行 `if (filter.isPresent()) return uncachedLoad();` 绕过）。 +- value：`List`（iceberg 另可缓存派生的 `ConnectorMvccPartitionView`）。 +- 配置：`CacheSpec` 键 `meta.cache..partition_view.(enable|ttl-second|capacity)`，默认 TTL 86400s、ON、capacity 与同引擎其它 entry 一致。 + +**接入点**（把原本昂贵的枚举+派生 body 包进 `cache.get(key, () -> body)`）： +- iceberg：`IcebergConnectorMetadata.getMvccPartitionView` / `listPartitions`，叠在现有 `IcebergPartitionCache`（raw 行）之上，缓存派生视图。 +- paimon：`getMvccPartitionView` / `listPartitions`，`(snapshotId, schemaId)` keyed。 +- hive：`HiveConnectorMetadata.listPartitions`，`(db, table)` keyed（snapshotId=-1）+ TTL/失效。 +- maxcompute：`listPartitions`，`(db, table)` keyed + TTL/失效。 +- jdbc/es/trino：分区退化/罕见，框架可接，本期 no-op。 + +**失效**：在各连接器已 override 的 `invalidateTable/invalidateDb/invalidateAll/invalidatePartition` 里加 `partitionViewCache.invalidate*`。`invalidatePartition`（SPI 只带 values 不带 name）对本缓存按 `(db,table)` 整表失效（本就是整表视图，粒度天然对齐、偏保守=安全）。iceberg 因 key 带 snapshotId，REFRESH 后新 snapshot 自然 miss，失效为冗余保险。 + +**session=user**：沿用现有规则——`iceberg.rest.session=user` 下该连接器缓存置 `null` 禁用，挂 `check-authz-cache-sharding.sh` 标记。 + +## 6. 缓存 B：接入 `NereidsSortedPartitionsCacheManager` + +**让外表实现 `SupportBinarySearchFilteringPartitions`**（`PluginDrivenMvccExternalTable` 覆盖 hive/iceberg/paimon；`PluginDrivenExternalTable` 覆盖 maxcompute 等非 MVCC）： +- `getOriginPartitions(scan)`：返回该 scan pinned 快照的**冻结 `nameToPartitionItem`**（`PluginDrivenMvccSnapshot.getNameToPartitionItem()`，数据来自缓存 A / 冻结层，非重新远程列举）。 +- `getPartitionMetaVersion(scan)`：版本令牌（见下）。 +- `getPartitionMetaLoadTimeMillis(scan)`：该分区元数据物化时刻（喂"插入过频跳过排序"启发式；取快照 pin / 缓存 A 条目载入时刻）。 + +**复活死桩**：`ExternalTable.getSortedPartitionRanges(scan)` 改为委托 `Env.getCurrentEnv().getSortedPartitionsCacheManager().get(this, scan)`——与 OLAP 的 `PruneOlapScanPartition` 同一套。 + +**裁剪路径接线**：`PruneFileScanPartition.pruneExternalPartitions` 的 ranges 获取改为： +```java +sortedPartitionRanges = scan.getSelectedPartitions().sortedPartitionRanges // 冻结优先 (#65659) + .or(() -> (Optional) externalTable.getSortedPartitionRanges(scan)) // 缓存 B + .or(() -> Optional.ofNullable(SortedPartitionRanges.build(nameToPartitionItem))); // 兜底 +``` +保留 #65659"冻结字段优先"的语义；缓存 B 命中即复用预建 ranges；缓存 B 返回空（跳过排序启发式/禁用）时退回现建；二者皆从 pinned 快照的同一 map 派生。 + +## 7. 版本令牌（缓存 B 正确性与两层联动） + +统一走**已每语句穿到底的 SPI 类型 `ConnectorMvccSnapshot` 上新增的一个 `metaVersion` 字段**（不新增 SPI 方法，只在既有类型加字段）： +- **iceberg / paimon**：`metaVersion = (snapshotId, schemaId)`——不可变、精确（`ConnectorMvccSnapshot.getSnapshotId():55 / getSchemaId():73` 已有；iceberg 由 `loadLatestSnapshotPin`（`IcebergConnectorMetadata.java:1630`）原子捕获）。新快照自然换 key。 +- **hive / 非 MVCC**：`metaVersion = 连接器维护的单调 generation`（连接器内 `ConcurrentHashMap`，`invalidateTable/invalidatePartition` 时 bump、缓存 A reload 时 bump）。廉价 in-memory、精确，且**让缓存 A 与缓存 B 版本联动**（B 的 ranges 绝不比 A 的视图更旧）。 + +`getPartitionMetaVersion(scan)` 从 scan 的 pinned 快照读出该 `metaVersion`（scan 携带 `tableSnapshot`/`scanParams` → `StatementContext.getSnapshot` → pinned `PluginDrivenMvccSnapshot` → `ConnectorMvccSnapshot.metaVersion`）。 + +## 8. 正确性论证（两层皆不复发 #65659 TOCTOU） + +- **缓存 A**：只在 `materializeLatest()` 那一次被读，读出即冻结进 `PluginDrivenMvccSnapshot`；`PruneFileScanPartition` 只读冻结副本，绝不回连缓存 A。同一 key 并发命中返回同一不可变 value。跨查询：MVCC 令牌保证永不把 S+1 当 S；hive 为 bounded-staleness（TTL + 失效钩子，= `use_meta_cache` 契约）。 +- **缓存 B**：`getPartitionMetaVersion(scan)` = pinned 快照/generation，`getOriginPartitions(scan)` 返回同一 pinned 快照的冻结 `nameToPartitionItem`——ranges 与 `nameToPartitionItem` **同源**；版本不符即重建。故 `prunedPartitions ⊆ nameToPartitionItem.keySet()` 恒成立，`PruneFileScanPartition` 的 `Preconditions.checkState(item != null, ...)`（#65659）永不触发。 +- **查询内一致**由既有冻结层保证（物化一次）；**跨查询**由版本令牌（MVCC）/ TTL+失效（hive）保证 bounded staleness。二者组合等价于 Trino 的"共享 TTL 层 + per-transaction memoizer"。 + +## 9. 隔离与配置 + +- **SPI 隔离**：缓存 A 完全在 SPI 线以下（连接器 final 字段，缓存 SPI 类型），fe-core 一行不改。缓存 B 在 fe-core，但 key=`(表)+不透明 SPI 令牌`、`getOriginPartitions` 只消费 SPI 输出、失效走连接器钩子——不碰引擎内部，与 OLAP 同构。 +- **session=user**：缓存 A 沿用"置 null 禁用"。缓存 B 只存 `(表,快照)` 维度 ranges、**不含任何凭证**，与 OLAP 同样跨用户共享安全（同 snapshotId ⇒ 同分区集），不禁用。 +- **配置开关**：缓存 A `meta.cache..partition_view.(enable|ttl-second|capacity)`（默认 ON/86400s），`enable=false`/`ttl=0` 一键关（对应 Trino `cache-partitions=false` 逃生阀）；缓存 B 复用现有 session 变量 `enableBinarySearchFilteringPartitions` + `cacheSortedPartitionIntervalSecond` + FE 配置（`NereidsSortedPartitionsCacheManager` 现有的 `sortedPartitionTableManageNum`/`expireSortedPartitionTableInFeSecond`）。 + +## 10. 失效汇总（复用现成管线，零新增管线） + +| 触发 | 现有 fan-out | 本设计追加 | +|---|---|---| +| REFRESH TABLE / HMS REFRESH_TABLE | `RefreshManager.refreshTableInternal` → `connector.invalidateTable` | 钩子内 `partitionViewCache.invalidateTable` + bump generation；`NereidsSortedPartitionsCacheManager.invalidate(catalog,db,table)` | +| REFRESH CATALOG | `PluginDrivenExternalCatalog.onRefreshCache` → `connector.invalidateAll` | `partitionViewCache.invalidateAll`；ranges 缓存对应库表失效 | +| REFRESH DB | `connector.invalidateDb` | `partitionViewCache.invalidateDb` | +| HMS ADD/DROP/REFRESH_PARTITIONS | `connector.invalidatePartition` | `partitionViewCache.invalidateTable`(整表) + bump generation → 缓存 B 版本自然失配重建 | +| DROP/RENAME TABLE, DROP DB | `connector.invalidateTable/invalidateDb` | 同上 | + +## 11. 测试 + +**单元测试（我方跑）** +- 缓存 A：per-engine miss→hit；按 `(snapshotId,schemaId)` 分桶（不同快照互不串味）；4 钩子失效各自生效；`session=user` 下禁用（返回 null 走直连）；带 filter 绕过。仿 `IcebergPartitionCacheTest`/`CachingHmsClientTest`。 +- 缓存 B：外表实现 `SupportBinarySearchFilteringPartitions` 三方法；版本变更（snapshotId 变 / generation bump）触发重建；死桩 `getSortedPartitionRanges` 委托生效；`getOriginPartitions` 与冻结 map 同源 → `prunedPartitions ⊆ keySet`，`Preconditions` 不触发；"插入过频跳过排序"启发式。 +- `PruneFileScanPartition`：二分裁剪结果与 legacy/兜底 `build()` 一致的回归（同一谓词、同一分区集 → 同一 pruned 集）。 +- 复用/借鉴 `BinarySearchPartitionInconsistencyTest`（#65659）验证冻结一致性不被破坏。 + +**e2e（用户跑）** +- 大量分区外表重复查询：第二次起命中缓存 A（无远程枚举）、命中缓存 B（无 ranges 重建）。 +- `ALTER TABLE ADD/DROP PARTITION` 后失效可见（下次查询反映新分区集）。 +- iceberg 时间旅行 `FOR VERSION/TIME AS OF` 不串味（不同 snapshotId 独立缓存）。 +- `set enableBinarySearchFilteringPartitions=false` / `meta.cache..partition_view.enable=false` 回退行为。 +- `REFRESH TABLE`/`REFRESH CATALOG` 立即生效。 + +## 12. 风险与回滚 + +- 缓存 A 纯旁路：`enable=false` 即回到当前每查询枚举行为。 +- 缓存 B 是复用既有 OLAP 机制 + 复活死桩：`enableBinarySearchFilteringPartitions=false` 即回到 #65659 的每查询 `build()` 行为。 +- 均不改冻结层、不动 `PartitionPruner`/`PruneFileScanPartition` 的裁剪算法，故不影响 #65659 的正确性结论。 +- 主要风险面 = 失效遗漏导致 bounded staleness 超预期；缓解 = 两层失效均挂同一批已验证的连接器钩子 + TTL 上限。 + +## 13. 影响文件清单（预估） + +**新增** +- `fe/fe-connector/fe-connector-cache/.../cache/ConnectorPartitionViewCache.java`（+ 测试） +- `fe/fe-connector/fe-connector-cache/.../cache/PartitionViewCacheKey.java` + +**修改** +- SPI：`fe-connector-*/.../ConnectorMvccSnapshot`（加 `metaVersion` 字段 + builder）。 +- iceberg：`IcebergConnector`（持有缓存 A + session=user 置 null + 4 钩子）、`IcebergConnectorMetadata`（`getMvccPartitionView`/`listPartitions` 包缓存、`beginQuerySnapshot` 填 `metaVersion`）。 +- paimon：`PaimonConnector`/`PaimonConnectorMetadata` 同构。 +- hive：`HiveConnector`（缓存 A + generation map + 钩子）、`HiveConnectorMetadata`（`listPartitions` 包缓存、`beginQuerySnapshot` 填 generation）。 +- maxcompute：`MaxComputeConnector`/`MaxComputeConnectorMetadata` 同 hive 模式。 +- fe-core：`ExternalTable`（`getSortedPartitionRanges` 委托 `NereidsSortedPartitionsCacheManager`）、`PluginDrivenMvccExternalTable`/`PluginDrivenExternalTable`（实现 `SupportBinarySearchFilteringPartitions` 三方法）、`PruneFileScanPartition`（ranges 获取链加缓存 B）。 +- 失效：各连接器 `invalidate*` 钩子内追加两层失效调用。 + +## 14. 遗留 / follow-up（本设计不做） + +- jdbc/es/trino 连接器接入缓存 A（分区退化，收益低，待需求驱动）。 +- 若 profiling 显示某引擎 `getMvccPartitionView` 派生（dedup+buildRange+merge）仍是瓶颈，可在连接器内进一步拆分缓存粒度。 +- `use_meta_cache=false` 场景下两层缓存的一致行为（当前设计：`use_meta_cache` 为 catalog 级不可变属性，缓存开关独立于它，需在实现时确认交互并在 e2e 覆盖）。 From 90c92907d4dd66216fd9c57d679f12ffc05972ec Mon Sep 17 00:00:00 2001 From: morningman Date: Mon, 20 Jul 2026 23:59:24 +0800 Subject: [PATCH 02/19] [doc](catalog) partition-cache design: drop use_meta_cache=false handling (always true under new SPI) Per review: use_meta_cache is invariantly true in the new SPI framework, so no direct-read bypass path needs designing. Moved from a follow-up to an explicit non-goal; SHOW PARTITIONS/TVF freshness stays on the orthogonal per-call fresh path. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_014ZngdXGBAXNDRe7jA42mAS --- .../2026-07-20-external-partition-derived-cache-design.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plan-doc/designs/2026-07-20-external-partition-derived-cache-design.md b/plan-doc/designs/2026-07-20-external-partition-derived-cache-design.md index b222a580b39953..274e5d51785257 100644 --- a/plan-doc/designs/2026-07-20-external-partition-derived-cache-design.md +++ b/plan-doc/designs/2026-07-20-external-partition-derived-cache-design.md @@ -23,9 +23,10 @@ catalog-SPI 翻闸（CACHE-P1 决策）把 legacy fe-core 里引擎特定的"分 - 通用框架，全 SPI 引擎（`SPI_READY_TYPES = jdbc, es, trino-connector, max_compute, paimon, iceberg, hms`）可受益；按收益接线（iceberg/paimon/hive/maxcompute 先接，jdbc/es 分区退化可暂 no-op）。 **非目标** -- 不引入全局 as-of 快照语义；跨查询沿用 `use_meta_cache` 的 bounded-staleness 契约。 +- 不引入全局 as-of 快照语义；跨查询采用 bounded-staleness 契约（TTL + 失效钩子）。 - 不改变 `PruneFileScanPartition` 的二分裁剪算法（`PartitionPruner.binarySearchFiltering` 保持原样）。 - 不为带谓词的 `listPartitions`（当前不存在此调用）设计缓存分桶。 +- **不考虑 `use_meta_cache=false`**：新 SPI 框架下 `use_meta_cache` **恒为 true**（连接器始终经元数据缓存），无直连绕过路径需设计。SHOW PARTITIONS / partitions-TVF 的"取新"仍由既有 per-call fresh 路径（`listPartitionNamesFresh` 等）承担，与本缓存正交、绕过缓存 A。 ## 3. 现状梳理（设计所依赖的事实） @@ -176,4 +177,3 @@ sortedPartitionRanges = scan.getSelectedPartitions().sortedPartitionRanges - jdbc/es/trino 连接器接入缓存 A（分区退化，收益低，待需求驱动）。 - 若 profiling 显示某引擎 `getMvccPartitionView` 派生（dedup+buildRange+merge）仍是瓶颈，可在连接器内进一步拆分缓存粒度。 -- `use_meta_cache=false` 场景下两层缓存的一致行为（当前设计：`use_meta_cache` 为 catalog 级不可变属性，缓存开关独立于它，需在实现时确认交互并在 e2e 覆盖）。 From cccdc268e6df36b0dd4d84dabcff5519688de2ce Mon Sep 17 00:00:00 2001 From: morningman Date: Tue, 21 Jul 2026 00:06:24 +0800 Subject: [PATCH 03/19] =?UTF-8?q?[doc](catalog)=20impl=20plan:=20Cache=20B?= =?UTF-8?q?=20=E2=80=94=20external=20SortedPartitionRanges=20reuse=20(iceb?= =?UTF-8?q?erg+paimon)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit First of the partition-cache plans (Cache B / ranges). Wires iceberg+paimon into the existing NereidsSortedPartitionsCacheManager via SupportBinarySearchFilteringPartitions (snapshot-keyed), revives the dead ExternalTable.getSortedPartitionRanges stub, and consults the cache in PruneFileScanPartition before build-fresh. 5 TDD tasks, fe-core only, no SPI change. Cache A (connector-view) and hive/maxcompute generation token are separate plans. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_014ZngdXGBAXNDRe7jA42mAS --- ...partition-ranges-cache-B-iceberg-paimon.md | 419 ++++++++++++++++++ 1 file changed, 419 insertions(+) create mode 100644 plan-doc/plans/2026-07-21-partition-ranges-cache-B-iceberg-paimon.md diff --git a/plan-doc/plans/2026-07-21-partition-ranges-cache-B-iceberg-paimon.md b/plan-doc/plans/2026-07-21-partition-ranges-cache-B-iceberg-paimon.md new file mode 100644 index 00000000000000..43122849289dd6 --- /dev/null +++ b/plan-doc/plans/2026-07-21-partition-ranges-cache-B-iceberg-paimon.md @@ -0,0 +1,419 @@ +# Cache B — External SortedPartitionRanges Reuse (iceberg + paimon) Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Restore cross-query caching of the pre-built `SortedPartitionRanges` for MVCC external tables (iceberg, paimon) by wiring them into the existing fe-core `NereidsSortedPartitionsCacheManager`, eliminating the per-query rebuild for many-partition pruning — without SPI changes or re-introducing the #65659 TOCTOU. + +**Architecture:** External MVCC tables implement the existing `SupportBinarySearchFilteringPartitions` interface (the same one OLAP tables use), sourcing the version token from the already-pinned `ConnectorMvccSnapshot(snapshotId, schemaId)` and the origin partition map from the frozen per-statement `PluginDrivenMvccSnapshot`. The dead `ExternalTable.getSortedPartitionRanges` stub is revived to delegate to `NereidsSortedPartitionsCacheManager`, and `PruneFileScanPartition` consults it before its build-fresh fallback. Because the cache is keyed by the pinned snapshot and reads the frozen map, ranges stay consistent with `nameToPartitionItem` (no TOCTOU). + +**Tech Stack:** Java 8, Maven (Doris fe reactor), JUnit 5, Caffeine (via existing `NereidsSortedPartitionsCacheManager`). + +## Global Constraints + +- fe-core stays connector-agnostic: no engine-specific (iceberg/paimon) types leak into `ExternalTable`/`PluginDrivenMvccExternalTable` generic methods; the version token is the opaque `(snapshotId, schemaId)` pair from the SPI type `ConnectorMvccSnapshot`. +- Do NOT change the pruning algorithm in `PartitionPruner.binarySearchFiltering`. +- Preserve #65659 semantics: `PruneFileScanPartition` reads the frozen `scan.getSelectedPartitions().sortedPartitionRanges` first; the cache is only a fallback source before `SortedPartitionRanges.build(...)`. +- Gated by the existing session variable `enableBinarySearchFilteringPartitions` (already checked inside `NereidsSortedPartitionsCacheManager.get`). +- `use_meta_cache` is invariantly `true`; no bypass path. +- Scope of THIS plan: iceberg + paimon only (true MVCC with a real `snapshotId`). hive/maxcompute (which need a connector generation token) and Cache A (connector-view cache) are separate plans. +- Build/test invocation (per repo gotchas): run fe-core unit tests with + `mvn -f /mnt/disk1/yy/git/doris/fe/pom.xml -pl fe-core -am surefire:test -Dtest= -DfailIfNoTests=false -Dmaven.build.cache.enabled=false` + (a full `mvn -f fe/pom.xml install -DskipTests -Dmaven.build.cache.enabled=false` once first, so deps + `${revision}` resolve). + +--- + +## File Structure + +- `fe/fe-core/src/main/java/org/apache/doris/datasource/mvcc/PluginDrivenMvccExternalTable.java` — implement `SupportBinarySearchFilteringPartitions` (3 methods) [Task 1]. +- `fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalTable.java` — revive `getSortedPartitionRanges` to delegate to the cache manager [Task 2]. +- `fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/PruneFileScanPartition.java` — insert the cache lookup into the ranges `.or(...)` chain [Task 3]. +- `fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalMetaCacheMgr.java` — invalidate the ranges cache alongside the schema cache on REFRESH/events [Task 4]. +- Tests: `fe/fe-core/src/test/java/org/apache/doris/common/cache/NereidsSortedPartitionsCacheManagerExternalTest.java` [Task 1 & 5]. + +--- + +### Task 1: External MVCC tables implement `SupportBinarySearchFilteringPartitions` + +**Files:** +- Modify: `fe/fe-core/src/main/java/org/apache/doris/datasource/mvcc/PluginDrivenMvccExternalTable.java` (class decl `:89`; add 3 methods near the partition-view section around `:581`) +- Test: `fe/fe-core/src/test/java/org/apache/doris/common/cache/NereidsSortedPartitionsCacheManagerExternalTest.java` (create) + +**Interfaces:** +- Consumes: `SupportBinarySearchFilteringPartitions` (`org.apache.doris.catalog`, methods `getOriginPartitions(CatalogRelation)`, `getPartitionMetaVersion(CatalogRelation)`, `getPartitionMetaLoadTimeMillis(CatalogRelation)`); `PluginDrivenMvccSnapshot.getNameToPartitionItem()`, `.getConnectorSnapshot()`; `ConnectorMvccSnapshot.getSnapshotId()/getSchemaId()`; `MvccUtil.getSnapshotFromContext(TableIf, Optional, Optional)`. +- Produces: `PluginDrivenMvccExternalTable implements SupportBinarySearchFilteringPartitions`; `getPartitionMetaVersion` returns a `String` token `"@"`. + +- [ ] **Step 1: Write the failing test** + +Create `fe/fe-core/src/test/java/org/apache/doris/common/cache/NereidsSortedPartitionsCacheManagerExternalTest.java`. It drives `NereidsSortedPartitionsCacheManager` with a hand-rolled fake table that mimics the external contract (version token switches to force a rebuild), so it needs no connector or FE server. + +```java +// Licensed to the Apache Software Foundation (ASF) under one ... (standard ASF header) +package org.apache.doris.common.cache; + +import org.apache.doris.analysis.PartitionValue; +import org.apache.doris.catalog.Column; +import org.apache.doris.catalog.DatabaseIf; +import org.apache.doris.catalog.ListPartitionItem; +import org.apache.doris.catalog.PartitionItem; +import org.apache.doris.catalog.PartitionKey; +import org.apache.doris.catalog.PrimitiveType; +import org.apache.doris.catalog.SupportBinarySearchFilteringPartitions; +import org.apache.doris.nereids.rules.expression.rules.SortedPartitionRanges; +import org.apache.doris.nereids.trees.plans.algebra.CatalogRelation; +import org.apache.doris.utframe.TestWithFeService; + +import com.google.common.collect.Maps; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +import java.util.Map; +import java.util.Optional; + +public class NereidsSortedPartitionsCacheManagerExternalTest extends TestWithFeService { + + /** Minimal external-style table: version token and origin map are settable to drive cache behavior. */ + private static class FakeExternalTable implements SupportBinarySearchFilteringPartitions { + Object version = "s1@0"; + Map parts = Maps.newHashMap(); + + @Override public Map getOriginPartitions(CatalogRelation scan) { return parts; } + @Override public Object getPartitionMetaVersion(CatalogRelation scan) { return version; } + @Override public long getPartitionMetaLoadTimeMillis(CatalogRelation scan) { return 0L; } + @Override public long getId() { return 1001L; } + @Override public String getName() { return "t"; } + @Override public DatabaseIf getDatabase() { return null; } // manager returns empty when db==null; overridden below + } + + private static PartitionItem listItem(int v) throws Exception { + PartitionKey k = PartitionKey.createListPartitionKeyWithTypes( + java.util.Collections.singletonList(new PartitionValue(String.valueOf(v))), + java.util.Collections.singletonList(new Column("id", PrimitiveType.INT).getType()), false); + return new ListPartitionItem(java.util.Collections.singletonList(k)); + } + + @Test + public void testExternalRangesRebuildOnVersionChange() throws Exception { + NereidsSortedPartitionsCacheManager mgr = new NereidsSortedPartitionsCacheManager(); + // getDatabase()==null short-circuits get() to empty; assert that contract holds so we know the + // manager consults getDatabase()/getPartitionMetaVersion (the wiring points Task 2/3 depend on). + FakeExternalTable t = new FakeExternalTable(); + Optional> r = mgr.get(t, (CatalogRelation) null); + Assertions.assertFalse(r.isPresent(), + "manager must return empty when getDatabase()==null (guards the external wiring contract)"); + } +} +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `mvn -f /mnt/disk1/yy/git/doris/fe/pom.xml -pl fe-core -am surefire:test -Dtest=NereidsSortedPartitionsCacheManagerExternalTest -DfailIfNoTests=false -Dmaven.build.cache.enabled=false` +Expected: COMPILE FAIL — `PluginDrivenMvccExternalTable` does not yet implement the interface referenced conceptually; and the test compiles only once `SupportBinarySearchFilteringPartitions` is importable (it is). Actually this test compiles today; it will PASS trivially. Its purpose is to lock the `getDatabase()==null ⇒ empty` contract. Proceed to implement the real methods on the table and re-point coverage in Task 5. Mark this step done once the test runs GREEN as a contract guard. + +- [ ] **Step 3: Implement the interface on `PluginDrivenMvccExternalTable`** + +Change the class declaration at `:89` to add the interface: +```java +public class PluginDrivenMvccExternalTable extends PluginDrivenExternalTable + implements org.apache.doris.catalog.SupportBinarySearchFilteringPartitions { +``` +Add these three methods next to `getNameToPartitionItems` (around `:581`). Cast the generic `CatalogRelation` to `LogicalFileScan` for the per-reference snapshot selectors (external partition pruning fires only on `LogicalFileScan`): +```java + // ── SupportBinarySearchFilteringPartitions: lets NereidsSortedPartitionsCacheManager cache the + // pre-built SortedPartitionRanges across queries, keyed by the pinned connector snapshot. ── + @Override + public Map getOriginPartitions(CatalogRelation scan) { + // The SAME frozen map the pin exposes — no re-list, so ranges stay consistent with + // nameToPartitionItem (no #65659 TOCTOU). + return getNameToPartitionItems(pinnedSnapshot(scan)); + } + + @Override + public Object getPartitionMetaVersion(CatalogRelation scan) { + ConnectorMvccSnapshot cs = getOrMaterialize(pinnedSnapshot(scan)).getConnectorSnapshot(); + // Opaque version token: iceberg/paimon expose an immutable (snapshotId, schemaId) pair. + return cs.getSnapshotId() + "@" + cs.getSchemaId(); + } + + @Override + public long getPartitionMetaLoadTimeMillis(CatalogRelation scan) { + // No insert-frequency signal for external tables; 0 = always allow sorting (the manager's + // "skip sort if loaded within cacheSortedPartitionIntervalSecond" heuristic never trips). + return 0L; + } + + private Optional pinnedSnapshot(CatalogRelation scan) { + if (scan instanceof LogicalFileScan) { + LogicalFileScan fileScan = (LogicalFileScan) scan; + return MvccUtil.getSnapshotFromContext(this, fileScan.getTableSnapshot(), fileScan.getScanParams()); + } + return MvccUtil.getSnapshotFromContext(this); + } +``` +Add imports if missing: `org.apache.doris.connector.api.mvcc.ConnectorMvccSnapshot`, `org.apache.doris.datasource.mvcc.MvccUtil`, `org.apache.doris.nereids.trees.plans.algebra.CatalogRelation`, `org.apache.doris.nereids.trees.plans.logical.LogicalFileScan`, `java.util.Optional`. + +- [ ] **Step 4: Run test to verify it passes** + +Run: `mvn -f /mnt/disk1/yy/git/doris/fe/pom.xml -pl fe-core -am surefire:test -Dtest=NereidsSortedPartitionsCacheManagerExternalTest -DfailIfNoTests=false -Dmaven.build.cache.enabled=false` +Expected: PASS (compiles with the new interface; contract guard green). + +- [ ] **Step 5: Commit** + +```bash +git add fe/fe-core/src/main/java/org/apache/doris/datasource/mvcc/PluginDrivenMvccExternalTable.java \ + fe/fe-core/src/test/java/org/apache/doris/common/cache/NereidsSortedPartitionsCacheManagerExternalTest.java +git commit -m "[feat](catalog) external MVCC tables implement SupportBinarySearchFilteringPartitions (snapshot-keyed ranges)" +``` + +--- + +### Task 2: Revive `ExternalTable.getSortedPartitionRanges` to delegate to the cache manager + +**Files:** +- Modify: `fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalTable.java:527` (the empty stub) + +**Interfaces:** +- Consumes: `Env.getCurrentEnv().getSortedPartitionsCacheManager()` → `NereidsSortedPartitionsCacheManager.get(SupportBinarySearchFilteringPartitions, CatalogRelation)`. +- Produces: `ExternalTable.getSortedPartitionRanges(CatalogRelation)` returns cached ranges for tables that implement `SupportBinarySearchFilteringPartitions`, else `Optional.empty()`. + +- [ ] **Step 1: Write the failing test** + +Add to `NereidsSortedPartitionsCacheManagerExternalTest`: +```java + @Test + public void testGetSortedPartitionRangesDelegates() throws Exception { + // A non-Support table returns empty; a Support table routes through the manager. + // Uses a subclass hook rather than a live catalog to stay unit-scoped. + Assertions.assertTrue( + new org.apache.doris.datasource.ExternalTable() {}.getSortedPartitionRanges(null).isPresent() + == false, + "base ExternalTable (not Support) yields empty"); + } +``` +(If `ExternalTable` is abstract/needs ctor args, replace the anonymous instance with a minimal stub subclass defined in the test that supplies the required no-arg-safe fields; keep the assertion: non-Support ⇒ empty.) + +- [ ] **Step 2: Run test to verify it fails** + +Run: same `-Dtest=NereidsSortedPartitionsCacheManagerExternalTest ...` +Expected: FAIL to compile or assert until the stub is revived to the delegating form (currently it unconditionally returns `Optional.empty()`, so this specific assertion would actually pass — the meaningful failure is Task 5's integration assertion; keep this as a guard that the empty-for-non-Support branch is preserved). + +- [ ] **Step 3: Rewrite the stub** + +Replace `ExternalTable.getSortedPartitionRanges` (`:519-529` region) with: +```java + /** + * Cross-query cache of the pre-built {@link SortedPartitionRanges} for binary-search partition + * pruning. Tables that implement {@link SupportBinarySearchFilteringPartitions} (external MVCC: + * iceberg/paimon) route through the shared {@link NereidsSortedPartitionsCacheManager}, keyed by the + * pinned connector snapshot; others return empty (the caller falls back to building ranges fresh). + */ + public Optional> getSortedPartitionRanges(CatalogRelation scan) { + if (!(this instanceof SupportBinarySearchFilteringPartitions)) { + return Optional.empty(); + } + Optional> cached = Env.getCurrentEnv().getSortedPartitionsCacheManager() + .get((SupportBinarySearchFilteringPartitions) this, scan); + return (Optional) cached; + } +``` +Add imports if missing: `org.apache.doris.catalog.SupportBinarySearchFilteringPartitions`, `org.apache.doris.common.cache.NereidsSortedPartitionsCacheManager`, `org.apache.doris.nereids.trees.plans.algebra.CatalogRelation`. + +- [ ] **Step 4: Run test to verify it passes** + +Run: same command. Expected: PASS. + +- [ ] **Step 5: Commit** + +```bash +git add fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalTable.java \ + fe/fe-core/src/test/java/org/apache/doris/common/cache/NereidsSortedPartitionsCacheManagerExternalTest.java +git commit -m "[feat](catalog) revive ExternalTable.getSortedPartitionRanges to delegate to NereidsSortedPartitionsCacheManager" +``` + +--- + +### Task 3: Wire the cache into `PruneFileScanPartition`'s ranges chain + +**Files:** +- Modify: `fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/PruneFileScanPartition.java:96-102` + +**Interfaces:** +- Consumes: `externalTable.getSortedPartitionRanges(scan)` (Task 2). +- Produces: ranges preference order `frozen field → cache manager → build-fresh`. + +- [ ] **Step 1: Write the failing test** + +Add to the external test a check that the prune rule prefers the cache. Since driving the full rule needs an FE server, assert the source order via a focused unit on the chain expression is impractical; instead this behavior is covered by the integration Task 5. Mark Step 1 done by adding a code comment TODO-free assertion in Task 5. (No separate failing test here — this task is a 3-line edit validated by Task 5's integration test and by the existing `BinarySearchPartitionInconsistencyTest` staying green.) + +- [ ] **Step 2: Verify current behavior (regression guard)** + +Run: `mvn -f /mnt/disk1/yy/git/doris/fe/pom.xml -pl fe-core -am surefire:test -Dtest=BinarySearchPartitionInconsistencyTest -DfailIfNoTests=false -Dmaven.build.cache.enabled=false` +Expected: PASS (6/6) — this is the #65659 freeze guard we must not break. + +- [ ] **Step 3: Edit the ranges acquisition** + +In `pruneExternalPartitions`, replace the `sortedPartitionRanges` acquisition (`:99-102`): +```java + if (enableBinarySearch && !nameToPartitionItem.isEmpty()) { + sortedPartitionRanges = scan.getSelectedPartitions().sortedPartitionRanges + .or(() -> (Optional) externalTable.getSortedPartitionRanges(scan)) + .or(() -> Optional.ofNullable(SortedPartitionRanges.build(nameToPartitionItem))); + } +``` +(The middle `.or` is new; frozen-field-first and build-fresh-last are unchanged from #65659.) + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `mvn -f /mnt/disk1/yy/git/doris/fe/pom.xml -pl fe-core -am surefire:test -Dtest=BinarySearchPartitionInconsistencyTest -DfailIfNoTests=false -Dmaven.build.cache.enabled=false` +Expected: PASS (6/6) — behavior unchanged when cache is empty; the new source only adds a hit path. + +- [ ] **Step 5: Commit** + +```bash +git add fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/PruneFileScanPartition.java +git commit -m "[feat](catalog) PruneFileScanPartition: consult NereidsSortedPartitionsCacheManager before building ranges" +``` + +--- + +### Task 4: Invalidate the ranges cache on REFRESH TABLE / partition events + +**Files:** +- Modify: `fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalMetaCacheMgr.java` (the `invalidateTable(catalogId, db, table)` path — same place the schema entry is dropped, `:212-216/:355-363`) + +**Interfaces:** +- Consumes: `Env.getCurrentEnv().getSortedPartitionsCacheManager().invalidate(catalogName, dbName, tableName)` (existing method, `NereidsSortedPartitionsCacheManager:70`). +- Produces: ranges cache dropped whenever the external schema/table cache is invalidated (REFRESH TABLE/CATALOG/DB, HMS events all funnel here). + +- [ ] **Step 1: Write the failing test** + +Add to the external test: +```java + @Test + public void testInvalidateEvictsRanges() throws Exception { + NereidsSortedPartitionsCacheManager mgr = new NereidsSortedPartitionsCacheManager(); + // Populate then invalidate by (catalog,db,table); getPartitionCaches() must not contain the key. + // (Populate via a Support table whose getDatabase() returns a stub catalog/db; assert size 0 after invalidate.) + Assertions.assertEquals(0, mgr.getPartitionCaches().estimatedSize(), + "fresh manager is empty; invalidate is a no-op that must not throw"); + mgr.invalidate("ctl", "db", "t"); // must not throw on absent key + } +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `-Dtest=NereidsSortedPartitionsCacheManagerExternalTest` +Expected: PASS as a guard (invalidate on absent key is safe); the real assertion is that the MgR path calls it — verified by reading the edit. Proceed. + +- [ ] **Step 3: Add the invalidation call** + +In `ExternalMetaCacheMgr.invalidateTable(long catalogId, String dbName, String tblName)` (the method that drops the `"schema"` entry), after the schema-cache invalidation, add: +```java + // Also drop the Nereids sorted-partition-ranges cache for this external table so binary-search + // pruning does not serve ranges older than the refreshed metadata. + ExternalCatalog ctl = (ExternalCatalog) Env.getCurrentEnv().getCatalogMgr().getCatalog(catalogId); + if (ctl != null) { + Env.getCurrentEnv().getSortedPartitionsCacheManager().invalidate(ctl.getName(), dbName, tblName); + } +``` +(Use the catalog/db/table NAMES that `NereidsSortedPartitionsCacheManager` keys on — its `TableIdentifier(catalog, db, table)` matches `catalog.getName()`, `database.getFullName()`, `table.getName()` as used in its `get()`.) + +- [ ] **Step 4: Run test to verify it passes** + +Run: `-Dtest=NereidsSortedPartitionsCacheManagerExternalTest` +Expected: PASS. + +- [ ] **Step 5: Commit** + +```bash +git add fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalMetaCacheMgr.java \ + fe/fe-core/src/test/java/org/apache/doris/common/cache/NereidsSortedPartitionsCacheManagerExternalTest.java +git commit -m "[feat](catalog) drop external SortedPartitionRanges cache on table invalidation" +``` + +--- + +### Task 5: Integration coverage — cache hit, version rebuild, no TOCTOU + +**Files:** +- Modify: `fe/fe-core/src/test/java/org/apache/doris/common/cache/NereidsSortedPartitionsCacheManagerExternalTest.java` + +**Interfaces:** +- Consumes: all of Tasks 1–4. + +- [ ] **Step 1: Write the tests** + +Add two tests using a `SupportBinarySearchFilteringPartitions` fake whose `getDatabase()` returns a stub `DatabaseIf` with a stub `CatalogIf` (so `TableIdentifier` builds), populated with a small partition map: +```java + @Test + public void testCacheHitThenRebuildOnVersionChange() throws Exception { + NereidsSortedPartitionsCacheManager mgr = new NereidsSortedPartitionsCacheManager(); + FakeExternalTable t = fakeWithDb(); // helper: getDatabase() returns stub ctl/db "ctl"/"db" + t.parts.put("id=1", listItem(1)); + t.parts.put("id=2", listItem(2)); + + t.version = "s1@0"; + SortedPartitionRanges first = mgr.get(t, null).orElse(null); + Assertions.assertNotNull(first, "ranges built and cached at snapshot s1"); + SortedPartitionRanges hit = mgr.get(t, null).orElse(null); + Assertions.assertSame(first, hit, "same snapshot ⇒ cache hit returns the SAME instance"); + + t.version = "s2@0"; // snapshot advanced (ALTER ADD PARTITION) + t.parts.put("id=3", listItem(3)); + SortedPartitionRanges rebuilt = mgr.get(t, null).orElse(null); + Assertions.assertNotSame(first, rebuilt, "version change ⇒ rebuild"); + Assertions.assertEquals(3, rebuilt.sortedPartitions.size(), "rebuilt from the new partition set"); + } + + @Test + public void testRangesConsistentWithOriginMap() throws Exception { + // The cached ranges are built from getOriginPartitions(scan); every range id must be a key of + // that same map — the invariant PruneFileScanPartition.Preconditions relies on (no TOCTOU). + NereidsSortedPartitionsCacheManager mgr = new NereidsSortedPartitionsCacheManager(); + FakeExternalTable t = fakeWithDb(); + t.parts.put("id=1", listItem(1)); + t.parts.put("id=2", listItem(2)); + SortedPartitionRanges r = mgr.get(t, null).orElse(null); + r.sortedPartitions.forEach(p -> + Assertions.assertTrue(t.parts.containsKey(p.id), "every range id ∈ origin map keys")); + } +``` +Provide the `fakeWithDb()` helper and stub `DatabaseIf`/`CatalogIf` returning names `"db"`/`"ctl"` (only `getFullName()`/`getName()`/`getCatalog()` are exercised by `NereidsSortedPartitionsCacheManager.get`). + +- [ ] **Step 2: Run to verify they fail (before Tasks 1–4 wired) / pass (after)** + +Run: `-Dtest=NereidsSortedPartitionsCacheManagerExternalTest` +Expected after Tasks 1–4: PASS (all cases). + +- [ ] **Step 3: Full fe-core sanity + #65659 guard** + +Run: +```bash +mvn -f /mnt/disk1/yy/git/doris/fe/pom.xml install -DskipTests -Dmaven.build.cache.enabled=false +mvn -f /mnt/disk1/yy/git/doris/fe/pom.xml -pl fe-core -am surefire:test \ + -Dtest=NereidsSortedPartitionsCacheManagerExternalTest,BinarySearchPartitionInconsistencyTest \ + -DfailIfNoTests=false -Dmaven.build.cache.enabled=false +``` +Expected: BUILD SUCCESS; both test classes GREEN. + +- [ ] **Step 4: Commit** + +```bash +git add fe/fe-core/src/test/java/org/apache/doris/common/cache/NereidsSortedPartitionsCacheManagerExternalTest.java +git commit -m "[test](catalog) external SortedPartitionRanges cache: hit, version-rebuild, origin-map consistency" +``` + +--- + +## e2e (user runs; not in this plan's automated scope) + +- Many-partition iceberg/paimon table: repeated identical query → second query reuses cached ranges (no rebuild); verify via profile/timing. +- `ALTER TABLE ... ADD/DROP PARTITION` then query → new partition set reflected (snapshot advanced ⇒ version token changes ⇒ rebuild). +- iceberg time travel `FOR VERSION AS OF` two snapshots in one session → each keyed independently, no cross-contamination. +- `set enable_binary_search_filtering_partitions=false` → cache bypassed, behavior identical to pre-change. +- `REFRESH TABLE` → ranges cache dropped (Task 4), next query rebuilds. + +## Self-review notes (author) + +- Spec coverage: this plan implements §6 (cache B wiring) + §10 (invalidation) + §8 (no-TOCTOU via origin-map-consistency test) of the design, scoped to iceberg/paimon per Global Constraints. Cache A (§5) and hive/maxcompute generation token (§7) are explicitly out of scope → separate plans. +- The `getPartitionMetaLoadTimeMillis` returns 0 (always-sort); revisit if a connector later exposes a cheap load-time for the skip-sort heuristic. +- `getDatabase()` on the external table must return the real `ExternalDatabase` so `TableIdentifier` keys match invalidation (`ExternalTable.getDatabase():352` already does). From 9b988c46d0575738bb54346ecac1c995d2c3c61d Mon Sep 17 00:00:00 2001 From: morningman Date: Tue, 21 Jul 2026 00:52:31 +0800 Subject: [PATCH 04/19] [feat](catalog) external MVCC tables implement SupportBinarySearchFilteringPartitions (snapshot-keyed ranges) PluginDrivenMvccExternalTable (iceberg/paimon) now implements SupportBinarySearchFilteringPartitions, sourcing the version token from the already-pinned ConnectorMvccSnapshot(snapshotId, schemaId) and the origin partition map from the frozen PluginDrivenMvccSnapshot -- so a later NereidsSortedPartitionsCacheManager lookup reads the SAME frozen map the scan uses (no #65659 TOCTOU). Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_014ZngdXGBAXNDRe7jA42mAS --- .../mvcc/PluginDrivenMvccExternalTable.java | 36 +++++- ...tedPartitionsCacheManagerExternalTest.java | 114 ++++++++++++++++++ 2 files changed, 149 insertions(+), 1 deletion(-) create mode 100644 fe/fe-core/src/test/java/org/apache/doris/common/cache/NereidsSortedPartitionsCacheManagerExternalTest.java diff --git a/fe/fe-core/src/main/java/org/apache/doris/datasource/mvcc/PluginDrivenMvccExternalTable.java b/fe/fe-core/src/main/java/org/apache/doris/datasource/mvcc/PluginDrivenMvccExternalTable.java index bb3edc19f44fad..e0adcf77c99d90 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/datasource/mvcc/PluginDrivenMvccExternalTable.java +++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/mvcc/PluginDrivenMvccExternalTable.java @@ -27,6 +27,7 @@ import org.apache.doris.catalog.PartitionKey; import org.apache.doris.catalog.PartitionType; import org.apache.doris.catalog.RangePartitionItem; +import org.apache.doris.catalog.SupportBinarySearchFilteringPartitions; import org.apache.doris.catalog.Type; import org.apache.doris.common.AnalysisException; import org.apache.doris.connector.api.Connector; @@ -54,6 +55,8 @@ import org.apache.doris.mtmv.MTMVSnapshotIdSnapshot; import org.apache.doris.mtmv.MTMVSnapshotIf; import org.apache.doris.mtmv.MTMVTimestampSnapshot; +import org.apache.doris.nereids.trees.plans.algebra.CatalogRelation; +import org.apache.doris.nereids.trees.plans.logical.LogicalFileScan; import com.google.common.base.Preconditions; import com.google.common.collect.Lists; @@ -87,7 +90,7 @@ * degrades MTMV to conservative over-refresh (the partition never matches its prior snapshot).

*/ public class PluginDrivenMvccExternalTable extends PluginDrivenExternalTable - implements MTMVRelatedTableIf, MTMVBaseTableIf, MvccTable { + implements MTMVRelatedTableIf, MTMVBaseTableIf, MvccTable, SupportBinarySearchFilteringPartitions { private static final Logger LOG = LogManager.getLogger(PluginDrivenMvccExternalTable.class); @@ -619,6 +622,37 @@ public Set getPartitionColumnNames(Optional snapshot) { .map(c -> c.getName().toLowerCase()).collect(Collectors.toSet()); } + // ── SupportBinarySearchFilteringPartitions: lets NereidsSortedPartitionsCacheManager cache the + // pre-built SortedPartitionRanges across queries, keyed by the pinned connector snapshot. ── + @Override + public Map getOriginPartitions(CatalogRelation scan) { + // The SAME frozen map the pin exposes — no re-list, so ranges stay consistent with + // nameToPartitionItem (no #65659 TOCTOU). + return getNameToPartitionItems(pinnedSnapshot(scan)); + } + + @Override + public Object getPartitionMetaVersion(CatalogRelation scan) { + ConnectorMvccSnapshot cs = getOrMaterialize(pinnedSnapshot(scan)).getConnectorSnapshot(); + // Opaque version token: iceberg/paimon expose an immutable (snapshotId, schemaId) pair. + return cs.getSnapshotId() + "@" + cs.getSchemaId(); + } + + @Override + public long getPartitionMetaLoadTimeMillis(CatalogRelation scan) { + // No insert-frequency signal for external tables; 0 = always allow sorting (the manager's + // "skip sort if loaded within cacheSortedPartitionIntervalSecond" heuristic never trips). + return 0L; + } + + private Optional pinnedSnapshot(CatalogRelation scan) { + if (scan instanceof LogicalFileScan) { + LogicalFileScan fileScan = (LogicalFileScan) scan; + return MvccUtil.getSnapshotFromContext(this, fileScan.getTableSnapshot(), fileScan.getScanParams()); + } + return MvccUtil.getSnapshotFromContext(this); + } + // ──────────────────── MTMV snapshots ──────────────────── @Override diff --git a/fe/fe-core/src/test/java/org/apache/doris/common/cache/NereidsSortedPartitionsCacheManagerExternalTest.java b/fe/fe-core/src/test/java/org/apache/doris/common/cache/NereidsSortedPartitionsCacheManagerExternalTest.java new file mode 100644 index 00000000000000..26932ef7d61daf --- /dev/null +++ b/fe/fe-core/src/test/java/org/apache/doris/common/cache/NereidsSortedPartitionsCacheManagerExternalTest.java @@ -0,0 +1,114 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.doris.common.cache; + +import org.apache.doris.catalog.DatabaseIf; +import org.apache.doris.catalog.PartitionItem; +import org.apache.doris.catalog.SupportBinarySearchFilteringPartitions; +import org.apache.doris.datasource.CatalogIf; +import org.apache.doris.datasource.ExternalTable; +import org.apache.doris.nereids.rules.expression.rules.SortedPartitionRanges; +import org.apache.doris.nereids.trees.plans.algebra.CatalogRelation; +import org.apache.doris.qe.ConnectContext; +import org.apache.doris.rpc.RpcException; + +import com.google.common.collect.Maps; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; +import org.mockito.Mockito; + +import java.util.Map; +import java.util.Optional; + +/** + * Unit coverage for wiring external MVCC tables (iceberg/paimon) into + * {@link NereidsSortedPartitionsCacheManager}: cache hit/miss/rebuild-on-version-change, origin-map + * consistency (no #65659 TOCTOU), invalidation, and the {@link ExternalTable#getSortedPartitionRanges} + * delegation contract. + * + *

Drives the manager with a Mockito mock of {@link SupportBinarySearchFilteringPartitions} rather than + * a hand-written fake class: the interface extends {@code TableIf}, whose large method surface makes a + * hand-rolled implementer impractical. Only the methods the manager actually calls are stubbed + * ({@code getOriginPartitions}, {@code getPartitionMetaVersion}, {@code getPartitionMetaLoadTimeMillis}, + * {@code getId}, {@code getName}, {@code getDatabase}).

+ * + *

{@link NereidsSortedPartitionsCacheManager#get} dereferences {@code ConnectContext.get() + * .getSessionVariable()} unconditionally once {@code ConnectContext.get() != null}, so every test needs a + * live {@link ConnectContext} (mirrors the lightweight idiom in {@code LogicalFileScanTest}: a plain + * {@code new ConnectContext()} + {@code setThreadLocalInfo()}, no FE server bootstrap).

+ */ +public class NereidsSortedPartitionsCacheManagerExternalTest { + + private static final String CTL = "ctl"; + private static final String DB = "db"; + private static final String TBL = "t"; + + @AfterEach + public void tearDown() { + ConnectContext.remove(); + } + + private static void newLiveConnectContext() { + ConnectContext ctx = new ConnectContext(); + ctx.setThreadLocalInfo(); + } + + /** + * A settable-state mock of {@link SupportBinarySearchFilteringPartitions}: {@link #version} and + * {@link #parts} drive the cache manager's hit/rebuild decision; when {@code withDatabase} is true the + * constructor also stubs a database/catalog pair (names {@link #CTL}/{@link #DB}) so + * {@code TableIdentifier} can build. + */ + private static final class FakeExternalTable { + final SupportBinarySearchFilteringPartitions table = Mockito.mock(SupportBinarySearchFilteringPartitions.class); + Object version = "s1@0"; + Map parts = Maps.newHashMap(); + + @SuppressWarnings({"unchecked", "rawtypes"}) + FakeExternalTable(boolean withDatabase) throws RpcException { + if (withDatabase) { + DatabaseIf db = Mockito.mock(DatabaseIf.class); + CatalogIf catalog = Mockito.mock(CatalogIf.class); + Mockito.when(catalog.getName()).thenReturn(CTL); + Mockito.when(db.getFullName()).thenReturn(DB); + Mockito.when(db.getCatalog()).thenReturn(catalog); + Mockito.when(table.getDatabase()).thenReturn(db); + } + Mockito.when(table.getId()).thenReturn(1001L); + Mockito.when(table.getName()).thenReturn(TBL); + Mockito.when(table.getOriginPartitions(Mockito.any())).thenAnswer(inv -> parts); + Mockito.when(table.getPartitionMetaVersion(Mockito.any())).thenAnswer(inv -> version); + Mockito.when(table.getPartitionMetaLoadTimeMillis(Mockito.any())).thenReturn(0L); + } + } + + // ──────────────────── Task 1: getDatabase()==null guards the external wiring contract ──────────────────── + + @Test + public void testManagerReturnsEmptyWhenDatabaseNull() throws Exception { + newLiveConnectContext(); + NereidsSortedPartitionsCacheManager mgr = new NereidsSortedPartitionsCacheManager(); + FakeExternalTable t = new FakeExternalTable(false); // getDatabase() -> null (unstubbed mock default) + + Optional> r = mgr.get(t.table, (CatalogRelation) null); + + Assertions.assertFalse(r.isPresent(), + "manager must return empty when getDatabase()==null (guards the external wiring contract)"); + } +} From 1bda18cc0849d83d120b46f1af90fa506d770746 Mon Sep 17 00:00:00 2001 From: morningman Date: Tue, 21 Jul 2026 00:55:59 +0800 Subject: [PATCH 05/19] [feat](catalog) revive ExternalTable.getSortedPartitionRanges to delegate to NereidsSortedPartitionsCacheManager The stub unconditionally returned Optional.empty() with zero callers. Revive it: tables that implement SupportBinarySearchFilteringPartitions (external MVCC: iceberg/paimon) now route through the shared NereidsSortedPartitionsCacheManager, keyed by the pinned connector snapshot; every other table keeps returning empty so the caller falls back to building ranges fresh. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_014ZngdXGBAXNDRe7jA42mAS --- .../apache/doris/datasource/ExternalTable.java | 17 +++++++++++++---- ...ortedPartitionsCacheManagerExternalTest.java | 11 +++++++++++ 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalTable.java b/fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalTable.java index ade57ee4262db6..94010de31d700f 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalTable.java +++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalTable.java @@ -21,11 +21,13 @@ import org.apache.doris.catalog.DatabaseIf; import org.apache.doris.catalog.Env; import org.apache.doris.catalog.PartitionItem; +import org.apache.doris.catalog.SupportBinarySearchFilteringPartitions; import org.apache.doris.catalog.TableAttributes; import org.apache.doris.catalog.TableIf; import org.apache.doris.catalog.TableIndexes; import org.apache.doris.common.AnalysisException; import org.apache.doris.common.Pair; +import org.apache.doris.common.cache.NereidsSortedPartitionsCacheManager; import org.apache.doris.common.io.Text; import org.apache.doris.common.io.Writable; import org.apache.doris.common.util.PropertyAnalyzer; @@ -517,15 +519,22 @@ public boolean supportInternalPartitionPruned() { } /** - * Get sorted partition ranges for binary search filtering. - * Subclasses can override this method to provide sorted partition ranges - * for efficient partition pruning. + * Cross-query cache of the pre-built {@link SortedPartitionRanges} for binary-search partition + * pruning. Tables that implement {@link SupportBinarySearchFilteringPartitions} (external MVCC: + * iceberg/paimon) route through the shared {@link NereidsSortedPartitionsCacheManager}, keyed by the + * pinned connector snapshot; others return empty (the caller falls back to building ranges fresh). * * @param scan the catalog relation * @return sorted partition ranges, or empty if not supported */ + @SuppressWarnings({"unchecked", "rawtypes"}) public Optional> getSortedPartitionRanges(CatalogRelation scan) { - return Optional.empty(); + if (!(this instanceof SupportBinarySearchFilteringPartitions)) { + return Optional.empty(); + } + Optional> cached = Env.getCurrentEnv().getSortedPartitionsCacheManager() + .get((SupportBinarySearchFilteringPartitions) this, scan); + return (Optional) cached; } @Override diff --git a/fe/fe-core/src/test/java/org/apache/doris/common/cache/NereidsSortedPartitionsCacheManagerExternalTest.java b/fe/fe-core/src/test/java/org/apache/doris/common/cache/NereidsSortedPartitionsCacheManagerExternalTest.java index 26932ef7d61daf..3995be74b1e110 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/common/cache/NereidsSortedPartitionsCacheManagerExternalTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/common/cache/NereidsSortedPartitionsCacheManagerExternalTest.java @@ -111,4 +111,15 @@ public void testManagerReturnsEmptyWhenDatabaseNull() throws Exception { Assertions.assertFalse(r.isPresent(), "manager must return empty when getDatabase()==null (guards the external wiring contract)"); } + + // ──────────────────── Task 2: ExternalTable.getSortedPartitionRanges delegation ──────────────────── + + @Test + public void testGetSortedPartitionRangesEmptyForNonSupportTable() { + // A plain ExternalTable does not implement SupportBinarySearchFilteringPartitions, so the + // delegation must short-circuit to empty WITHOUT touching Env/the cache manager. + ExternalTable table = new ExternalTable(); + Assertions.assertFalse(table.getSortedPartitionRanges(null).isPresent(), + "base ExternalTable (not Support) yields empty"); + } } From f519751cbb721af89039798d66bdc585aa16fd1e Mon Sep 17 00:00:00 2001 From: morningman Date: Tue, 21 Jul 2026 00:56:38 +0800 Subject: [PATCH 06/19] [feat](catalog) PruneFileScanPartition: consult NereidsSortedPartitionsCacheManager before building ranges Insert externalTable.getSortedPartitionRanges(scan) into the ranges acquisition chain between the frozen scan.getSelectedPartitions() field (checked first, per #65659) and the build-fresh fallback (checked last). The cache is only ever a fallback source, never a replacement for the frozen per-statement snapshot, so the #65659 TOCTOU guard is unaffected -- verified by BinarySearchPartitionInconsistencyTest staying green (6/6). Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_014ZngdXGBAXNDRe7jA42mAS --- .../doris/nereids/rules/rewrite/PruneFileScanPartition.java | 1 + 1 file changed, 1 insertion(+) diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/PruneFileScanPartition.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/PruneFileScanPartition.java index ad09a5307e03a1..45b5af91c1d35e 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/PruneFileScanPartition.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/PruneFileScanPartition.java @@ -99,6 +99,7 @@ private SelectedPartitions pruneExternalPartitions(ExternalTable externalTable, || ctx.getConnectContext().getSessionVariable().enableBinarySearchFilteringPartitions; if (enableBinarySearch && !nameToPartitionItem.isEmpty()) { sortedPartitionRanges = scan.getSelectedPartitions().sortedPartitionRanges + .or(() -> (Optional) externalTable.getSortedPartitionRanges(scan)) .or(() -> Optional.ofNullable(SortedPartitionRanges.build(nameToPartitionItem))); } PartitionPruneResult result = PartitionPruner.pruneWithResult( From b869818f19fc8b89b3e63401f2a84bfa74580f26 Mon Sep 17 00:00:00 2001 From: morningman Date: Tue, 21 Jul 2026 00:59:46 +0800 Subject: [PATCH 07/19] [feat](catalog) drop external SortedPartitionRanges cache on table invalidation ExternalMetaCacheMgr.invalidateTable(catalogId, db, table) now also calls NereidsSortedPartitionsCacheManager.invalidateTable(catalogName, db, table) (same catalog/db/table NAMES the manager keys on) so REFRESH TABLE/CATALOG/DB and HMS-style events -- which all funnel through this one method -- drop the cached ranges alongside the schema entry, instead of serving binary-search ranges older than the refreshed metadata. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_014ZngdXGBAXNDRe7jA42mAS --- .../apache/doris/datasource/ExternalMetaCacheMgr.java | 6 ++++++ ...reidsSortedPartitionsCacheManagerExternalTest.java | 11 +++++++++++ 2 files changed, 17 insertions(+) diff --git a/fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalMetaCacheMgr.java b/fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalMetaCacheMgr.java index e1ff655002044e..ee4017a49eb698 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalMetaCacheMgr.java +++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalMetaCacheMgr.java @@ -213,6 +213,12 @@ public void invalidateTable(long catalogId, String dbName, String tableName) { routeCatalogEngines(catalogId, cache -> safeInvalidate( cache, catalogId, "invalidateTable", () -> cache.invalidateTable(catalogId, dbName, tableName))); + // Also drop the Nereids sorted-partition-ranges cache for this external table so binary-search + // pruning does not serve ranges older than the refreshed metadata. + CatalogIf ctl = getCatalog(catalogId); + if (ctl != null) { + Env.getCurrentEnv().getSortedPartitionsCacheManager().invalidateTable(ctl.getName(), dbName, tableName); + } } public void invalidateTableByEngine(long catalogId, String engine, String dbName, String tableName) { diff --git a/fe/fe-core/src/test/java/org/apache/doris/common/cache/NereidsSortedPartitionsCacheManagerExternalTest.java b/fe/fe-core/src/test/java/org/apache/doris/common/cache/NereidsSortedPartitionsCacheManagerExternalTest.java index 3995be74b1e110..2932a9cc9493e4 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/common/cache/NereidsSortedPartitionsCacheManagerExternalTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/common/cache/NereidsSortedPartitionsCacheManagerExternalTest.java @@ -122,4 +122,15 @@ public void testGetSortedPartitionRangesEmptyForNonSupportTable() { Assertions.assertFalse(table.getSortedPartitionRanges(null).isPresent(), "base ExternalTable (not Support) yields empty"); } + + // ──────────────────── Task 4: invalidate is safe on an absent key ──────────────────── + + @Test + public void testInvalidateEvictsRanges() { + NereidsSortedPartitionsCacheManager mgr = new NereidsSortedPartitionsCacheManager(); + Assertions.assertEquals(0, mgr.getPartitionCaches().estimatedSize(), + "fresh manager is empty; invalidate is a no-op that must not throw"); + Assertions.assertDoesNotThrow(() -> mgr.invalidateTable(CTL, DB, TBL), + "invalidateTable on an absent key must not throw"); + } } From ceb11eebef74b9c627342244cd11a60b0821ce4a Mon Sep 17 00:00:00 2001 From: morningman Date: Tue, 21 Jul 2026 01:03:03 +0800 Subject: [PATCH 08/19] [test](catalog) external SortedPartitionRanges cache: hit, version-rebuild, origin-map consistency Adds testCacheHitThenRebuildOnVersionChange (same snapshot -> cache hit returns the SAME SortedPartitionRanges instance; version change -> rebuild; explicit invalidateTable forces a rebuild) and testRangesConsistentWithOriginMap (every cached range id is a key of the getOriginPartitions(scan) map it was built from -- the no-#65659-TOCTOU invariant PruneFileScanPartition's Preconditions.checkState relies on). Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_014ZngdXGBAXNDRe7jA42mAS --- ...tedPartitionsCacheManagerExternalTest.java | 59 +++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/fe/fe-core/src/test/java/org/apache/doris/common/cache/NereidsSortedPartitionsCacheManagerExternalTest.java b/fe/fe-core/src/test/java/org/apache/doris/common/cache/NereidsSortedPartitionsCacheManagerExternalTest.java index 2932a9cc9493e4..d22e6b58d9f089 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/common/cache/NereidsSortedPartitionsCacheManagerExternalTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/common/cache/NereidsSortedPartitionsCacheManagerExternalTest.java @@ -17,8 +17,13 @@ package org.apache.doris.common.cache; +import org.apache.doris.analysis.PartitionValue; +import org.apache.doris.catalog.Column; import org.apache.doris.catalog.DatabaseIf; +import org.apache.doris.catalog.ListPartitionItem; import org.apache.doris.catalog.PartitionItem; +import org.apache.doris.catalog.PartitionKey; +import org.apache.doris.catalog.PrimitiveType; import org.apache.doris.catalog.SupportBinarySearchFilteringPartitions; import org.apache.doris.datasource.CatalogIf; import org.apache.doris.datasource.ExternalTable; @@ -27,6 +32,7 @@ import org.apache.doris.qe.ConnectContext; import org.apache.doris.rpc.RpcException; +import com.google.common.collect.ImmutableList; import com.google.common.collect.Maps; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Assertions; @@ -69,6 +75,14 @@ private static void newLiveConnectContext() { ctx.setThreadLocalInfo(); } + private static ListPartitionItem listItem(int value) throws Exception { + Column partitionColumn = new Column("id", PrimitiveType.INT); + PartitionValue partitionValue = new PartitionValue(String.valueOf(value)); + PartitionKey partitionKey = PartitionKey.createPartitionKey( + ImmutableList.of(partitionValue), ImmutableList.of(partitionColumn)); + return new ListPartitionItem(ImmutableList.of(partitionKey)); + } + /** * A settable-state mock of {@link SupportBinarySearchFilteringPartitions}: {@link #version} and * {@link #parts} drive the cache manager's hit/rebuild decision; when {@code withDatabase} is true the @@ -133,4 +147,49 @@ public void testInvalidateEvictsRanges() { Assertions.assertDoesNotThrow(() -> mgr.invalidateTable(CTL, DB, TBL), "invalidateTable on an absent key must not throw"); } + + // ──────────────────── Task 5: cache hit / version-rebuild / origin-map consistency ──────────────────── + + @Test + public void testCacheHitThenRebuildOnVersionChange() throws Exception { + newLiveConnectContext(); + NereidsSortedPartitionsCacheManager mgr = new NereidsSortedPartitionsCacheManager(); + FakeExternalTable t = new FakeExternalTable(true); + t.parts.put("id=1", listItem(1)); + t.parts.put("id=2", listItem(2)); + + t.version = "s1@0"; + SortedPartitionRanges first = mgr.get(t.table, (CatalogRelation) null).orElse(null); + Assertions.assertNotNull(first, "ranges built and cached at snapshot s1"); + SortedPartitionRanges hit = mgr.get(t.table, (CatalogRelation) null).orElse(null); + Assertions.assertSame(first, hit, "same snapshot => cache hit returns the SAME instance"); + + t.version = "s2@0"; // snapshot advanced (ALTER ADD PARTITION) + t.parts.put("id=3", listItem(3)); + SortedPartitionRanges rebuilt = mgr.get(t.table, (CatalogRelation) null).orElse(null); + Assertions.assertNotSame(first, rebuilt, "version change => rebuild"); + Assertions.assertEquals(3, rebuilt.sortedPartitions.size(), "rebuilt from the new partition set"); + + // Task 4 wiring: dropping the cache by (catalog, db, table) forces the next get() to rebuild too. + mgr.invalidateTable(CTL, DB, TBL); + SortedPartitionRanges afterInvalidate = mgr.get(t.table, (CatalogRelation) null).orElse(null); + Assertions.assertNotSame(rebuilt, afterInvalidate, + "explicit invalidateTable(catalog, db, table) forces a rebuild on the next get()"); + } + + @Test + public void testRangesConsistentWithOriginMap() throws Exception { + // The cached ranges are built from getOriginPartitions(scan); every range id must be a key of + // that same map -- the invariant PruneFileScanPartition's Preconditions relies on (no TOCTOU). + newLiveConnectContext(); + NereidsSortedPartitionsCacheManager mgr = new NereidsSortedPartitionsCacheManager(); + FakeExternalTable t = new FakeExternalTable(true); + t.parts.put("id=1", listItem(1)); + t.parts.put("id=2", listItem(2)); + + SortedPartitionRanges r = mgr.get(t.table, (CatalogRelation) null).orElse(null); + Assertions.assertNotNull(r); + r.sortedPartitions.forEach(p -> + Assertions.assertTrue(t.parts.containsKey(p.id), "every range id is a key of the origin map")); + } } From d4b38306f6f675d9911fef03ad64b388f00eed53 Mon Sep 17 00:00:00 2001 From: morningman Date: Tue, 21 Jul 2026 02:09:02 +0800 Subject: [PATCH 09/19] [test](catalog) close review gap: drive real PluginDrivenMvccExternalTable + ExternalMetaCacheMgr wiring The 5 existing tests in NereidsSortedPartitionsCacheManagerExternalTest mock SupportBinarySearchFilteringPartitions directly, so they never exercised the NEW production wiring: PluginDrivenMvccExternalTable#getOriginPartitions / getPartitionMetaVersion / pinnedSnapshot (including the LogicalFileScan branch), and ExternalMetaCacheMgr#invalidateTable's new call into the ranges cache. Add two tests that drive the real code: - testPluginDrivenMvccExternalTableRealOriginPartitionsAndVersion: a CALLS_REAL_METHODS mock of PluginDrivenMvccExternalTable pinned at two different @tag snapshots (forcing the version-blind fallback to be ambiguous), then calls getOriginPartitions/getPartitionMetaVersion via a real LogicalFileScan and asserts the t1 pin's data is returned. - testExternalMetaCacheMgrInvalidateTableDropsRangesCacheEntry: populates a real NereidsSortedPartitionsCacheManager, then calls the real ExternalMetaCacheMgr#invalidateTable through a mocked Env, and asserts the ranges-cache entry is dropped. Also reword the comment above the Preconditions.checkState in PruneFileScanPartition (no logic change): it previously implied nameToPartitionItem and sortedPartitionRanges always come from the literal same object, which is only true within a single query -- a cross-query cache hit reuses ranges keyed by the identical (snapshotId, schemaId) token, so content is identical via the MVCC determinism premise, not object identity. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_014ZngdXGBAXNDRe7jA42mAS --- .../rules/rewrite/PruneFileScanPartition.java | 9 +- ...tedPartitionsCacheManagerExternalTest.java | 140 ++++++++++++++++++ 2 files changed, 146 insertions(+), 3 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/PruneFileScanPartition.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/PruneFileScanPartition.java index 45b5af91c1d35e..f53530c2837561 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/PruneFileScanPartition.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/PruneFileScanPartition.java @@ -109,9 +109,12 @@ private SelectedPartitions pruneExternalPartitions(ExternalTable externalTable, for (String name : prunedPartitions) { PartitionItem item = nameToPartitionItem.get(name); - // Both nameToPartitionItem and sortedPartitionRanges now come from the same frozen - // snapshot, so a missing item is an invariant violation rather than a partition to - // skip. Failing here surfaces the bug instead of silently returning a partial scan. + // Within THIS query, nameToPartitionItem and sortedPartitionRanges are built from the same + // frozen map. On a cross-query cache HIT, sortedPartitionRanges instead reuses ranges built by + // an earlier query keyed by the identical (snapshotId, schemaId) version token -- content is + // identical only via the MVCC determinism premise (same version token => same partition set), + // not because the two maps are literally the same object. A missing item here means that + // premise was violated, so fail loud rather than silently returning a partial scan. Preconditions.checkState(item != null, "pruned partition %s is missing in the selected partitions snapshot", name); selectedPartitionItems.put(name, item); diff --git a/fe/fe-core/src/test/java/org/apache/doris/common/cache/NereidsSortedPartitionsCacheManagerExternalTest.java b/fe/fe-core/src/test/java/org/apache/doris/common/cache/NereidsSortedPartitionsCacheManagerExternalTest.java index d22e6b58d9f089..754d89135ff872 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/common/cache/NereidsSortedPartitionsCacheManagerExternalTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/common/cache/NereidsSortedPartitionsCacheManagerExternalTest.java @@ -18,27 +18,42 @@ package org.apache.doris.common.cache; import org.apache.doris.analysis.PartitionValue; +import org.apache.doris.analysis.TableScanParams; import org.apache.doris.catalog.Column; import org.apache.doris.catalog.DatabaseIf; +import org.apache.doris.catalog.Env; import org.apache.doris.catalog.ListPartitionItem; import org.apache.doris.catalog.PartitionItem; import org.apache.doris.catalog.PartitionKey; import org.apache.doris.catalog.PrimitiveType; import org.apache.doris.catalog.SupportBinarySearchFilteringPartitions; +import org.apache.doris.connector.api.mvcc.ConnectorMvccSnapshot; import org.apache.doris.datasource.CatalogIf; +import org.apache.doris.datasource.CatalogMgr; +import org.apache.doris.datasource.ExternalDatabase; +import org.apache.doris.datasource.ExternalMetaCacheMgr; import org.apache.doris.datasource.ExternalTable; +import org.apache.doris.datasource.mvcc.PluginDrivenMvccExternalTable; +import org.apache.doris.datasource.mvcc.PluginDrivenMvccSnapshot; +import org.apache.doris.nereids.StatementContext; import org.apache.doris.nereids.rules.expression.rules.SortedPartitionRanges; +import org.apache.doris.nereids.trees.plans.RelationId; import org.apache.doris.nereids.trees.plans.algebra.CatalogRelation; +import org.apache.doris.nereids.trees.plans.logical.LogicalFileScan; +import org.apache.doris.nereids.trees.plans.logical.LogicalFileScan.SelectedPartitions; import org.apache.doris.qe.ConnectContext; import org.apache.doris.rpc.RpcException; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.Maps; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; +import org.mockito.MockedStatic; import org.mockito.Mockito; +import java.util.Collections; import java.util.Map; import java.util.Optional; @@ -58,6 +73,13 @@ * .getSessionVariable()} unconditionally once {@code ConnectContext.get() != null}, so every test needs a * live {@link ConnectContext} (mirrors the lightweight idiom in {@code LogicalFileScanTest}: a plain * {@code new ConnectContext()} + {@code setThreadLocalInfo()}, no FE server bootstrap).

+ * + *

The two tests at the bottom of this file additionally drive the REAL production wiring the + * FakeExternalTable-based tests above bypass: {@code PluginDrivenMvccExternalTable#getOriginPartitions} + * / {@code #getPartitionMetaVersion} / {@code #pinnedSnapshot} (via a {@code CALLS_REAL_METHODS} mock, + * the same technique as {@code LogicalFileScanTest}), and {@code ExternalMetaCacheMgr#invalidateTable}'s + * call into this manager (via a real {@link NereidsSortedPartitionsCacheManager} instance reached through + * a mocked {@code Env}).

*/ public class NereidsSortedPartitionsCacheManagerExternalTest { @@ -192,4 +214,122 @@ public void testRangesConsistentWithOriginMap() throws Exception { r.sortedPartitions.forEach(p -> Assertions.assertTrue(t.parts.containsKey(p.id), "every range id is a key of the origin map")); } + + // ──────────────────── Real production wiring: PluginDrivenMvccExternalTable ──────────────────── + // + // The tests above drive the manager with a hand-stubbed SupportBinarySearchFilteringPartitions mock + // and never touch PluginDrivenMvccExternalTable, so they miss the NEW wiring in + // getOriginPartitions/getPartitionMetaVersion/pinnedSnapshot (PluginDrivenMvccExternalTable.java + // around :628-651). This test drives those REAL method bodies: Mockito.CALLS_REAL_METHODS runs every + // unstubbed method for real, so only the connector round-trip (loadSnapshot) and the identity fields + // MvccTableInfo needs (getName/getDatabase) are stubbed -- mirroring the technique in + // LogicalFileScanTest#computeOutputBindsThisReferencesOwnVersionNotLatest. + + @Test + public void testPluginDrivenMvccExternalTableRealOriginPartitionsAndVersion() throws Exception { + Map pinnedPartsT1 = Maps.newHashMap(); + pinnedPartsT1.put("id=1", listItem(1)); + ConnectorMvccSnapshot connectorSnapshotT1 = ConnectorMvccSnapshot.builder() + .snapshotId(42L).schemaId(7L).build(); + PluginDrivenMvccSnapshot pinT1 = new PluginDrivenMvccSnapshot( + connectorSnapshotT1, pinnedPartsT1, Maps.newHashMap()); + + // A DIFFERENT pin for the SAME table at a second @tag reference. With two non-default versions + // pinned and no default ("") entry, the version-BLIND lookup (StatementContext#getSnapshot(TableIf)) + // is ambiguous and gives up (see its javadoc); only the version-AWARE lookup that the + // LogicalFileScan branch of pinnedSnapshot uses resolves the exact t1 reference. MUTATION: + // collapsing pinnedSnapshot to the version-blind fallback makes this test observably diverge + // (an unresolved pin sends getOrMaterialize to materializeLatest() on a field-less mock, or the + // assertions below simply see the wrong values). + ConnectorMvccSnapshot connectorSnapshotT2 = ConnectorMvccSnapshot.builder() + .snapshotId(99L).schemaId(3L).build(); + PluginDrivenMvccSnapshot pinT2 = new PluginDrivenMvccSnapshot( + connectorSnapshotT2, Maps.newHashMap(), Maps.newHashMap()); + + // NOTE: table's default answer is CALLS_REAL_METHODS, so every stub below MUST use the + // doReturn(...).when(table)... form (never when(table.foo()).thenReturn(...)) -- the latter would + // evaluate table.foo() for REAL as part of recording the stub, exactly the pitfall Mockito spies + // have, and several of these real bodies dereference fields this field-less mock never set. + PluginDrivenMvccExternalTable table = + Mockito.mock(PluginDrivenMvccExternalTable.class, Mockito.CALLS_REAL_METHODS); + ExternalDatabase database = Mockito.mock(ExternalDatabase.class); + CatalogIf catalog = Mockito.mock(CatalogIf.class); + Mockito.doReturn(TBL).when(table).getName(); + Mockito.doReturn(database).when(table).getDatabase(); + Mockito.when(database.getFullName()).thenReturn(DB); + Mockito.when(database.getCatalog()).thenReturn((CatalogIf) catalog); + Mockito.when(catalog.getName()).thenReturn(CTL); + // Not under test here (see LogicalFileScanTest precedent): bypass its real body, which needs + // schema/catalog wiring this bare mock doesn't carry. + Mockito.doReturn(SelectedPartitions.NOT_PRUNED).when(table).initSelectedPartitions(Mockito.any()); + + TableScanParams tagT1 = new TableScanParams("tag", ImmutableMap.of(), ImmutableList.of("t1")); + TableScanParams tagT2 = new TableScanParams("tag", ImmutableMap.of(), ImmutableList.of("t2")); + Mockito.doReturn(pinT1).when(table).loadSnapshot(Optional.empty(), Optional.of(tagT1)); + Mockito.doReturn(pinT2).when(table).loadSnapshot(Optional.empty(), Optional.of(tagT2)); + + ConnectContext ctx = new ConnectContext(); + StatementContext stmtCtx = new StatementContext(ctx, null); + ctx.setStatementContext(stmtCtx); + ctx.setThreadLocalInfo(); + try { + // Pin via loadSnapshots (not a hand-rolled key) so the version key is computed by the SAME + // function the lookup uses -- the test must not hand-roll a key and accidentally agree with + // itself. + stmtCtx.loadSnapshots(table, Optional.empty(), Optional.of(tagT1)); + stmtCtx.loadSnapshots(table, Optional.empty(), Optional.of(tagT2)); + + LogicalFileScan scan = new LogicalFileScan(new RelationId(1), table, + Collections.singletonList(DB), Collections.emptyList(), + Optional.empty(), Optional.empty(), Optional.of(tagT1), Optional.empty()); + + Map origin = table.getOriginPartitions(scan); + Object version = table.getPartitionMetaVersion(scan); + + Assertions.assertEquals(pinnedPartsT1, origin, + "getOriginPartitions must dispatch through the LogicalFileScan branch of pinnedSnapshot " + + "and return exactly the t1 pin's partition map (not t2's, not latest's)"); + Assertions.assertEquals("42@7", version, + "getPartitionMetaVersion must return @ from the t1 pin's connector " + + "snapshot, dispatched via the same LogicalFileScan branch"); + } finally { + ConnectContext.remove(); + } + } + + // ──────────────────── Real production wiring: ExternalMetaCacheMgr.invalidateTable ──────────────────── + + @Test + public void testExternalMetaCacheMgrInvalidateTableDropsRangesCacheEntry() throws Exception { + // Drives the REAL ExternalMetaCacheMgr.invalidateTable(...) (the new call at + // ExternalMetaCacheMgr.java:217-220), not NereidsSortedPartitionsCacheManager directly, so the + // production wiring between the two managers is exercised end-to-end. + newLiveConnectContext(); + NereidsSortedPartitionsCacheManager rangesCacheMgr = new NereidsSortedPartitionsCacheManager(); + FakeExternalTable t = new FakeExternalTable(true); + t.parts.put("id=1", listItem(1)); + Assertions.assertTrue(rangesCacheMgr.get(t.table, (CatalogRelation) null).isPresent(), + "ranges built and cached before invalidation"); + Assertions.assertEquals(1, rangesCacheMgr.getPartitionCaches().estimatedSize(), + "one entry cached before invalidation"); + + long catalogId = 7L; + Env env = Mockito.mock(Env.class); + CatalogMgr catalogMgr = Mockito.mock(CatalogMgr.class); + CatalogIf catalog = Mockito.mock(CatalogIf.class); + Mockito.when(catalog.getName()).thenReturn(CTL); + Mockito.when(catalogMgr.getCatalog(catalogId)).thenReturn(catalog); + Mockito.when(env.getCatalogMgr()).thenReturn(catalogMgr); + Mockito.when(env.getSortedPartitionsCacheManager()).thenReturn(rangesCacheMgr); + + ExternalMetaCacheMgr metaCacheMgr = new ExternalMetaCacheMgr(true); + try (MockedStatic envStatic = Mockito.mockStatic(Env.class)) { + envStatic.when(Env::getCurrentEnv).thenReturn(env); + metaCacheMgr.invalidateTable(catalogId, DB, TBL); + } + + Assertions.assertEquals(0, rangesCacheMgr.getPartitionCaches().estimatedSize(), + "ExternalMetaCacheMgr.invalidateTable must also drop the " + + "NereidsSortedPartitionsCacheManager entry (ExternalMetaCacheMgr.java:217-220)"); + } } From 5c17b74888046183ca4626e42c12472b0f49ea6c Mon Sep 17 00:00:00 2001 From: morningman Date: Tue, 21 Jul 2026 02:21:43 +0800 Subject: [PATCH 10/19] [fix](catalog) gate Cache B partition-ranges reuse on a real (non-sentinel) MVCC snapshot id PluginDrivenMvccExternalTable#getPartitionMetaVersion keys NereidsSortedPartitionsCacheManager (Cache B) on "@" from the pinned ConnectorMvccSnapshot. Hive's beginQuerySnapshot always pins snapshotId == -1 (hive has no real MVCC snapshot), so for hive that token is a CONSTANT across queries and cannot detect a partition-set change: a cache hit could serve SortedPartitionRanges built from an older partition set than the current pin's nameToPartitionItem (e.g. after a CachingHmsClient TTL refresh with no invalidate event), pruning against a stale range set and dropping partitions from the result. Override getSortedPartitionRanges to return Optional.empty() when the pinned snapshot's snapshotId == -1, so the caller (PruneFileScanPartition) builds ranges fresh every query for hive (and for an empty iceberg/paimon table, which also pins -1 but is already unreachable via the caller's !nameToPartitionItem.isEmpty() guard); a real snapshot id (iceberg/paimon) keeps delegating to super's cache-manager lookup, unchanged. --- .../mvcc/PluginDrivenMvccExternalTable.java | 24 +++++++++ ...tedPartitionsCacheManagerExternalTest.java | 52 +++++++++++++++++++ 2 files changed, 76 insertions(+) diff --git a/fe/fe-core/src/main/java/org/apache/doris/datasource/mvcc/PluginDrivenMvccExternalTable.java b/fe/fe-core/src/main/java/org/apache/doris/datasource/mvcc/PluginDrivenMvccExternalTable.java index e0adcf77c99d90..c6205e723a2a52 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/datasource/mvcc/PluginDrivenMvccExternalTable.java +++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/mvcc/PluginDrivenMvccExternalTable.java @@ -55,6 +55,7 @@ import org.apache.doris.mtmv.MTMVSnapshotIdSnapshot; import org.apache.doris.mtmv.MTMVSnapshotIf; import org.apache.doris.mtmv.MTMVTimestampSnapshot; +import org.apache.doris.nereids.rules.expression.rules.SortedPartitionRanges; import org.apache.doris.nereids.trees.plans.algebra.CatalogRelation; import org.apache.doris.nereids.trees.plans.logical.LogicalFileScan; @@ -645,6 +646,29 @@ public long getPartitionMetaLoadTimeMillis(CatalogRelation scan) { return 0L; } + @Override + public Optional> getSortedPartitionRanges(CatalogRelation scan) { + // Cache B (NereidsSortedPartitionsCacheManager) is keyed by getPartitionMetaVersion(scan), i.e. + // "@". That token only detects a partition-set change when snapshotId is a + // REAL, monotonically-changing value (iceberg/paimon). Hive's beginQuerySnapshot always pins the + // sentinel snapshotId == -1 (hive has no MVCC snapshot), so the token would be the CONSTANT + // "-1@" across queries -- a cache hit could then serve ranges built from an OLDER + // partition set than the current pin's nameToPartitionItem (e.g. after a CachingHmsClient TTL + // refresh with no invalidate event), silently pruning against a stale range set. Skip the cache + // for a -1 (non-MVCC / sentinel) snapshot: return empty so the caller (PruneFileScanPartition) + // builds ranges fresh from THIS query's pin every time, which is always consistent with it -- the + // pre-cache behavior. A real snapshot id (iceberg/paimon) keeps the cache via super's delegation + // to the cache manager. An EMPTY iceberg table also pins snapshotId == -1, but its + // nameToPartitionItem is then empty too, and PruneFileScanPartition's + // "!nameToPartitionItem.isEmpty()" guard already skips ranges entirely -- so gating on -1 here is + // safe for that case as well. + ConnectorMvccSnapshot cs = getOrMaterialize(pinnedSnapshot(scan)).getConnectorSnapshot(); + if (cs.getSnapshotId() == -1L) { + return Optional.empty(); + } + return super.getSortedPartitionRanges(scan); + } + private Optional pinnedSnapshot(CatalogRelation scan) { if (scan instanceof LogicalFileScan) { LogicalFileScan fileScan = (LogicalFileScan) scan; diff --git a/fe/fe-core/src/test/java/org/apache/doris/common/cache/NereidsSortedPartitionsCacheManagerExternalTest.java b/fe/fe-core/src/test/java/org/apache/doris/common/cache/NereidsSortedPartitionsCacheManagerExternalTest.java index 754d89135ff872..5ebae7ed4361e8 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/common/cache/NereidsSortedPartitionsCacheManagerExternalTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/common/cache/NereidsSortedPartitionsCacheManagerExternalTest.java @@ -297,6 +297,58 @@ public void testPluginDrivenMvccExternalTableRealOriginPartitionsAndVersion() th } } + // ──────────────────── Hive sentinel snapshot (-1) must skip Cache B ──────────────────── + // + // getPartitionMetaVersion's token is "@". Hive's beginQuerySnapshot always pins + // snapshotId == -1 (no real MVCC snapshot), so that token is a CONSTANT across queries and cannot + // detect a partition-set change. PluginDrivenMvccExternalTable#getSortedPartitionRanges must therefore + // gate on the pinned connector snapshot's snapshotId and return empty (skip Cache B, build fresh every + // query) rather than delegating to the cache manager. + + @Test + public void testGetSortedPartitionRangesEmptyForSentinelSnapshotId() throws Exception { + Map pinnedParts = Maps.newHashMap(); + pinnedParts.put("id=1", listItem(1)); + // Hive's sentinel: no real MVCC snapshot id. + ConnectorMvccSnapshot sentinelSnapshot = ConnectorMvccSnapshot.builder() + .snapshotId(-1L).schemaId(0L).build(); + PluginDrivenMvccSnapshot pin = new PluginDrivenMvccSnapshot( + sentinelSnapshot, pinnedParts, Maps.newHashMap()); + + PluginDrivenMvccExternalTable table = + Mockito.mock(PluginDrivenMvccExternalTable.class, Mockito.CALLS_REAL_METHODS); + ExternalDatabase database = Mockito.mock(ExternalDatabase.class); + CatalogIf catalog = Mockito.mock(CatalogIf.class); + Mockito.doReturn(TBL).when(table).getName(); + Mockito.doReturn(database).when(table).getDatabase(); + Mockito.when(database.getFullName()).thenReturn(DB); + Mockito.when(database.getCatalog()).thenReturn((CatalogIf) catalog); + Mockito.when(catalog.getName()).thenReturn(CTL); + Mockito.doReturn(SelectedPartitions.NOT_PRUNED).when(table).initSelectedPartitions(Mockito.any()); + + ConnectContext ctx = new ConnectContext(); + StatementContext stmtCtx = new StatementContext(ctx, null); + ctx.setStatementContext(stmtCtx); + ctx.setThreadLocalInfo(); + try { + // B5a implicit query-begin (latest) pin -- mirrors a plain (no @tag/@branch/time-travel) hive scan. + Mockito.doReturn(pin).when(table).loadSnapshot(Optional.empty(), Optional.empty()); + stmtCtx.loadSnapshots(table, Optional.empty(), Optional.empty()); + + LogicalFileScan scan = new LogicalFileScan(new RelationId(1), table, + Collections.singletonList(DB), Collections.emptyList(), + Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty()); + + Optional> ranges = table.getSortedPartitionRanges(scan); + + Assertions.assertFalse(ranges.isPresent(), + "snapshotId == -1 (hive sentinel) must skip Cache B and yield empty, so the caller " + + "(PruneFileScanPartition) builds ranges fresh from this query's pin"); + } finally { + ConnectContext.remove(); + } + } + // ──────────────────── Real production wiring: ExternalMetaCacheMgr.invalidateTable ──────────────────── @Test From 3cb9c5d6076415ae94a253ea3dad675eae8d3c7b Mon Sep 17 00:00:00 2001 From: morningman Date: Tue, 21 Jul 2026 02:27:16 +0800 Subject: [PATCH 11/19] [feature](catalog) add generic ConnectorPartitionViewCache (cache A) framework MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cache A of the external-partition-derived-cache design (design doc 2026-07-20-external-partition-derived-cache-design.md §5): a generic, engine-agnostic cross-query cache for a connector's derived partition view, modeled on IcebergPartitionCache's construction pattern (contextual-only + manual-miss-load MetaCacheEntry, CacheSpec-driven config under meta.cache..partition_view.*). Keyed by PartitionViewCacheKey (db, table, snapshotId, schemaId). Framework only, no consumers yet -- iceberg/paimon/hive/maxcompute wiring is later subsystems. No fe-core changes. --- .../cache/ConnectorPartitionViewCache.java | 110 +++++++ .../cache/PartitionViewCacheKey.java | 100 +++++++ .../ConnectorPartitionViewCacheTest.java | 278 ++++++++++++++++++ 3 files changed, 488 insertions(+) create mode 100644 fe/fe-connector/fe-connector-cache/src/main/java/org/apache/doris/connector/cache/ConnectorPartitionViewCache.java create mode 100644 fe/fe-connector/fe-connector-cache/src/main/java/org/apache/doris/connector/cache/PartitionViewCacheKey.java create mode 100644 fe/fe-connector/fe-connector-cache/src/test/java/org/apache/doris/connector/cache/ConnectorPartitionViewCacheTest.java diff --git a/fe/fe-connector/fe-connector-cache/src/main/java/org/apache/doris/connector/cache/ConnectorPartitionViewCache.java b/fe/fe-connector/fe-connector-cache/src/main/java/org/apache/doris/connector/cache/ConnectorPartitionViewCache.java new file mode 100644 index 00000000000000..cd7f03be986ee7 --- /dev/null +++ b/fe/fe-connector/fe-connector-cache/src/main/java/org/apache/doris/connector/cache/ConnectorPartitionViewCache.java @@ -0,0 +1,110 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.doris.connector.cache; + +import java.util.Collections; +import java.util.Map; +import java.util.Objects; +import java.util.concurrent.ForkJoinPool; +import java.util.function.Supplier; + +/** + * GENERIC (engine-agnostic) cross-query cache of a connector's derived partition view — "cache A" of the + * external-partition-derived-cache design (design doc {@code 2026-07-20-external-partition-derived-cache-design.md} + * §5). It is the generic version of {@code org.apache.doris.connector.iceberg.IcebergPartitionCache}: same + * construction pattern (a contextual-only, manual-miss-load {@link MetaCacheEntry}), same {@link CacheSpec} wiring, + * same invalidation style — but keyed by the engine-agnostic {@link PartitionViewCacheKey} and holding an opaque + * value {@code V} instead of an iceberg-specific raw-partition list, so it has no engine-specific imports and can be + * shared by every connector (iceberg/paimon/hive/maxcompute, wired in later subsystems — this class has NO + * consumers yet). + * + *

Config: {@code meta.cache..partition_view.(enable|ttl-second|capacity)}, default ON / 86400s / + * 1000 entries (matching {@code IcebergPartitionCache}'s {@code DEFAULT_TABLE_CACHE_CAPACITY}). {@code enable=false} + * / {@code ttl-second=0} / {@code capacity=0} each disable the cache (see {@link CacheSpec#isCacheEnabled}): {@link + * #get} then calls the loader on every call, matching {@code IcebergPartitionCache}'s disabled-cache bypass. + * + *

Concurrency: mirrors {@code IcebergPartitionCache} / {@code MaxComputePartitionCache} exactly — the + * entry is contextual-only (no built-in loader; the caller supplies one per {@link #get} call) with manual miss + * load, so a slow remote enumeration runs OUTSIDE Caffeine's compute lock (deduplicated per key by a striped lock) + * on the calling thread, and its exception propagates to the caller unwrapped without poisoning the cache. + */ +public final class ConnectorPartitionViewCache { + + /** {@code meta.cache..partition_view.*} — the property-namespace entry name for this cache. */ + static final String ENTRY_PARTITION_VIEW = "partition_view"; + /** Default TTL: 24h, matching the design doc's stated default and sibling caches' 24h TTL. */ + static final long DEFAULT_TTL_SECOND = 86400L; + /** Default capacity, matching {@code IcebergPartitionCache}'s {@code DEFAULT_TABLE_CACHE_CAPACITY}. */ + static final long DEFAULT_CAPACITY = 1000L; + + private final MetaCacheEntry entry; + + /** + * @param engine engine token for the {@code meta.cache..partition_view.*} property namespace, e.g. + * {@code "iceberg"}/{@code "paimon"}/{@code "hive"}/{@code "max_compute"}. + * @param props the catalog properties; drives the {@link CacheSpec} (enable/ttl-second/capacity). May be + * {@code null}, treated as empty (defaults apply). + */ + public ConnectorPartitionViewCache(String engine, Map props) { + Objects.requireNonNull(engine, "engine can not be null"); + Map properties = props == null ? Collections.emptyMap() : props; + CacheSpec spec = CacheSpec.fromProperties(properties, engine, ENTRY_PARTITION_VIEW, + CacheSpec.of(true, DEFAULT_TTL_SECOND, DEFAULT_CAPACITY)); + // contextual-only (loader == null, supplied per-call by get()) + manual-miss-load, no auto-refresh -- + // identical shape to IcebergPartitionCache / MaxComputePartitionCache's entry construction. + this.entry = new MetaCacheEntry<>(engine + "." + ENTRY_PARTITION_VIEW, null, spec, + ForkJoinPool.commonPool(), false, true, 0L, true); + } + + /** Caching is on only when the resolved {@link CacheSpec} is effectively enabled (see {@link #entry}'s spec). */ + public boolean isEnabled() { + return entry.stats().isEffectiveEnabled(); + } + + /** + * Returns the cached value for {@code key} if present, else runs {@code loader}, caches and returns it. + * Disabled cache -> {@code loader} runs on every call. The loader runs OUTSIDE Caffeine's compute lock + * (single-flight per key) and its exception propagates unwrapped; a failed load is never cached. + */ + public V get(PartitionViewCacheKey key, Supplier loader) { + Objects.requireNonNull(loader, "loader can not be null"); + return entry.get(key, ignored -> loader.get()); + } + + /** Evicts every cached snapshot/schema entry of one (db, table). Backs REFRESH TABLE / table-level invalidation. */ + public void invalidateTable(String db, String table) { + entry.invalidateIf(key -> key.matches(db, table)); + } + + /** Evicts every cached entry of one db (all its tables). Backs REFRESH DATABASE / db-level invalidation. */ + public void invalidateDb(String db) { + entry.invalidateIf(key -> key.matchesDb(db)); + } + + /** Evicts every cached entry. Backs REFRESH CATALOG. */ + public void invalidateAll() { + entry.invalidateAll(); + } + + /** Test-only: current number of cached entries (accurate map membership, not Caffeine's estimate). */ + long size() { + int[] count = {0}; + entry.forEach((key, value) -> count[0]++); + return count[0]; + } +} diff --git a/fe/fe-connector/fe-connector-cache/src/main/java/org/apache/doris/connector/cache/PartitionViewCacheKey.java b/fe/fe-connector/fe-connector-cache/src/main/java/org/apache/doris/connector/cache/PartitionViewCacheKey.java new file mode 100644 index 00000000000000..f00d39465475a6 --- /dev/null +++ b/fe/fe-connector/fe-connector-cache/src/main/java/org/apache/doris/connector/cache/PartitionViewCacheKey.java @@ -0,0 +1,100 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.doris.connector.cache; + +import java.util.Objects; + +/** + * Immutable cache key for {@link ConnectorPartitionViewCache}: {@code (db, table, snapshotId, schemaId)}. + * + *

Engine-agnostic (external-partition-derived-cache design doc §5, "cache A"): a table's derived partition + * view is a pure function of its identity plus the MVCC coordinate it was read at, so pinning that coordinate + * into the key is what makes the cache "always correct" — a new snapshot/schema yields a new key, never a stale + * hit. Non-MVCC engines (hive) or engines without a separate schema version pass {@code snapshotId = -1} / + * {@code schemaId = -1}; the key still holds them, it just means "unversioned" for that axis. + * + *

{@link #matches} / {@link #matchesDb} back {@link ConnectorPartitionViewCache#invalidateTable} / + * {@link ConnectorPartitionViewCache#invalidateDb}, which must drop every snapshot/schema of a (db, table) or + * every table of a db — mirrors the {@code matches}/{@code matchesDb} helpers on the sibling connector caches + * ({@code MaxComputePartitionCache.PartitionKey}, {@code HiveFileListingCache.FileListingKey}). + */ +public final class PartitionViewCacheKey { + private final String db; + private final String table; + private final long snapshotId; + private final long schemaId; + + public PartitionViewCacheKey(String db, String table, long snapshotId, long schemaId) { + this.db = db; + this.table = table; + this.snapshotId = snapshotId; + this.schemaId = schemaId; + } + + public String getDb() { + return db; + } + + public String getTable() { + return table; + } + + public long getSnapshotId() { + return snapshotId; + } + + public long getSchemaId() { + return schemaId; + } + + /** Whether this key belongs to the given (db, table), regardless of snapshotId/schemaId. */ + public boolean matches(String db, String table) { + return Objects.equals(this.db, db) && Objects.equals(this.table, table); + } + + /** Whether this key belongs to the given db, regardless of table/snapshotId/schemaId. */ + public boolean matchesDb(String db) { + return Objects.equals(this.db, db); + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (!(o instanceof PartitionViewCacheKey)) { + return false; + } + PartitionViewCacheKey that = (PartitionViewCacheKey) o; + return snapshotId == that.snapshotId + && schemaId == that.schemaId + && Objects.equals(db, that.db) + && Objects.equals(table, that.table); + } + + @Override + public int hashCode() { + return Objects.hash(db, table, snapshotId, schemaId); + } + + @Override + public String toString() { + return "PartitionViewCacheKey{db=" + db + ", table=" + table + + ", snapshotId=" + snapshotId + ", schemaId=" + schemaId + '}'; + } +} diff --git a/fe/fe-connector/fe-connector-cache/src/test/java/org/apache/doris/connector/cache/ConnectorPartitionViewCacheTest.java b/fe/fe-connector/fe-connector-cache/src/test/java/org/apache/doris/connector/cache/ConnectorPartitionViewCacheTest.java new file mode 100644 index 00000000000000..95a80e5d6f164d --- /dev/null +++ b/fe/fe-connector/fe-connector-cache/src/test/java/org/apache/doris/connector/cache/ConnectorPartitionViewCacheTest.java @@ -0,0 +1,278 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.doris.connector.cache; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +import java.util.HashMap; +import java.util.Map; +import java.util.concurrent.atomic.AtomicInteger; + +/** + * Unit tests for {@link ConnectorPartitionViewCache} — the GENERIC (engine-agnostic) cache A of the + * external-partition-derived-cache design (design doc {@code 2026-07-20-external-partition-derived-cache-design.md} + * §5). Mirrors {@link org.apache.doris.connector.iceberg.IcebergPartitionCache}'s test shape (that class is the + * iceberg-specific ancestor this generic framework is modeled on), but with a generic string value type ({@code V}) + * since this module must not depend on any engine-specific type. Covers: miss-then-hit, distinct-snapshot keying, + * per-table / per-db / whole-cache invalidation, and the disabled-cache bypass (both {@code enable=false} and + * {@code ttl-second=0}). + */ +public class ConnectorPartitionViewCacheTest { + + private static final String ENGINE = "testengine"; + + private static PartitionViewCacheKey key(String db, String table, long snapshotId, long schemaId) { + return new PartitionViewCacheKey(db, table, snapshotId, schemaId); + } + + private static ConnectorPartitionViewCache newCache() { + return new ConnectorPartitionViewCache<>(ENGINE, new HashMap<>()); + } + + @Test + public void missThenHitLoaderRunsOnceWithinTtl() { + AtomicInteger loads = new AtomicInteger(); + ConnectorPartitionViewCache cache = newCache(); + + String first = cache.get(key("db", "t", 5L, 1L), () -> { + loads.incrementAndGet(); + return "view-v1"; + }); + // Second read of the SAME key must be served from cache, NOT re-invoke the loader -- this is the whole + // point of memoizing the expensive partition enumeration across queries. MUTATION: always calling the + // loader -> second != first / loads == 2 -> red. + String second = cache.get(key("db", "t", 5L, 1L), () -> { + loads.incrementAndGet(); + return "view-v2"; + }); + + Assertions.assertEquals("view-v1", first); + Assertions.assertEquals("view-v1", second, "within TTL the cached value must be served, not a fresh load"); + Assertions.assertEquals(1, loads.get(), "the loader must run exactly once for the same key"); + Assertions.assertTrue(cache.isEnabled()); + } + + @Test + public void differentSnapshotIdIsADistinctEntry() { + AtomicInteger loads = new AtomicInteger(); + ConnectorPartitionViewCache cache = newCache(); + + cache.get(key("db", "t", 1L, 1L), () -> { + loads.incrementAndGet(); + return "snap-1"; + }); + // A new commit yields a new snapshotId -> a distinct key -> the loader must run again. MUTATION: keying + // by (db, table) only (ignoring snapshotId/schemaId) -> loads stays 1 / serves the stale snap-1 value. + String second = cache.get(key("db", "t", 2L, 1L), () -> { + loads.incrementAndGet(); + return "snap-2"; + }); + + Assertions.assertEquals("snap-2", second); + Assertions.assertEquals(2, loads.get(), "distinct snapshotId must trigger a distinct load"); + } + + @Test + public void differentSchemaIdIsADistinctEntry() { + AtomicInteger loads = new AtomicInteger(); + ConnectorPartitionViewCache cache = newCache(); + + cache.get(key("db", "t", 1L, 1L), () -> { + loads.incrementAndGet(); + return "schema-1"; + }); + // schemaId is part of the key too (paimon evolves schema independently of snapshot). MUTATION: dropping + // schemaId from equals/hashCode -> this second call would incorrectly hit -> loads stays 1. + String second = cache.get(key("db", "t", 1L, 2L), () -> { + loads.incrementAndGet(); + return "schema-2"; + }); + + Assertions.assertEquals("schema-2", second); + Assertions.assertEquals(2, loads.get(), "distinct schemaId must trigger a distinct load"); + } + + @Test + public void invalidateTableEvictsAllSnapshotsOfThatTableOnly() { + AtomicInteger loads = new AtomicInteger(); + ConnectorPartitionViewCache cache = newCache(); + + cache.get(key("db", "t", 1L, 1L), () -> "v1"); + cache.get(key("db", "t", 2L, 1L), () -> "v2"); + cache.get(key("db", "other", 1L, 1L), () -> "v3"); + + // REFRESH TABLE db.t must drop BOTH snapshot entries of db.t and leave db.other intact. MUTATION: + // invalidating a single (db, table, snapshotId) key instead of every snapshot of the table -> the + // reload below would still hit the stale "v1". + cache.invalidateTable("db", "t"); + + String reload = cache.get(key("db", "t", 1L, 1L), () -> { + loads.incrementAndGet(); + return "v1-reloaded"; + }); + Assertions.assertEquals("v1-reloaded", reload, "invalidateTable must force a fresh load for db.t"); + Assertions.assertEquals(1, loads.get()); + + // db.other must be untouched by the db.t invalidation. + AtomicInteger otherLoads = new AtomicInteger(); + String other = cache.get(key("db", "other", 1L, 1L), () -> { + otherLoads.incrementAndGet(); + return "v3-should-not-reload"; + }); + Assertions.assertEquals("v3", other, "invalidateTable(db, t) must not touch db.other"); + Assertions.assertEquals(0, otherLoads.get()); + } + + @Test + public void invalidateDbClearsOnlyThatDbsTables() { + AtomicInteger loads = new AtomicInteger(); + ConnectorPartitionViewCache cache = newCache(); + + cache.get(key("db1", "t1", 1L, 1L), () -> "a"); + cache.get(key("db1", "t2", 1L, 1L), () -> "b"); + cache.get(key("db2", "t1", 1L, 1L), () -> "c"); + + // REFRESH DATABASE db1 must drop every table of db1 and leave db2 intact. MUTATION: invalidateDb acting + // like invalidateAll (or a no-op) -> either db2 also reloads, or db1 does not. + cache.invalidateDb("db1"); + + String reloadedT1 = cache.get(key("db1", "t1", 1L, 1L), () -> { + loads.incrementAndGet(); + return "a-reloaded"; + }); + Assertions.assertEquals("a-reloaded", reloadedT1); + Assertions.assertEquals(1, loads.get()); + + AtomicInteger db2Loads = new AtomicInteger(); + String db2Value = cache.get(key("db2", "t1", 1L, 1L), () -> { + db2Loads.incrementAndGet(); + return "c-should-not-reload"; + }); + Assertions.assertEquals("c", db2Value, "invalidateDb(db1) must not touch db2"); + Assertions.assertEquals(0, db2Loads.get()); + } + + @Test + public void invalidateAllClearsEveryEntry() { + AtomicInteger loads = new AtomicInteger(); + ConnectorPartitionViewCache cache = newCache(); + + cache.get(key("db1", "t1", 1L, 1L), () -> "a"); + cache.get(key("db2", "t2", 1L, 1L), () -> "b"); + + cache.invalidateAll(); + + String reloaded = cache.get(key("db1", "t1", 1L, 1L), () -> { + loads.incrementAndGet(); + return "a-reloaded"; + }); + Assertions.assertEquals("a-reloaded", reloaded, "invalidateAll must drop every entry"); + Assertions.assertEquals(1, loads.get()); + } + + @Test + public void enableFalseDisablesCacheAlwaysLoads() { + AtomicInteger loads = new AtomicInteger(); + Map props = new HashMap<>(); + props.put("meta.cache." + ENGINE + ".partition_view.enable", "false"); + ConnectorPartitionViewCache cache = new ConnectorPartitionViewCache<>(ENGINE, props); + + String first = cache.get(key("db", "t", 5L, 1L), () -> { + loads.incrementAndGet(); + return "v1"; + }); + // enable=false must bypass the cache entirely -- every call re-invokes the loader. MUTATION: ignoring the + // enable=false property -> second call would return "v1" (cached) / loads would stay 1 -> red. + String second = cache.get(key("db", "t", 5L, 1L), () -> { + loads.incrementAndGet(); + return "v2"; + }); + + Assertions.assertEquals("v1", first); + Assertions.assertEquals("v2", second, "enable=false must call the loader on every get"); + Assertions.assertEquals(2, loads.get()); + Assertions.assertFalse(cache.isEnabled()); + } + + @Test + public void ttlZeroDisablesCacheAlwaysLoads() { + AtomicInteger loads = new AtomicInteger(); + Map props = new HashMap<>(); + props.put("meta.cache." + ENGINE + ".partition_view.ttl-second", "0"); + ConnectorPartitionViewCache cache = new ConnectorPartitionViewCache<>(ENGINE, props); + + cache.get(key("db", "t", 5L, 1L), () -> { + loads.incrementAndGet(); + return "v1"; + }); + String second = cache.get(key("db", "t", 5L, 1L), () -> { + loads.incrementAndGet(); + return "v2"; + }); + + // ttl-second=0 is CacheSpec's explicit disable signal (distinct from -1 = "no expiration"). MUTATION: + // treating ttl=0 as "no expiration" instead of "disabled" -> second call would return the cached "v1". + Assertions.assertEquals("v2", second, "ttl-second=0 must always load live"); + Assertions.assertEquals(2, loads.get()); + Assertions.assertFalse(cache.isEnabled()); + } + + @Test + public void capacityZeroDisablesCacheAlwaysLoads() { + AtomicInteger loads = new AtomicInteger(); + Map props = new HashMap<>(); + props.put("meta.cache." + ENGINE + ".partition_view.capacity", "0"); + ConnectorPartitionViewCache cache = new ConnectorPartitionViewCache<>(ENGINE, props); + + cache.get(key("db", "t", 5L, 1L), () -> { + loads.incrementAndGet(); + return "v1"; + }); + String second = cache.get(key("db", "t", 5L, 1L), () -> { + loads.incrementAndGet(); + return "v2"; + }); + + Assertions.assertEquals("v2", second, "capacity=0 must always load live"); + Assertions.assertEquals(2, loads.get()); + Assertions.assertFalse(cache.isEnabled()); + } + + @Test + public void defaultsAreEnabledWithNoProperties() { + // No meta.cache..partition_view.* properties set at all -> the built-in default (TTL 86400s, + // capacity 1000, ON) applies, matching IcebergPartitionCache's DEFAULT_TABLE_CACHE_CAPACITY. MUTATION: + // defaulting to disabled -> isEnabled() would be false here. + ConnectorPartitionViewCache cache = new ConnectorPartitionViewCache<>(ENGINE, new HashMap<>()); + Assertions.assertTrue(cache.isEnabled(), "the cache must be ON by default with no override properties"); + } + + @Test + public void loaderExceptionPropagatesUnwrappedAndIsNotCached() { + ConnectorPartitionViewCache cache = newCache(); + // A failed load (e.g. a remote enumeration RPC failure) must propagate to the caller verbatim and must + // NOT poison the cache -- a transient failure should not make every subsequent query fail for the TTL. + Assertions.assertThrows(IllegalStateException.class, () -> cache.get(key("db", "t", 5L, 1L), () -> { + throw new IllegalStateException("boom"); + })); + + String recovered = cache.get(key("db", "t", 5L, 1L), () -> "recovered"); + Assertions.assertEquals("recovered", recovered, "a failed load must not be cached"); + } +} From 96d5611b5d4fa8791b147cd7337c80064786512e Mon Sep 17 00:00:00 2001 From: morningman Date: Tue, 21 Jul 2026 02:55:02 +0800 Subject: [PATCH 12/19] [perf](catalog) wire cross-query derived partition-view cache A into the iceberg connector (S3) Layer the generic ConnectorPartitionViewCache (S2 framework) above the raw IcebergPartitionCache so a repeated query on a partitioned iceberg table skips the derived-view BUILD, not just the remote PARTITIONS scan. Two typed per-catalog fields on IcebergConnector -- one for getMvccPartitionView's ConnectorMvccPartitionView and one for listPartitions' List -- because the two SPI hooks return structurally different derived types and neither derives from the other; the shared raw scan underneath is already deduped by IcebergPartitionCache, so two caches never double-scan remotely. - Key = (db, table, snapshotId, schemaId) from the pinned handle (pure function of the MVCC coordinate). getMvccPartitionView and the empty-filter listPartitions pruning path are wrapped; a present filter bypasses the cache (not the pruning path). - session=user: both fields nulled under isUserSessionEnabled(), mirroring the sibling iceberg caches, with the check-authz-cache-sharding.sh marker on each declaration line; a null cache computes directly every call. - Invalidation routed from invalidateTable/invalidateDb/invalidateAll to both caches (null-guarded). Tests: new IcebergConnectorMetadataPartitionViewCacheTest (7) proves same-key-hit enumerates once, distinct-snapshot re-enumerates, invalidateTable/All re-enumerate, filter bypass, and null-cache re-enumeration; IcebergConnectorCacheTest (+3) proves the session=user null-gate and the 4-hook routing on the real fields. Full module suite 1084/1084 green. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_014ZngdXGBAXNDRe7jA42mAS --- .../connector/iceberg/IcebergConnector.java | 59 +++- .../iceberg/IcebergConnectorMetadata.java | 99 ++++++- .../iceberg/IcebergConnectorCacheTest.java | 99 +++++++ ...nnectorMetadataPartitionViewCacheTest.java | 260 ++++++++++++++++++ 4 files changed, 503 insertions(+), 14 deletions(-) create mode 100644 fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/IcebergConnectorMetadataPartitionViewCacheTest.java 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 47fb3dd5abd2ae..95677a9dd8c023 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 @@ -20,14 +20,17 @@ import org.apache.doris.connector.api.Connector; import org.apache.doris.connector.api.ConnectorCapability; import org.apache.doris.connector.api.ConnectorMetadata; +import org.apache.doris.connector.api.ConnectorPartitionInfo; import org.apache.doris.connector.api.ConnectorSession; import org.apache.doris.connector.api.ConnectorTestResult; import org.apache.doris.connector.api.ConnectorValidationContext; import org.apache.doris.connector.api.DorisConnectorException; import org.apache.doris.connector.api.handle.ConnectorTableHandle; +import org.apache.doris.connector.api.mvcc.ConnectorMvccPartitionView; import org.apache.doris.connector.api.procedure.ConnectorProcedureOps; import org.apache.doris.connector.api.scan.ConnectorScanPlanProvider; import org.apache.doris.connector.api.write.ConnectorWritePlanProvider; +import org.apache.doris.connector.cache.ConnectorPartitionViewCache; import org.apache.doris.connector.metastore.HmsMetaStoreProperties; import org.apache.doris.connector.metastore.spi.JdbcDriverSupport; import org.apache.doris.connector.metastore.spi.MetaStoreProviders; @@ -76,6 +79,7 @@ import java.util.Collections; import java.util.EnumSet; import java.util.HashMap; +import java.util.List; import java.util.Locale; import java.util.Map; import java.util.Optional; @@ -181,6 +185,19 @@ public class IcebergConnector implements Connector { // the comment path, and session=user must stay live because the loadTable itself carries per-user // authorization a shared cache would bypass. null for every other flavor. private final IcebergCommentCache commentCache; // authz-cache-session-user-disabled + // PERF-06: cross-query DERIVED partition-view cache ("cache A", the generic ConnectorPartitionViewCache from + // fe-connector-cache), layered ABOVE the raw partitionCache (PERF-02): it memoizes the BUILT derived view + // (transform-to-range math + overlap merge for the MTMV view; the value-map construction for listPartitions) + // keyed by (db, table, snapshotId, schemaId), so a repeated query on a partitioned table skips the derived + // rebuild, not just the remote scan. Two typed fields because the two SPI hooks return structurally different + // derived types and neither derives from the other; the shared raw PARTITIONS scan underneath is already + // deduped by partitionCache, so two derived caches never double-scan remotely. Authorization-sensitive + // projection like partitionCache/formatCache: disabled (null) under iceberg.rest.session=user so a shared + // (no user dimension) hit cannot disclose one user's partition view; kept for every other flavor. + private final ConnectorPartitionViewCache // authz-cache-session-user-disabled + mvccPartitionViewCache; + private final ConnectorPartitionViewCache> // authz-cache-session-user-disabled + listPartitionsViewCache; // Manifest content cache — pure metadata, default-off (meta.cache.iceberg.manifest.enable), and consumed // ONLY after a per-user resolveTable(ForRead). authz-cache-exempt (no read path without a per-user load). private final IcebergManifestCache manifestCache = new IcebergManifestCache(); @@ -254,6 +271,17 @@ public IcebergConnector(Map properties, ConnectorContext context ? new IcebergCommentCache( resolveTableCacheTtlSecond(this.properties), DEFAULT_TABLE_CACHE_CAPACITY) : null; + // PERF-06: derived partition-view cache A (generic ConnectorPartitionViewCache). Same + // authorization-sensitive treatment as partitionCache -- disabled (null) under iceberg.rest.session=user so + // a shared (no user dimension) hit cannot disclose one user's partition view; kept otherwise. Reads its + // own meta.cache.iceberg.partition_view.(enable|ttl-second|capacity) from the catalog properties via the + // framework's CacheSpec (default ON / 24h / 1000). Two typed instances (MVCC view + partition-info list). + this.mvccPartitionViewCache = isUserSessionEnabled() + ? null + : new ConnectorPartitionViewCache<>("iceberg", this.properties); + this.listPartitionsViewCache = isUserSessionEnabled() + ? null + : new ConnectorPartitionViewCache<>("iceberg", this.properties); } /** @@ -278,7 +306,8 @@ static long resolveTableCacheTtlSecond(Map properties) { @Override public ConnectorMetadata getMetadata(ConnectorSession session) { return new IcebergConnectorMetadata(newCatalogBackedOps(session), properties, context, - latestSnapshotCache, tableCache, partitionCache, commentCache); + latestSnapshotCache, tableCache, partitionCache, commentCache, + mvccPartitionViewCache, listPartitionsViewCache); } /** @@ -603,6 +632,12 @@ public void invalidateTable(String dbName, String tableName) { if (commentCache != null) { commentCache.invalidate(TableIdentifier.of(dbName, tableName)); } + if (mvccPartitionViewCache != null) { + mvccPartitionViewCache.invalidateTable(dbName, tableName); + } + if (listPartitionsViewCache != null) { + listPartitionsViewCache.invalidateTable(dbName, tableName); + } } /** @@ -633,6 +668,12 @@ public void invalidateDb(String dbName) { if (commentCache != null) { commentCache.invalidateDb(dbName); } + if (mvccPartitionViewCache != null) { + mvccPartitionViewCache.invalidateDb(dbName); + } + if (listPartitionsViewCache != null) { + listPartitionsViewCache.invalidateDb(dbName); + } } /** @@ -659,6 +700,12 @@ public void invalidateAll() { if (commentCache != null) { commentCache.invalidateAll(); } + if (mvccPartitionViewCache != null) { + mvccPartitionViewCache.invalidateAll(); + } + if (listPartitionsViewCache != null) { + listPartitionsViewCache.invalidateAll(); + } manifestCache.invalidateAll(); } @@ -713,6 +760,16 @@ IcebergCommentCache commentCacheForTest() { return commentCache; } + /** Test-only: the derived MVCC partition-view cache (PERF-06), or {@code null} for a session=user catalog. */ + ConnectorPartitionViewCache mvccPartitionViewCacheForTest() { + return mvccPartitionViewCache; + } + + /** Test-only: the derived listPartitions view cache (PERF-06), or {@code null} for a session=user catalog. */ + ConnectorPartitionViewCache> listPartitionsViewCacheForTest() { + return listPartitionsViewCache; + } + @Override public ConnectorScanPlanProvider getScanPlanProvider() { // Mirrors PaimonConnector.getScanPlanProvider: build a fresh provider per call over the lazily-built diff --git a/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergConnectorMetadata.java b/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergConnectorMetadata.java index 70f5d0a146e056..99a54b500ee866 100644 --- a/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergConnectorMetadata.java +++ b/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergConnectorMetadata.java @@ -41,6 +41,8 @@ import org.apache.doris.connector.api.mvcc.ConnectorMvccSnapshot; import org.apache.doris.connector.api.mvcc.ConnectorTimeTravelSpec; import org.apache.doris.connector.api.pushdown.ConnectorExpression; +import org.apache.doris.connector.cache.ConnectorPartitionViewCache; +import org.apache.doris.connector.cache.PartitionViewCacheKey; import org.apache.doris.connector.spi.ConnectorContext; import org.apache.doris.thrift.THiveTable; import org.apache.doris.thrift.TIcebergTable; @@ -155,6 +157,14 @@ public class IcebergConnectorMetadata implements ConnectorMetadata { // connector is a REST vended-credentials, non-session catalog (see IcebergConnector); the convenience ctors // used by direct-construction tests pass null. Consumed only by getTableComment. private final IcebergCommentCache commentCache; + // PERF-06: cross-query DERIVED partition-view cache A (generic ConnectorPartitionViewCache), injected by the + // owning IcebergConnector; null = no cross-query derived layer (the convenience ctors used by + // direct-construction tests pass null; a session=user catalog also passes null). Layered ABOVE partitionCache + // (raw rows): a hit skips the derived-view BUILD, keyed by (db, table, snapshotId, schemaId). Two typed fields + // because getMvccPartitionView and listPartitions return different derived types. Consumed by + // getMvccPartitionView / listPartitions respectively. + private final ConnectorPartitionViewCache mvccPartitionViewCache; + private final ConnectorPartitionViewCache> listPartitionsViewCache; public IcebergConnectorMetadata(IcebergCatalogOps catalogOps, Map properties, ConnectorContext context) { @@ -180,11 +190,28 @@ public IcebergConnectorMetadata(IcebergCatalogOps catalogOps, Map properties, ConnectorContext context, IcebergLatestSnapshotCache latestSnapshotCache, IcebergTableCache tableCache, IcebergPartitionCache partitionCache, IcebergCommentCache commentCache) { + this(catalogOps, properties, context, latestSnapshotCache, tableCache, partitionCache, commentCache, + null, null); + } + + /** + * Full ctor used by {@link IcebergConnector#getMetadata}, adding the PERF-06 derived partition-view caches + * (cache A): {@code mvccPartitionViewCache} memoizes {@link #getMvccPartitionView}'s built RANGE view and + * {@code listPartitionsViewCache} memoizes {@link #listPartitions}'s built partition-info list, each keyed by + * {@code (db, table, snapshotId, schemaId)}. Both {@code null} for a session=user catalog / the convenience + * ctors (no cross-query derived layer -> compute directly every call). + */ + public IcebergConnectorMetadata(IcebergCatalogOps catalogOps, Map properties, + ConnectorContext context, IcebergLatestSnapshotCache latestSnapshotCache, + IcebergTableCache tableCache, IcebergPartitionCache partitionCache, + IcebergCommentCache commentCache, + ConnectorPartitionViewCache mvccPartitionViewCache, + ConnectorPartitionViewCache> listPartitionsViewCache) { this.catalogOps = catalogOps; this.properties = properties; this.context = context; @@ -192,6 +219,8 @@ public IcebergConnectorMetadata(IcebergCatalogOps catalogOps, Map getMvccPartitionView( ConnectorSession session, ConnectorTableHandle handle) { IcebergTableHandle iceHandle = (IcebergTableHandle) handle; try { + // PERF-06 cache A: memoize the BUILT derived view keyed by (db, table, snapshotId, schemaId) -- pure + // function of the pinned MVCC coordinate (a new snapshot/schema yields a new key, never a stale hit). + // The lookup sits INSIDE executeAuthenticated so a miss runs the loader (resolveTableForRead + the + // remote PARTITIONS build) under the FE-injected auth scope; a hit returns without any remote call. A + // null cache (session=user / no-cache catalog) computes directly every call. -1 (empty table / unpinned) + // enumerates the current snapshot and caches a trivially-empty view (harmless; REFRESH re-pins). return context.executeAuthenticated(() -> { - Table table = resolveTableForRead(session, iceHandle); - return Optional.of(IcebergPartitionUtils.buildMvccPartitionView(table, iceHandle.getSnapshotId(), - TableIdentifier.of(iceHandle.getDbName(), iceHandle.getTableName()), partitionCache)); + if (mvccPartitionViewCache == null) { + return Optional.of(buildMvccPartitionViewUncached(session, iceHandle)); + } + PartitionViewCacheKey key = new PartitionViewCacheKey(iceHandle.getDbName(), + iceHandle.getTableName(), iceHandle.getSnapshotId(), iceHandle.getSchemaId()); + return Optional.of(mvccPartitionViewCache.get(key, + () -> buildMvccPartitionViewUncached(session, iceHandle))); }); } catch (Exception e) { throw new RuntimeException("Failed to build iceberg MVCC partition view, error message is:" @@ -1531,6 +1570,19 @@ public Optional getMvccPartitionView( } } + /** + * Builds the derived MVCC partition view live (no cache-A layer): the same resolveTableForRead + remote + * PARTITIONS build the pre-cache code ran. Extracted so it can serve as cache A's per-miss loader. MUST run + * inside {@code context.executeAuthenticated} (the caller wraps it). A {@code NoSuchTableException} propagates + * (fail-loud parity) and, running through the framework loader, is never cached. + */ + private ConnectorMvccPartitionView buildMvccPartitionViewUncached( + ConnectorSession session, IcebergTableHandle iceHandle) { + Table table = resolveTableForRead(session, iceHandle); + return IcebergPartitionUtils.buildMvccPartitionView(table, iceHandle.getSnapshotId(), + TableIdentifier.of(iceHandle.getDbName(), iceHandle.getTableName()), partitionCache); + } + /** * The physical iceberg partition display names ({@code "f1=v1/f2=v2"}) for SHOW PARTITIONS (single-column * form — iceberg does not declare {@code SUPPORTS_PARTITION_STATS}). Post-cutover this restores real rows @@ -1574,17 +1626,18 @@ public List listPartitions(ConnectorSession session, ConnectorTableHandle handle, Optional filter) { IcebergTableHandle iceHandle = (IcebergTableHandle) handle; try { + // PERF-06 cache A: memoize the BUILT partition-info list keyed by (db, table, snapshotId, schemaId). + // The lookup sits INSIDE executeAuthenticated (a miss runs the remote build under the auth scope; a hit + // returns without a remote call). BYPASS the cache when the filter is present -- that is not the + // pruning path (which always passes Optional.empty()) and is not keyed by (snapshot, schema) alone -- or + // when the cache is null (session=user / no-cache catalog): compute directly every call. return context.executeAuthenticated(() -> { - Table table; - try { - table = resolveTableForRead(session, iceHandle); - } catch (NoSuchTableException e) { - LOG.warn("Iceberg table not found while listing partitions: {}.{}", - iceHandle.getDbName(), iceHandle.getTableName(), e); - return Collections.emptyList(); + if (listPartitionsViewCache == null || filter.isPresent()) { + return listPartitionsUncached(session, iceHandle); } - return IcebergPartitionUtils.listPartitions(table, - TableIdentifier.of(iceHandle.getDbName(), iceHandle.getTableName()), partitionCache); + PartitionViewCacheKey key = new PartitionViewCacheKey(iceHandle.getDbName(), + iceHandle.getTableName(), iceHandle.getSnapshotId(), iceHandle.getSchemaId()); + return listPartitionsViewCache.get(key, () -> listPartitionsUncached(session, iceHandle)); }); } catch (Exception e) { throw new RuntimeException("Failed to list iceberg partitions, error message is:" @@ -1592,6 +1645,26 @@ public List listPartitions(ConnectorSession session, } } + /** + * Builds the derived partition-info list live (no cache-A layer): the same resolveTableForRead + remote + * PARTITIONS build the pre-cache code ran, including the concurrent-drop degrade (a {@code NoSuchTableException} + * yields an empty list). Extracted so it can serve as cache A's per-miss loader. MUST run inside + * {@code context.executeAuthenticated} (the caller wraps it). + */ + private List listPartitionsUncached( + ConnectorSession session, IcebergTableHandle iceHandle) { + Table table; + try { + table = resolveTableForRead(session, iceHandle); + } catch (NoSuchTableException e) { + LOG.warn("Iceberg table not found while listing partitions: {}.{}", + iceHandle.getDbName(), iceHandle.getTableName(), e); + return Collections.emptyList(); + } + return IcebergPartitionUtils.listPartitions(table, + TableIdentifier.of(iceHandle.getDbName(), iceHandle.getTableName()), partitionCache); + } + // ========== E5: MVCC snapshots / time travel ========== /** diff --git a/fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/IcebergConnectorCacheTest.java b/fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/IcebergConnectorCacheTest.java index 0a22ffff2fb6e4..5d5b5285f07d43 100644 --- a/fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/IcebergConnectorCacheTest.java +++ b/fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/IcebergConnectorCacheTest.java @@ -17,6 +17,10 @@ package org.apache.doris.connector.iceberg; +import org.apache.doris.connector.api.ConnectorPartitionInfo; +import org.apache.doris.connector.cache.ConnectorPartitionViewCache; +import org.apache.doris.connector.cache.PartitionViewCacheKey; + import org.apache.iceberg.DataFiles; import org.apache.iceberg.ManifestFile; import org.apache.iceberg.PartitionSpec; @@ -31,8 +35,10 @@ import java.util.Collections; import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.OptionalLong; +import java.util.function.Supplier; /** * Tests IcebergConnector's T08 cache knobs: the latest-snapshot cache TTL resolution @@ -451,4 +457,97 @@ public void refreshHooksInvalidateCommentCache() { connector.invalidateAll(); Assertions.assertEquals(0, cache.size(), "REFRESH CATALOG drops everything"); } + + // ============ PERF-06: derived partition-view cache A (session=user gated) + invalidation ============ + + @Test + public void partitionViewCacheBuiltUnlessSessionUser() { + // Cache A (the derived partition-view cache) stores pure metadata (a partition view carries no + // FileIO/credential), so like the partition cache it stays built for a REST vended-credentials catalog. But + // under iceberg.rest.session=user it is an AUTHORIZATION-sensitive projection -- a shared (no user + // dimension) hit would disclose one user's partition view -- so BOTH typed instances are disabled (null) + // there. MUTATION: dropping the session=user gate -> non-null for session -> red; gating on the vended flag + // -> null for vended -> red. + IcebergConnector plain = new IcebergConnector(Collections.emptyMap(), new RecordingConnectorContext()); + Assertions.assertNotNull(plain.mvccPartitionViewCacheForTest(), "a plain catalog builds the MVCC view cache"); + Assertions.assertNotNull(plain.listPartitionsViewCacheForTest(), + "a plain catalog builds the listPartitions view cache"); + Map vended = new HashMap<>(); + vended.put(IcebergConnectorProperties.ICEBERG_CATALOG_TYPE, IcebergConnectorProperties.TYPE_REST); + vended.put(IcebergConnectorProperties.REST_VENDED_CREDENTIALS_ENABLED, "true"); + IcebergConnector vendedConn = new IcebergConnector(vended, new RecordingConnectorContext()); + Assertions.assertNotNull(vendedConn.mvccPartitionViewCacheForTest(), + "a vended-credentials catalog still builds the MVCC view cache (metadata carries no credentials)"); + Assertions.assertNotNull(vendedConn.listPartitionsViewCacheForTest(), + "a vended-credentials catalog still builds the listPartitions view cache"); + Map session = new HashMap<>(); + session.put(IcebergConnectorProperties.ICEBERG_CATALOG_TYPE, IcebergConnectorProperties.TYPE_REST); + session.put(IcebergConnectorProperties.REST_SESSION, IcebergConnectorProperties.SESSION_USER); + IcebergConnector sessionConn = new IcebergConnector(session, new RecordingConnectorContext()); + Assertions.assertNull(sessionConn.mvccPartitionViewCacheForTest(), + "a session=user catalog must NOT build the MVCC view cache (per-user authz must not be bypassed)"); + Assertions.assertNull(sessionConn.listPartitionsViewCacheForTest(), + "a session=user catalog must NOT build the listPartitions view cache"); + } + + @Test + public void refreshHooksInvalidatePartitionViewCache() { + // The REFRESH hooks must clear cache A too (else external DDL/writes stay invisible beyond the pin): REFRESH + // TABLE drops that table's snapshot entries, REFRESH DATABASE that db's, REFRESH CATALOG everything. Asserted + // via a counting loader (the framework's size() is package-private): after invalidation the loader must run + // again. MUTATION: an invalidate* hook not routed to the view cache -> the entry survives -> loader not + // re-run -> a loads assert below red. + IcebergConnector connector = + new IcebergConnector(Collections.emptyMap(), new RecordingConnectorContext()); + ConnectorPartitionViewCache> cache = connector.listPartitionsViewCacheForTest(); + Assertions.assertNotNull(cache); + int[] loads = {0}; + Supplier> loader = () -> { + loads[0]++; + return Collections.emptyList(); + }; + PartitionViewCacheKey db1t1 = new PartitionViewCacheKey("db1", "t1", 1L, 1L); + PartitionViewCacheKey db1t2 = new PartitionViewCacheKey("db1", "t2", 1L, 1L); + PartitionViewCacheKey db2t1 = new PartitionViewCacheKey("db2", "t1", 1L, 1L); + + // REFRESH TABLE db1.t1 -> only db1.t1 re-loads. + cache.get(db1t1, loader); + cache.get(db1t1, loader); + Assertions.assertEquals(1, loads[0], "second get is a hit"); + connector.invalidateTable("db1", "t1"); + cache.get(db1t1, loader); + Assertions.assertEquals(2, loads[0], "REFRESH TABLE forces a reload of db1.t1"); + + // REFRESH DATABASE db1 -> db1.t2 re-loads; db2.t1 unaffected. + cache.get(db1t2, loader); // loads=3 (miss) + cache.get(db2t1, loader); // loads=4 (miss) + cache.get(db1t2, loader); // hit + cache.get(db2t1, loader); // hit + Assertions.assertEquals(4, loads[0]); + connector.invalidateDb("db1"); + cache.get(db2t1, loader); // db2 untouched -> hit + Assertions.assertEquals(4, loads[0], "REFRESH DATABASE db1 must NOT drop db2's entries"); + cache.get(db1t2, loader); // db1.t2 dropped -> miss + Assertions.assertEquals(5, loads[0], "REFRESH DATABASE db1 drops db1's entries"); + + // REFRESH CATALOG -> everything re-loads. + connector.invalidateAll(); + cache.get(db2t1, loader); + Assertions.assertEquals(6, loads[0], "REFRESH CATALOG drops everything"); + } + + @Test + public void invalidateHooksNoThrowForSessionUserPartitionViewCaches() { + // Under session=user cache A's two instances are null; the invalidate hooks must null-guard them (no NPE). + // MUTATION: an unguarded view-cache invalidate on a nulled field -> NPE -> red. + Map session = new HashMap<>(); + session.put(IcebergConnectorProperties.ICEBERG_CATALOG_TYPE, IcebergConnectorProperties.TYPE_REST); + session.put(IcebergConnectorProperties.REST_SESSION, IcebergConnectorProperties.SESSION_USER); + IcebergConnector connector = new IcebergConnector(session, new RecordingConnectorContext()); + Assertions.assertNull(connector.mvccPartitionViewCacheForTest()); + Assertions.assertNull(connector.listPartitionsViewCacheForTest()); + Assertions.assertDoesNotThrow(() -> connector.invalidateTable("db1", "t1")); + Assertions.assertDoesNotThrow(() -> connector.invalidateDb("db1")); + Assertions.assertDoesNotThrow(connector::invalidateAll); + } } diff --git a/fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/IcebergConnectorMetadataPartitionViewCacheTest.java b/fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/IcebergConnectorMetadataPartitionViewCacheTest.java new file mode 100644 index 00000000000000..0ca659a7bd24f1 --- /dev/null +++ b/fe/fe-connector/fe-connector-iceberg/src/test/java/org/apache/doris/connector/iceberg/IcebergConnectorMetadataPartitionViewCacheTest.java @@ -0,0 +1,260 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.doris.connector.iceberg; + +import org.apache.doris.connector.api.ConnectorPartitionInfo; +import org.apache.doris.connector.api.mvcc.ConnectorMvccPartition; +import org.apache.doris.connector.api.mvcc.ConnectorMvccPartitionView; +import org.apache.doris.connector.api.pushdown.ConnectorExpression; +import org.apache.doris.connector.cache.ConnectorPartitionViewCache; + +import org.apache.iceberg.DataFiles; +import org.apache.iceberg.FileFormat; +import org.apache.iceberg.PartitionSpec; +import org.apache.iceberg.Schema; +import org.apache.iceberg.Table; +import org.apache.iceberg.catalog.Namespace; +import org.apache.iceberg.catalog.TableIdentifier; +import org.apache.iceberg.inmemory.InMemoryCatalog; +import org.apache.iceberg.types.Types; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +import java.util.Collections; +import java.util.List; +import java.util.Optional; +import java.util.stream.Collectors; + +/** + * PERF-06 tests for the cross-query DERIVED partition-view cache ("cache A", the generic + * {@link ConnectorPartitionViewCache}) wired into {@link IcebergConnectorMetadata#getMvccPartitionView} / + * {@link IcebergConnectorMetadata#listPartitions}. Uses the real {@link InMemoryCatalog} + + * {@link RecordingIcebergCatalogOps} harness (no Mockito, no docker): the cache sits ABOVE the per-query build, + * whose first step is {@code resolveTableForRead -> catalogOps.loadTable} (logged as {@code loadTable:db1.t1}), so + * a cache HIT skips the whole loader and the {@code loadTable} count is the enumeration counter — the SAME proxy + * the sibling cache tests use ({@code IcebergConnectorMetadataMvccTest.beginQuerySnapshot*Cache*}). The raw + * partition cache (PERF-02) is passed null throughout, so the {@code loadTable} count isolates cache A's effect. + * The partition math/merge parity itself is covered by {@link IcebergPartitionUtilsTest}. + */ +public class IcebergConnectorMetadataPartitionViewCacheTest { + + private static final Schema PARTITIONED_SCHEMA = new Schema( + Types.NestedField.required(1, "id", Types.IntegerType.get()), + Types.NestedField.optional(2, "ts", Types.TimestampType.withoutZone())); + + /** A real db1.t1 partitioned by day(ts) carrying TWO snapshots (day=100, then +day=101). */ + private static final class TwoSnap { + Table table; + long s1; + long s2; + long schemaId; + } + + private static TwoSnap twoSnapshotTable() { + InMemoryCatalog catalog = new InMemoryCatalog(); + catalog.initialize("test", Collections.emptyMap()); + catalog.createNamespace(Namespace.of("db1")); + PartitionSpec spec = PartitionSpec.builderFor(PARTITIONED_SCHEMA).day("ts").build(); + Table table = catalog.createTable(TableIdentifier.of("db1", "t1"), PARTITIONED_SCHEMA, spec); + table.newAppend().appendFile(dayFile(spec, "s3://b/db1/t1/f1.parquet", "ts_day=1970-04-11")).commit(); + TwoSnap f = new TwoSnap(); + f.s1 = table.currentSnapshot().snapshotId(); + table.newAppend().appendFile(dayFile(spec, "s3://b/db1/t1/f2.parquet", "ts_day=1970-04-12")).commit(); + f.s2 = table.currentSnapshot().snapshotId(); + f.schemaId = table.schema().schemaId(); + f.table = catalog.loadTable(TableIdentifier.of("db1", "t1")); + Assertions.assertNotEquals(f.s1, f.s2, "two appends must create two distinct snapshots"); + return f; + } + + private static org.apache.iceberg.DataFile dayFile(PartitionSpec spec, String path, String partitionPath) { + return DataFiles.builder(spec) + .withPath(path).withFileSizeInBytes(100).withRecordCount(1) + .withPartitionPath(partitionPath).withFormat(FileFormat.PARQUET).build(); + } + + private static IcebergConnectorMetadata metadataWithMvccCache(RecordingIcebergCatalogOps ops, + ConnectorPartitionViewCache cache) { + // 9-arg ctor: disabled latest-snapshot cache + null table/partition/comment caches so the loadTable count + // reflects cache A alone; only the mvcc view cache under test is injected. + return new IcebergConnectorMetadata(ops, Collections.emptyMap(), new RecordingConnectorContext(), + new IcebergLatestSnapshotCache(0L, 1), null, null, null, cache, null); + } + + private static IcebergConnectorMetadata metadataWithListCache(RecordingIcebergCatalogOps ops, + ConnectorPartitionViewCache> cache) { + return new IcebergConnectorMetadata(ops, Collections.emptyMap(), new RecordingConnectorContext(), + new IcebergLatestSnapshotCache(0L, 1), null, null, null, null, cache); + } + + private static IcebergTableHandle handle() { + return new IcebergTableHandle("db1", "t1"); + } + + private static long loadCount(RecordingIcebergCatalogOps ops) { + return ops.log.stream().filter(s -> s.equals("loadTable:db1.t1")).count(); + } + + private static ConnectorPartitionViewCache mvccCache() { + return new ConnectorPartitionViewCache<>("iceberg", Collections.emptyMap()); + } + + private static ConnectorPartitionViewCache> listCache() { + return new ConnectorPartitionViewCache<>("iceberg", Collections.emptyMap()); + } + + private static List mvccNames(Optional view) { + return view.get().getPartitions().stream() + .map(ConnectorMvccPartition::getName).collect(Collectors.toList()); + } + + // --------------------------------------------------------------------- + // getMvccPartitionView + // --------------------------------------------------------------------- + + @Test + public void getMvccPartitionViewCachesDerivedViewAcrossQueries() { + // WHY: cache A must memoize the BUILT MVCC view keyed by (db, table, snapshotId, schemaId), so a repeated + // query on the same pin skips the derived rebuild AND the underlying loadTable/scan. MUTATION: not + // consulting the cache (compute directly every call) -> loadTable runs twice -> red. + TwoSnap f = twoSnapshotTable(); + RecordingIcebergCatalogOps ops = new RecordingIcebergCatalogOps(); + ops.table = f.table; + ConnectorPartitionViewCache cache = mvccCache(); + IcebergConnectorMetadata md = metadataWithMvccCache(ops, cache); + + Optional first = md.getMvccPartitionView(null, handle()); + Optional second = md.getMvccPartitionView(null, handle()); + + Assertions.assertEquals(java.util.Arrays.asList("ts_day=100", "ts_day=101"), mvccNames(first)); + Assertions.assertEquals(mvccNames(first), mvccNames(second), "the cached view is returned verbatim"); + Assertions.assertEquals(1, loadCount(ops), "a cache hit must not re-enumerate (loadTable once)"); + } + + @Test + public void getMvccPartitionViewDifferentSnapshotReEnumerates() { + // WHY: pinning a different snapshot id yields a different cache key (time-travel must not serve another + // snapshot's view), so each pin re-enumerates. MUTATION: keying on (db,table) only -> the S1 view would be + // served for the S2 pin -> names/loadCount below red. + TwoSnap f = twoSnapshotTable(); + RecordingIcebergCatalogOps ops = new RecordingIcebergCatalogOps(); + ops.table = f.table; + IcebergConnectorMetadata md = metadataWithMvccCache(ops, mvccCache()); + + IcebergTableHandle atS1 = handle().withSnapshot(f.s1, null, f.schemaId); + IcebergTableHandle atS2 = handle().withSnapshot(f.s2, null, f.schemaId); + List namesS1 = mvccNames(md.getMvccPartitionView(null, atS1)); + List namesS2 = mvccNames(md.getMvccPartitionView(null, atS2)); + + Assertions.assertEquals(Collections.singletonList("ts_day=100"), namesS1); + Assertions.assertEquals(java.util.Arrays.asList("ts_day=100", "ts_day=101"), namesS2); + Assertions.assertEquals(2, loadCount(ops), "distinct snapshot keys must each enumerate"); + } + + @Test + public void getMvccPartitionViewInvalidateTableForcesReEnumeration() { + // WHY: REFRESH TABLE (Connector.invalidateTable -> cache.invalidateTable) must drop the cached view so the + // next query re-enumerates live. MUTATION: invalidateTable not wired -> the second call hits the stale + // entry -> loadCount stays 1 -> red. + TwoSnap f = twoSnapshotTable(); + RecordingIcebergCatalogOps ops = new RecordingIcebergCatalogOps(); + ops.table = f.table; + ConnectorPartitionViewCache cache = mvccCache(); + IcebergConnectorMetadata md = metadataWithMvccCache(ops, cache); + + md.getMvccPartitionView(null, handle()); + cache.invalidateTable("db1", "t1"); + md.getMvccPartitionView(null, handle()); + Assertions.assertEquals(2, loadCount(ops), "invalidateTable must force a re-enumeration"); + } + + @Test + public void getMvccPartitionViewNullCacheEnumeratesEveryCall() { + // The session=user analogue at the metadata level: a null cache (IcebergConnector passes null under + // iceberg.rest.session=user) means compute directly every call -> loadTable on every query (no cross-query + // sharing). MUTATION: defaulting to a shared cache when null was intended -> loadCount 1 -> red. + TwoSnap f = twoSnapshotTable(); + RecordingIcebergCatalogOps ops = new RecordingIcebergCatalogOps(); + ops.table = f.table; + IcebergConnectorMetadata md = metadataWithMvccCache(ops, null); + md.getMvccPartitionView(null, handle()); + md.getMvccPartitionView(null, handle()); + Assertions.assertEquals(2, loadCount(ops), "a null (disabled) cache must re-enumerate every call"); + } + + // --------------------------------------------------------------------- + // listPartitions + // --------------------------------------------------------------------- + + @Test + public void listPartitionsCachesDerivedListAcrossQueries() { + // WHY: the empty-filter pruning path must memoize the built partition-info list. MUTATION: not consulting + // the cache -> loadTable twice -> red. + TwoSnap f = twoSnapshotTable(); + RecordingIcebergCatalogOps ops = new RecordingIcebergCatalogOps(); + ops.table = f.table; + ConnectorPartitionViewCache> cache = listCache(); + IcebergConnectorMetadata md = metadataWithListCache(ops, cache); + + List first = md.listPartitions(null, handle(), Optional.empty()); + List second = md.listPartitions(null, handle(), Optional.empty()); + + List names = first.stream().map(ConnectorPartitionInfo::getPartitionName).collect(Collectors.toList()); + Assertions.assertEquals(java.util.Arrays.asList("ts_day=100", "ts_day=101"), names); + Assertions.assertEquals(names, + second.stream().map(ConnectorPartitionInfo::getPartitionName).collect(Collectors.toList())); + Assertions.assertEquals(1, loadCount(ops), "a cache hit must not re-enumerate (loadTable once)"); + } + + @Test + public void listPartitionsWithFilterBypassesCache() { + // WHY: only the empty-filter pruning path is cached; a non-empty filter must BYPASS the cache and compute + // directly (the pruning path always passes Optional.empty()). MUTATION: caching regardless of filter -> + // the second filtered call hits -> loadCount 1 + cache.size 1 -> red. + TwoSnap f = twoSnapshotTable(); + RecordingIcebergCatalogOps ops = new RecordingIcebergCatalogOps(); + ops.table = f.table; + ConnectorPartitionViewCache> cache = listCache(); + IcebergConnectorMetadata md = metadataWithListCache(ops, cache); + + ConnectorExpression filter = Collections::emptyList; // any non-empty filter + md.listPartitions(null, handle(), Optional.of(filter)); + md.listPartitions(null, handle(), Optional.of(filter)); + Assertions.assertEquals(2, loadCount(ops), "a present filter must bypass the cache (compute every call)"); + // A bypassed call must also NOT populate the cache: a following empty-filter (pruning) call must MISS + // (loadCount 3), not be served from a filtered-populated entry. + md.listPartitions(null, handle(), Optional.empty()); + Assertions.assertEquals(3, loadCount(ops), "the empty-filter call must miss (filtered calls never populate)"); + } + + @Test + public void listPartitionsInvalidateAllForcesReEnumeration() { + // WHY: REFRESH CATALOG (Connector.invalidateAll -> cache.invalidateAll) must drop the cached list. + // MUTATION: invalidateAll not wired -> the second call hits -> loadCount stays 1 -> red. + TwoSnap f = twoSnapshotTable(); + RecordingIcebergCatalogOps ops = new RecordingIcebergCatalogOps(); + ops.table = f.table; + ConnectorPartitionViewCache> cache = listCache(); + IcebergConnectorMetadata md = metadataWithListCache(ops, cache); + + md.listPartitions(null, handle(), Optional.empty()); + cache.invalidateAll(); + md.listPartitions(null, handle(), Optional.empty()); + Assertions.assertEquals(2, loadCount(ops), "invalidateAll must force a re-enumeration"); + } +} From e3be4ad9b008564cd5429331f8df9eafe292f08c Mon Sep 17 00:00:00 2001 From: morningman Date: Tue, 21 Jul 2026 03:22:13 +0800 Subject: [PATCH 13/19] [perf](catalog) wire cross-query derived partition-view cache A into the paimon connector (S4) Layer the generic ConnectorPartitionViewCache (S2 framework) above paimon's raw catalogOps.listPartitions round-trip so a repeated query on a partitioned paimon table skips the derived-view BUILD (display-name rendering + null-sentinel normalization, PaimonConnectorMetadata#collectPartitions), not just avoids re-deriving from an already-cached remote result -- there was no such cache before. ONE typed field (unlike iceberg's two): paimon does not override getMvccPartitionView (the generic MTMV model falls back to its default listPartitions/LIST/timestamp path for paimon), so listPartitions is the only enumeration hook to wrap. - Key = (db, table, snapshotId, schemaId). snapshotId reads the SAME per-catalog PaimonLatestSnapshotCache that beginQuerySnapshot pins queries to -- collectPartitions' remote call is base-identifier-only (it does not apply the handle's pinned scanOptions, unlike the scan path) so it always reflects the CURRENT catalog state; keying off "current" (not whatever pin happens to be on the handle) keeps the cache key an honest descriptor of what was actually computed. schemaId is pinned -1 ("unversioned" for that axis): PaimonTableHandle carries no schemaId field, applySnapshot never threads one onto it, and beginQuerySnapshot's own ConnectorMvccSnapshot leaves it at the builder default (-1) for the common latest-pin path -- and collectPartitions' output does not depend on schema version in practice (paimon partition columns are immutable post-creation). - listPartitions: bypass cache when filter.isPresent() (mirrors the iceberg pattern; not currently load-bearing since paimon ignores the filter value, but keeps the two paths independent) and when the handle is unpartitioned (preserves the pre-existing "no seam call for unpartitioned tables" contract). - Paimon has NO iceberg.rest.session=user-style per-user cache-disabling convention (a paimon catalog authenticates at catalog-creation time -- Kerberos UGI / HMS principal -- not per-query session identity), so the cache is constructed unconditionally on PaimonConnector (never null there); PaimonConnectorMetadata's convenience/test ctors still pass null (no cross-query layer) for the ~15 existing direct-construction tests. - Invalidation routed from invalidateTable/invalidateDb/invalidateAll to the new cache alongside the existing latestSnapshotCache/schemaAtMemo. Tests: new PaimonConnectorMetadataPartitionViewCacheTest (7) proves same-key-hit enumerates once, distinct-latest-snapshot re-enumerates, invalidateTable/All re-enumerate, filter bypass, null-cache re-enumeration, and the unpartitioned fast path touches no remote seam; PaimonConnectorCacheTest (+2) proves the cache is always built (no session=user gate) and the 3-hook invalidation routing on the real field. Full module suite 370/370 green (1 pre-existing skip, PaimonLiveConnectivityTest). Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_014ZngdXGBAXNDRe7jA42mAS --- .../connector/paimon/PaimonConnector.java | 31 ++- .../paimon/PaimonConnectorMetadata.java | 74 ++++- .../paimon/PaimonConnectorCacheTest.java | 66 +++++ ...nnectorMetadataPartitionViewCacheTest.java | 262 ++++++++++++++++++ 4 files changed, 431 insertions(+), 2 deletions(-) create mode 100644 fe/fe-connector/fe-connector-paimon/src/test/java/org/apache/doris/connector/paimon/PaimonConnectorMetadataPartitionViewCacheTest.java diff --git a/fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonConnector.java b/fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonConnector.java index 58df5e326b111d..12c85993a69f94 100644 --- a/fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonConnector.java +++ b/fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonConnector.java @@ -20,9 +20,11 @@ import org.apache.doris.connector.api.Connector; import org.apache.doris.connector.api.ConnectorCapability; import org.apache.doris.connector.api.ConnectorMetadata; +import org.apache.doris.connector.api.ConnectorPartitionInfo; import org.apache.doris.connector.api.ConnectorSession; import org.apache.doris.connector.api.ConnectorValidationContext; import org.apache.doris.connector.api.scan.ConnectorScanPlanProvider; +import org.apache.doris.connector.cache.ConnectorPartitionViewCache; import org.apache.doris.connector.metastore.HmsMetaStoreProperties; import org.apache.doris.connector.metastore.spi.JdbcDriverSupport; import org.apache.doris.connector.metastore.spi.MetaStoreProviders; @@ -50,6 +52,7 @@ import java.util.Collections; import java.util.EnumSet; import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.Optional; import java.util.OptionalLong; @@ -126,6 +129,19 @@ public class PaimonConnector implements Connector { // Cleared wholesale on REFRESH CATALOG (the connector is rebuilt). See PaimonSchemaAtMemo. private final PaimonSchemaAtMemo schemaAtMemo = new PaimonSchemaAtMemo(PaimonSchemaAtMemo.DEFAULT_MAX_SIZE); + // PERF-06: cross-query DERIVED partition-view cache ("cache A", the generic ConnectorPartitionViewCache from + // fe-connector-cache), layered ABOVE the raw remote catalog.listPartitions call (PaimonCatalogOps#listPartitions): + // it memoizes the BUILT List (display-name rendering + null-sentinel normalization, + // see PaimonConnectorMetadata#collectPartitions) keyed by (db, table, snapshotId, schemaId), so a repeated + // query on a partitioned table skips the derived rebuild AND the remote catalog round-trip. ONE typed field + // (unlike iceberg's two): paimon does not override getMvccPartitionView, so the generic MTMV model falls + // back to its default listPartitions/LIST/timestamp path for paimon -- listPartitions is the only + // enumeration hook to wrap. Unlike iceberg, paimon has NO session=user / per-user credential-isolation + // cache-disabling convention (a paimon catalog authenticates at catalog-creation time -- Kerberos UGI / + // HMS principal -- not per-query session identity), so this is constructed unconditionally: never null on + // a live connector (only PaimonConnectorMetadata's convenience/test constructors pass null). + private final ConnectorPartitionViewCache> partitionViewCache; + public PaimonConnector(Map properties, ConnectorContext context) { this.properties = properties; // Wrap the FE-injected context so every executeAuthenticated pins the TCCL to the plugin loader (the @@ -137,6 +153,9 @@ public PaimonConnector(Map properties, ConnectorContext context) this::pluginAuthenticator); this.latestSnapshotCache = new PaimonLatestSnapshotCache(resolveTableCacheTtlSecond(properties), DEFAULT_TABLE_CACHE_CAPACITY); + // Reads its own meta.cache.paimon.partition_view.(enable|ttl-second|capacity) from the catalog + // properties via the framework's CacheSpec (default ON / 24h / 1000). + this.partitionViewCache = new ConnectorPartitionViewCache<>("paimon", properties); } /** @@ -225,7 +244,7 @@ private static long resolveTableCacheTtlSecond(Map properties) { public ConnectorMetadata getMetadata(ConnectorSession session) { return new PaimonConnectorMetadata( new PaimonCatalogOps.CatalogBackedPaimonCatalogOps(ensureCatalog()), properties, context, - schemaAtMemo, latestSnapshotCache); + schemaAtMemo, latestSnapshotCache, partitionViewCache); } @Override @@ -239,6 +258,9 @@ public void invalidateTable(String dbName, String tableName) { // (db,table,sysTable,branch,schemaId) and would otherwise serve a stale schema-at-snapshot after a // drop+recreate that reuses a schemaId (the memo's narrow write-once-per-schemaId assumption breaks). schemaAtMemo.invalidate(dbName, tableName); + // PERF-06: also drop this table's cached derived partition-view entries (every snapshotId cached for + // it), so the next listPartitions re-enumerates live. + partitionViewCache.invalidateTable(dbName, tableName); } /** @@ -255,12 +277,14 @@ public void invalidateTable(String dbName, String tableName) { public void invalidateDb(String dbName) { latestSnapshotCache.invalidateDb(dbName); schemaAtMemo.invalidateDb(dbName); + partitionViewCache.invalidateDb(dbName); } @Override public void invalidateAll() { latestSnapshotCache.invalidateAll(); schemaAtMemo.invalidateAll(); + partitionViewCache.invalidateAll(); } @Override @@ -316,6 +340,11 @@ public Set getCapabilities() { ConnectorCapability.SUPPORTS_SHOW_CREATE_DDL); } + /** Test-only: the derived listPartitions view cache (PERF-06). Never null (paimon has no session=user gate). */ + ConnectorPartitionViewCache> partitionViewCacheForTest() { + return partitionViewCache; + } + private Catalog ensureCatalog() { if (catalog == null) { synchronized (this) { diff --git a/fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonConnectorMetadata.java b/fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonConnectorMetadata.java index c2e1fe021da017..bd59897e3f03ec 100644 --- a/fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonConnectorMetadata.java +++ b/fe/fe-connector/fe-connector-paimon/src/main/java/org/apache/doris/connector/paimon/PaimonConnectorMetadata.java @@ -32,6 +32,8 @@ import org.apache.doris.connector.api.mvcc.ConnectorTimeTravelSpec; import org.apache.doris.connector.api.pushdown.ConnectorExpression; import org.apache.doris.connector.api.scan.ConnectorPartitionValues; +import org.apache.doris.connector.cache.ConnectorPartitionViewCache; +import org.apache.doris.connector.cache.PartitionViewCacheKey; import org.apache.doris.connector.spi.ConnectorContext; import org.apache.doris.thrift.THiveTable; import org.apache.doris.thrift.TTableDescriptor; @@ -95,6 +97,15 @@ public class PaimonConnectorMetadata implements ConnectorMetadata { // existing direct-construction tests compile unchanged; production goes through the 5-arg ctor. private final PaimonLatestSnapshotCache latestSnapshotCache; + // PERF-06: cross-query DERIVED partition-view cache A (generic ConnectorPartitionViewCache), injected by the + // owning PaimonConnector; null = no cross-query derived layer (the convenience/test ctors used by ~15 + // existing direct-construction tests pass null). Layered ABOVE the raw remote catalogOps.listPartitions + // call: a hit skips both the derived-view BUILD (collectPartitions) and the remote round-trip, keyed by + // (db, table, snapshotId, schemaId). Consumed only by listPartitions -- paimon does not override + // getMvccPartitionView (see ConnectorMetadata's default), so the generic MTMV model already uses + // listPartitions for its LIST/timestamp partition view; there is no second hook to wrap. + private final ConnectorPartitionViewCache> partitionViewCache; + public PaimonConnectorMetadata(PaimonCatalogOps catalogOps, Map properties, ConnectorContext context) { this(catalogOps, properties, context, new PaimonSchemaAtMemo(PaimonSchemaAtMemo.DEFAULT_MAX_SIZE)); @@ -105,15 +116,30 @@ public PaimonConnectorMetadata(PaimonCatalogOps catalogOps, Map this(catalogOps, properties, context, schemaAtMemo, new PaimonLatestSnapshotCache(0L, 1)); } + /** Convenience ctor without the PERF-06 derived partition-view cache (null -> listPartitions always live). */ PaimonConnectorMetadata(PaimonCatalogOps catalogOps, Map properties, ConnectorContext context, PaimonSchemaAtMemo schemaAtMemo, PaimonLatestSnapshotCache latestSnapshotCache) { + this(catalogOps, properties, context, schemaAtMemo, latestSnapshotCache, null); + } + + /** + * Full ctor used by {@link PaimonConnector#getMetadata}, adding the PERF-06 derived partition-view cache + * (cache A): {@code partitionViewCache} memoizes {@link #listPartitions}'s built + * {@code List}, keyed by {@code (db, table, snapshotId, schemaId)}. {@code null} + * for the convenience/test ctors (no cross-query derived layer -> compute directly every call). + */ + PaimonConnectorMetadata(PaimonCatalogOps catalogOps, Map properties, + ConnectorContext context, PaimonSchemaAtMemo schemaAtMemo, + PaimonLatestSnapshotCache latestSnapshotCache, + ConnectorPartitionViewCache> partitionViewCache) { this.catalogOps = catalogOps; this.typeMappingOptions = buildTypeMappingOptions(properties); this.context = context; this.catalogProperties = properties; this.schemaAtMemo = schemaAtMemo; this.latestSnapshotCache = latestSnapshotCache; + this.partitionViewCache = partitionViewCache; } @Override @@ -955,11 +981,57 @@ public List listPartitionNames(ConnectorSession session, ConnectorTableH * {@code PaimonExternalCatalog.getPaimonPartitions} returns the full partition set without * pushing predicates into the Paimon catalog, and this preserves that behavior (mirrors * {@code MaxComputeConnectorMetadata}). + * + *

PERF-06 cache A: the BUILT {@code List} is memoized across queries in + * {@link #partitionViewCache}, keyed by {@code (db, table, snapshotId, schemaId)} (see + * {@link #partitionViewCacheKey}) — a hit skips both {@link #collectPartitions} and the remote + * {@code catalogOps.listPartitions} round-trip. The cache is BYPASSED (compute directly, never populated) + * when: {@code partitionViewCache} is {@code null} (the convenience/test ctors); {@code filter} is present + * (not the pruning path, and not keyed by filter); or the handle is unpartitioned (mirrors + * {@link #collectPartitions}'s own empty-partitionKeys short-circuit, so an unpartitioned table never + * touches {@link #latestSnapshotCache} either — preserving the "no seam call" contract the unpartitioned + * path already guarantees). */ @Override public List listPartitions(ConnectorSession session, ConnectorTableHandle handle, Optional filter) { - return collectPartitions((PaimonTableHandle) handle); + PaimonTableHandle paimonHandle = (PaimonTableHandle) handle; + List partitionKeys = paimonHandle.getPartitionKeys(); + if (partitionViewCache == null || filter.isPresent() + || partitionKeys == null || partitionKeys.isEmpty()) { + return collectPartitions(paimonHandle); + } + PartitionViewCacheKey key = partitionViewCacheKey(paimonHandle); + return partitionViewCache.get(key, () -> collectPartitions(paimonHandle)); + } + + /** + * Builds cache A's key for {@code paimonHandle}: {@code (db, table, snapshotId, schemaId)}. + * + *

snapshotId: {@link #collectPartitions}'s remote call ({@code catalogOps.listPartitions(Identifier)}) + * is BASE-identifier-only — it does not apply the handle's pinned {@code scanOptions} (unlike the scan path), + * so it always reflects the CURRENT catalog state, never a time-travel pin (branch / time-travel reads never + * reach this path at all — see {@link #collectPartitions}). The key must therefore track "current", not + * whatever snapshot happens to be threaded on the handle: it reads the SAME per-catalog + * {@link #latestSnapshotCache} that {@link #beginQuerySnapshot} pins queries to (a cheap in-memory hit within + * the query — {@code beginQuerySnapshot} already warmed it), so a repeat query within the TTL hits this cache, + * and a new snapshot (data change, once the entry expires or REFRESH invalidates it) naturally mints a new key. + * + *

schemaId: pinned {@code -1} ("unversioned" for that axis, matching + * {@link PartitionViewCacheKey}'s documented convention). Unlike iceberg, paimon's {@link PaimonTableHandle} + * carries no schemaId — {@code applySnapshot} threads only {@code scanOptions} (an opaque properties map; + * see its javadoc) onto the handle, and {@link #beginQuerySnapshot} (the common latest-pin path) never + * resolves a schemaId either (its {@code ConnectorMvccSnapshot} keeps the builder default {@code -1}). This + * is not a loss for THIS view: {@link #collectPartitions} derives its output from {@code partitionKeys} + * (fixed at handle-build time) and paimon's raw partition specs, and paimon partition columns are immutable + * post-creation, so schema evolution (e.g. ADD COLUMN) does not change what this method computes. + */ + private PartitionViewCacheKey partitionViewCacheKey(PaimonTableHandle paimonHandle) { + Identifier identifier = Identifier.create(paimonHandle.getDatabaseName(), paimonHandle.getTableName()); + long snapshotId = latestSnapshotCache.getOrLoad(identifier, + () -> catalogOps.latestSnapshotId(resolveTable(paimonHandle)).orElse(-1L)); + return new PartitionViewCacheKey( + paimonHandle.getDatabaseName(), paimonHandle.getTableName(), snapshotId, -1L); } @Override diff --git a/fe/fe-connector/fe-connector-paimon/src/test/java/org/apache/doris/connector/paimon/PaimonConnectorCacheTest.java b/fe/fe-connector/fe-connector-paimon/src/test/java/org/apache/doris/connector/paimon/PaimonConnectorCacheTest.java index 7e7aa033229238..f952e6d47a7def 100644 --- a/fe/fe-connector/fe-connector-paimon/src/test/java/org/apache/doris/connector/paimon/PaimonConnectorCacheTest.java +++ b/fe/fe-connector/fe-connector-paimon/src/test/java/org/apache/doris/connector/paimon/PaimonConnectorCacheTest.java @@ -17,13 +17,19 @@ package org.apache.doris.connector.paimon; +import org.apache.doris.connector.api.ConnectorPartitionInfo; +import org.apache.doris.connector.cache.ConnectorPartitionViewCache; +import org.apache.doris.connector.cache.PartitionViewCacheKey; + import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; import java.util.Collections; import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.OptionalLong; +import java.util.function.Supplier; /** * Tests PaimonConnector's FIX-4 cache knobs (CI 973411): the {@code meta.cache.paimon.table.ttl-second} @@ -83,4 +89,64 @@ public void invalidateHooksAreNoThrowOnFreshConnector() { Assertions.assertDoesNotThrow(() -> connector.invalidateDb("db1")); Assertions.assertDoesNotThrow(connector::invalidateAll); } + + // ============ PERF-06: derived partition-view cache A (no session=user gate) + invalidation ============ + + @Test + public void partitionViewCacheAlwaysBuilt() { + // Unlike iceberg, paimon has NO session=user / per-user credential-isolation cache-disabling + // convention (a paimon catalog authenticates at catalog-creation time, not per-query session + // identity), so the connector must construct cache A unconditionally on every flavor. MUTATION: a + // stray session-like gate leaving the field null on some property combination -> red. + Assertions.assertNotNull(connector(Collections.emptyMap()).partitionViewCacheForTest(), + "a fresh paimon connector must always build the partition-view cache"); + Map withTtl = props("3600"); + Assertions.assertNotNull(connector(withTtl).partitionViewCacheForTest(), + "the partition-view cache is independent of meta.cache.paimon.table.ttl-second"); + } + + @Test + public void refreshHooksInvalidatePartitionViewCache() { + // The REFRESH hooks must clear cache A too (else external DDL/writes stay invisible beyond the TTL): + // REFRESH TABLE drops that table's snapshot entries, REFRESH DATABASE that db's, REFRESH CATALOG + // everything. Asserted via a counting loader (the framework's size() is package-private): after + // invalidation the loader must run again. MUTATION: an invalidate* hook not routed to the view cache + // -> the entry survives -> loader not re-run -> a loads assert below red. + PaimonConnector connector = connector(Collections.emptyMap()); + ConnectorPartitionViewCache> cache = connector.partitionViewCacheForTest(); + Assertions.assertNotNull(cache); + int[] loads = {0}; + Supplier> loader = () -> { + loads[0]++; + return Collections.emptyList(); + }; + PartitionViewCacheKey db1t1 = new PartitionViewCacheKey("db1", "t1", 1L, -1L); + PartitionViewCacheKey db1t2 = new PartitionViewCacheKey("db1", "t2", 1L, -1L); + PartitionViewCacheKey db2t1 = new PartitionViewCacheKey("db2", "t1", 1L, -1L); + + // REFRESH TABLE db1.t1 -> only db1.t1 re-loads. + cache.get(db1t1, loader); + cache.get(db1t1, loader); + Assertions.assertEquals(1, loads[0], "second get is a hit"); + connector.invalidateTable("db1", "t1"); + cache.get(db1t1, loader); + Assertions.assertEquals(2, loads[0], "REFRESH TABLE forces a reload of db1.t1"); + + // REFRESH DATABASE db1 -> db1.t2 re-loads; db2.t1 unaffected. + cache.get(db1t2, loader); // loads=3 (miss) + cache.get(db2t1, loader); // loads=4 (miss) + cache.get(db1t2, loader); // hit + cache.get(db2t1, loader); // hit + Assertions.assertEquals(4, loads[0]); + connector.invalidateDb("db1"); + cache.get(db2t1, loader); // db2 untouched -> hit + Assertions.assertEquals(4, loads[0], "REFRESH DATABASE db1 must NOT drop db2's entries"); + cache.get(db1t2, loader); // db1.t2 dropped -> miss + Assertions.assertEquals(5, loads[0], "REFRESH DATABASE db1 drops db1's entries"); + + // REFRESH CATALOG -> everything re-loads. + connector.invalidateAll(); + cache.get(db2t1, loader); + Assertions.assertEquals(6, loads[0], "REFRESH CATALOG drops everything"); + } } diff --git a/fe/fe-connector/fe-connector-paimon/src/test/java/org/apache/doris/connector/paimon/PaimonConnectorMetadataPartitionViewCacheTest.java b/fe/fe-connector/fe-connector-paimon/src/test/java/org/apache/doris/connector/paimon/PaimonConnectorMetadataPartitionViewCacheTest.java new file mode 100644 index 00000000000000..e93126549df16d --- /dev/null +++ b/fe/fe-connector/fe-connector-paimon/src/test/java/org/apache/doris/connector/paimon/PaimonConnectorMetadataPartitionViewCacheTest.java @@ -0,0 +1,262 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.doris.connector.paimon; + +import org.apache.doris.connector.api.ConnectorPartitionInfo; +import org.apache.doris.connector.api.pushdown.ConnectorExpression; +import org.apache.doris.connector.cache.ConnectorPartitionViewCache; + +import org.apache.paimon.partition.Partition; +import org.apache.paimon.types.DataTypes; +import org.apache.paimon.types.RowType; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +import java.util.Arrays; +import java.util.Collections; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.OptionalLong; +import java.util.stream.Collectors; + +/** + * PERF-06 tests for the cross-query DERIVED partition-view cache ("cache A", the generic + * {@link ConnectorPartitionViewCache}) wired into {@link PaimonConnectorMetadata#listPartitions}. Paimon does + * NOT override {@code getMvccPartitionView} (the generic MTMV model falls back to its default + * listPartitions/LIST/timestamp path), so — unlike iceberg's two typed fields — there is exactly ONE + * enumeration hook to wrap. + * + *

Uses the real {@link RecordingPaimonCatalogOps} + {@link FakePaimonTable} harness (no Mockito, no docker): + * {@link PaimonConnectorMetadata#collectPartitions}'s first (and only) remote call is + * {@code catalogOps.listPartitions(Identifier)}, logged as {@code "listPartitions:db1.t1"}, so a cache HIT skips + * the whole loader and that log count is the enumeration counter — the SAME proxy + * {@link PaimonConnectorMetadataPartitionTest} already relies on ({@code ops.log.contains("listPartitions:...")}). + * + *

Every test uses a DISABLED {@link PaimonLatestSnapshotCache} ({@code ttl-second <= 0}, always-live) so the + * cache-A key's {@code snapshotId} component is driven directly and deterministically by + * {@code ops.latestSnapshotId}, isolating cache A (the only caching layer under test) — mirroring how the + * iceberg PERF-06 tests pass the raw partition cache as {@code null} throughout. + */ +public class PaimonConnectorMetadataPartitionViewCacheTest { + + private static PaimonConnectorMetadata metadataWithCache(RecordingPaimonCatalogOps ops, + ConnectorPartitionViewCache> cache) { + return new PaimonConnectorMetadata(ops, Collections.emptyMap(), new RecordingConnectorContext(), + new PaimonSchemaAtMemo(PaimonSchemaAtMemo.DEFAULT_MAX_SIZE), + new PaimonLatestSnapshotCache(0L, 1), cache); + } + + private static ConnectorPartitionViewCache> partitionViewCache() { + return new ConnectorPartitionViewCache<>("paimon", Collections.emptyMap()); + } + + private static RowType regionRowType() { + return RowType.builder() + .field("id", DataTypes.INT()) + .field("region", DataTypes.STRING()) + .build(); + } + + private static PaimonTableHandle handle(FakePaimonTable table) { + PaimonTableHandle h = new PaimonTableHandle( + "db1", "t1", Collections.singletonList("region"), Collections.emptyList()); + h.setPaimonTable(table); + return h; + } + + private static FakePaimonTable regionTable() { + FakePaimonTable table = new FakePaimonTable( + "t1", regionRowType(), Collections.singletonList("region"), Collections.emptyList()); + table.setOptions(Collections.singletonMap("partition.legacy-name", "true")); + return table; + } + + private static Partition partition(String regionValue) { + Map spec = new LinkedHashMap<>(); + spec.put("region", regionValue); + return new Partition(spec, 1L, 1L, 1, 1L, true); + } + + private static long loadCount(RecordingPaimonCatalogOps ops) { + return ops.log.stream().filter(s -> s.equals("listPartitions:db1.t1")).count(); + } + + private static List names(List infos) { + return infos.stream().map(ConnectorPartitionInfo::getPartitionName).collect(Collectors.toList()); + } + + @Test + public void listPartitionsCachesDerivedListAcrossQueries() { + // WHY: cache A must memoize the BUILT List keyed by (db, table, snapshotId, + // schemaId), so a repeated query on the same (unchanged) latest snapshot skips the derived rebuild AND + // the remote catalogOps.listPartitions round-trip. MUTATION: not consulting the cache (compute directly + // every call) -> the seam runs twice -> red. + RecordingPaimonCatalogOps ops = new RecordingPaimonCatalogOps(); + FakePaimonTable table = regionTable(); + ops.table = table; + ops.latestSnapshotId = OptionalLong.of(100L); + ops.partitions = Arrays.asList(partition("cn"), partition("us")); + ConnectorPartitionViewCache> cache = partitionViewCache(); + PaimonConnectorMetadata md = metadataWithCache(ops, cache); + PaimonTableHandle h = handle(table); + + List first = md.listPartitions(null, h, Optional.empty()); + List second = md.listPartitions(null, h, Optional.empty()); + + Assertions.assertEquals(Arrays.asList("region=cn", "region=us"), names(first)); + Assertions.assertEquals(names(first), names(second), "the cached list is returned verbatim"); + Assertions.assertEquals(1, loadCount(ops), "a cache hit must not re-enumerate (listPartitions once)"); + } + + @Test + public void listPartitionsDifferentSnapshotReEnumerates() { + // WHY: the underlying remote enumeration always reflects the CURRENT catalog state (it is + // base-identifier-only, never pinned to a historical snapshot -- see partitionViewCacheKey's javadoc), + // so the key must track "current" via latestSnapshotCache: a data change (new latest snapshot id) + // must mint a new key and re-enumerate. MUTATION: keying on (db,table) only, ignoring snapshotId -> + // the stale S1 list would be served after the snapshot changed -> loadCount/contents below red. + RecordingPaimonCatalogOps ops = new RecordingPaimonCatalogOps(); + FakePaimonTable table = regionTable(); + ops.table = table; + ConnectorPartitionViewCache> cache = partitionViewCache(); + PaimonConnectorMetadata md = metadataWithCache(ops, cache); + PaimonTableHandle h = handle(table); + + ops.latestSnapshotId = OptionalLong.of(100L); + ops.partitions = Collections.singletonList(partition("cn")); + List atS1 = md.listPartitions(null, h, Optional.empty()); + + ops.latestSnapshotId = OptionalLong.of(200L); + ops.partitions = Arrays.asList(partition("cn"), partition("us")); + List atS2 = md.listPartitions(null, h, Optional.empty()); + + Assertions.assertEquals(Collections.singletonList("region=cn"), names(atS1)); + Assertions.assertEquals(Arrays.asList("region=cn", "region=us"), names(atS2)); + Assertions.assertEquals(2, loadCount(ops), "distinct snapshot keys must each enumerate"); + } + + @Test + public void listPartitionsInvalidateTableForcesReEnumeration() { + // WHY: REFRESH TABLE (PaimonConnector.invalidateTable -> cache.invalidateTable) must drop the cached + // list so the next query re-enumerates live. MUTATION: invalidateTable not wired -> the second call + // hits the stale entry -> loadCount stays 1 -> red. + RecordingPaimonCatalogOps ops = new RecordingPaimonCatalogOps(); + FakePaimonTable table = regionTable(); + ops.table = table; + ops.latestSnapshotId = OptionalLong.of(100L); + ops.partitions = Collections.singletonList(partition("cn")); + ConnectorPartitionViewCache> cache = partitionViewCache(); + PaimonConnectorMetadata md = metadataWithCache(ops, cache); + PaimonTableHandle h = handle(table); + + md.listPartitions(null, h, Optional.empty()); + cache.invalidateTable("db1", "t1"); + md.listPartitions(null, h, Optional.empty()); + Assertions.assertEquals(2, loadCount(ops), "invalidateTable must force a re-enumeration"); + } + + @Test + public void listPartitionsInvalidateAllForcesReEnumeration() { + // WHY: REFRESH CATALOG (PaimonConnector.invalidateAll -> cache.invalidateAll) must drop the cached + // list. MUTATION: invalidateAll not wired -> the second call hits -> loadCount stays 1 -> red. + RecordingPaimonCatalogOps ops = new RecordingPaimonCatalogOps(); + FakePaimonTable table = regionTable(); + ops.table = table; + ops.latestSnapshotId = OptionalLong.of(100L); + ops.partitions = Collections.singletonList(partition("cn")); + ConnectorPartitionViewCache> cache = partitionViewCache(); + PaimonConnectorMetadata md = metadataWithCache(ops, cache); + PaimonTableHandle h = handle(table); + + md.listPartitions(null, h, Optional.empty()); + cache.invalidateAll(); + md.listPartitions(null, h, Optional.empty()); + Assertions.assertEquals(2, loadCount(ops), "invalidateAll must force a re-enumeration"); + } + + @Test + public void listPartitionsWithFilterBypassesCache() { + // WHY: a present filter must BYPASS the cache and compute directly -- and must NOT populate it either, + // so a later empty-filter (pruning) call still misses. legacy ignores the filter value entirely + // (returns the full set regardless), but the cache-A key carries no filter dimension, so caching a + // filtered call's result would be indistinguishable from caching an unfiltered one; bypassing keeps the + // two paths independent, mirroring the iceberg pattern. MUTATION: caching regardless of filter -> the + // second filtered call hits (loadCount stays 1) -> red; or a bypassed call populating the cache -> the + // following empty-filter call hits instead of missing -> red. + RecordingPaimonCatalogOps ops = new RecordingPaimonCatalogOps(); + FakePaimonTable table = regionTable(); + ops.table = table; + ops.latestSnapshotId = OptionalLong.of(100L); + ops.partitions = Collections.singletonList(partition("cn")); + ConnectorPartitionViewCache> cache = partitionViewCache(); + PaimonConnectorMetadata md = metadataWithCache(ops, cache); + PaimonTableHandle h = handle(table); + + ConnectorExpression filter = Collections::emptyList; // any non-empty filter + md.listPartitions(null, h, Optional.of(filter)); + md.listPartitions(null, h, Optional.of(filter)); + Assertions.assertEquals(2, loadCount(ops), "a present filter must bypass the cache (compute every call)"); + md.listPartitions(null, h, Optional.empty()); + Assertions.assertEquals(3, loadCount(ops), "the empty-filter call must miss (filtered calls never populate)"); + } + + @Test + public void listPartitionsNullCacheEnumeratesEveryCall() { + // The convenience/test-ctor analogue: a null cache means compute directly every call -> the seam runs + // on every query (no cross-query sharing). MUTATION: defaulting to a shared cache when null was + // intended -> loadCount 1 -> red. + RecordingPaimonCatalogOps ops = new RecordingPaimonCatalogOps(); + FakePaimonTable table = regionTable(); + ops.table = table; + ops.latestSnapshotId = OptionalLong.of(100L); + ops.partitions = Collections.singletonList(partition("cn")); + PaimonConnectorMetadata md = metadataWithCache(ops, null); + PaimonTableHandle h = handle(table); + + md.listPartitions(null, h, Optional.empty()); + md.listPartitions(null, h, Optional.empty()); + Assertions.assertEquals(2, loadCount(ops), "a null (disabled) cache must re-enumerate every call"); + } + + @Test + public void unpartitionedHandleBypassesCacheWithoutTouchingSnapshotSeam() { + // WHY: an unpartitioned handle must short-circuit to empty WITHOUT touching either seam + // (latestSnapshotId or listPartitions) -- mirrors collectPartitions' own pre-existing contract + // (PaimonConnectorMetadataPartitionTest#nonPartitionedHandleReturnsEmptyWithoutSeamCall). Building a + // cache-A key would otherwise call latestSnapshotCache -> catalogOps.latestSnapshotId for a table that + // is guaranteed to return an empty list, polluting the cache and the seam log for nothing. MUTATION: + // building the key before the emptiness check -> "latestSnapshotId" appears in ops.log -> red. + RecordingPaimonCatalogOps ops = new RecordingPaimonCatalogOps(); + FakePaimonTable table = new FakePaimonTable( + "t1", RowType.builder().field("id", DataTypes.INT()).build(), + Collections.emptyList(), Collections.emptyList()); + ops.table = table; + PaimonTableHandle h = new PaimonTableHandle( + "db1", "t1", Collections.emptyList(), Collections.emptyList()); + h.setPaimonTable(table); + ConnectorPartitionViewCache> cache = partitionViewCache(); + PaimonConnectorMetadata md = metadataWithCache(ops, cache); + + List result = md.listPartitions(null, h, Optional.empty()); + + Assertions.assertTrue(result.isEmpty()); + Assertions.assertTrue(ops.log.isEmpty(), "an unpartitioned handle must not touch any remote seam"); + } +} From 75fcd04e19cb0e8c7bf485cfb785426086b51112 Mon Sep 17 00:00:00 2001 From: morningman Date: Tue, 21 Jul 2026 03:49:52 +0800 Subject: [PATCH 14/19] [feat](catalog) enable Cache B for snapshot-less connectors via a partition-name-set version token 5c17b748880 disabled cross-query SortedPartitionRanges reuse (Cache B) for hive (and any snapshot-less MVCC-wrapped table) because the "@" version token is a constant "-1@" when the pinned connector snapshot has no real snapshot id, so a cache hit could silently serve ranges built from a stale partition set. PluginDrivenMvccExternalTable#getPartitionMetaVersion now derives the token from the frozen partition NAME SET (a compact HashSet copy of getOriginPartitions(scan).keySet()) when snapshotId == -1, instead of the constant string. Since getOriginPartitions and getPartitionMetaVersion read the SAME pin, version == exact content the ranges are built from, so the cache rebuilds precisely when the partition set changes -- no staleness risk, no SPI change, no generation counter. iceberg/paimon (snapshotId != -1) keep the unchanged "@" token. With the version now correct for snapshot-less connectors too, the getSortedPartitionRanges override (added in 5c17b748880 solely to gate on snapshotId == -1) is a pure pass-through to ExternalTable's cache-manager delegation, so it is deleted. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_014ZngdXGBAXNDRe7jA42mAS --- .../mvcc/PluginDrivenMvccExternalTable.java | 41 +++--- ...tedPartitionsCacheManagerExternalTest.java | 126 ++++++++++++++---- 2 files changed, 118 insertions(+), 49 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/datasource/mvcc/PluginDrivenMvccExternalTable.java b/fe/fe-core/src/main/java/org/apache/doris/datasource/mvcc/PluginDrivenMvccExternalTable.java index c6205e723a2a52..2a48a6a6072088 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/datasource/mvcc/PluginDrivenMvccExternalTable.java +++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/mvcc/PluginDrivenMvccExternalTable.java @@ -55,7 +55,6 @@ import org.apache.doris.mtmv.MTMVSnapshotIdSnapshot; import org.apache.doris.mtmv.MTMVSnapshotIf; import org.apache.doris.mtmv.MTMVTimestampSnapshot; -import org.apache.doris.nereids.rules.expression.rules.SortedPartitionRanges; import org.apache.doris.nereids.trees.plans.algebra.CatalogRelation; import org.apache.doris.nereids.trees.plans.logical.LogicalFileScan; @@ -68,6 +67,7 @@ import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Optional; @@ -635,8 +635,17 @@ public Map getOriginPartitions(CatalogRelation scan) { @Override public Object getPartitionMetaVersion(CatalogRelation scan) { ConnectorMvccSnapshot cs = getOrMaterialize(pinnedSnapshot(scan)).getConnectorSnapshot(); - // Opaque version token: iceberg/paimon expose an immutable (snapshotId, schemaId) pair. - return cs.getSnapshotId() + "@" + cs.getSchemaId(); + if (cs.getSnapshotId() != -1L) { + // Opaque version token: iceberg/paimon expose an immutable (snapshotId, schemaId) pair. + return cs.getSnapshotId() + "@" + cs.getSchemaId(); + } + // Snapshot-less connector (e.g. hive): no real MVCC snapshot id to version against. Derive the + // token from the frozen partition NAME SET instead -- getOriginPartitions(scan) reads the SAME + // pin, so version == exact content the ranges are built from: the cache rebuilds precisely when + // the partition set changes, never on a stale set. A compact copy of just the name strings (NOT + // the live keySet view), so the cross-query cache entry does not retain the whole pin's + // partition-item map. + return new HashSet<>(getOriginPartitions(scan).keySet()); } @Override @@ -646,28 +655,10 @@ public long getPartitionMetaLoadTimeMillis(CatalogRelation scan) { return 0L; } - @Override - public Optional> getSortedPartitionRanges(CatalogRelation scan) { - // Cache B (NereidsSortedPartitionsCacheManager) is keyed by getPartitionMetaVersion(scan), i.e. - // "@". That token only detects a partition-set change when snapshotId is a - // REAL, monotonically-changing value (iceberg/paimon). Hive's beginQuerySnapshot always pins the - // sentinel snapshotId == -1 (hive has no MVCC snapshot), so the token would be the CONSTANT - // "-1@" across queries -- a cache hit could then serve ranges built from an OLDER - // partition set than the current pin's nameToPartitionItem (e.g. after a CachingHmsClient TTL - // refresh with no invalidate event), silently pruning against a stale range set. Skip the cache - // for a -1 (non-MVCC / sentinel) snapshot: return empty so the caller (PruneFileScanPartition) - // builds ranges fresh from THIS query's pin every time, which is always consistent with it -- the - // pre-cache behavior. A real snapshot id (iceberg/paimon) keeps the cache via super's delegation - // to the cache manager. An EMPTY iceberg table also pins snapshotId == -1, but its - // nameToPartitionItem is then empty too, and PruneFileScanPartition's - // "!nameToPartitionItem.isEmpty()" guard already skips ranges entirely -- so gating on -1 here is - // safe for that case as well. - ConnectorMvccSnapshot cs = getOrMaterialize(pinnedSnapshot(scan)).getConnectorSnapshot(); - if (cs.getSnapshotId() == -1L) { - return Optional.empty(); - } - return super.getSortedPartitionRanges(scan); - } + // getSortedPartitionRanges is now a pure pass-through to ExternalTable's cache-manager delegation + // (5c17b748880's snapshotId==-1 short-circuit was removed: getPartitionMetaVersion above derives a + // content-comparable version for snapshot-less connectors, so Cache B is now correct for them too) -- + // no override needed here. private Optional pinnedSnapshot(CatalogRelation scan) { if (scan instanceof LogicalFileScan) { diff --git a/fe/fe-core/src/test/java/org/apache/doris/common/cache/NereidsSortedPartitionsCacheManagerExternalTest.java b/fe/fe-core/src/test/java/org/apache/doris/common/cache/NereidsSortedPartitionsCacheManagerExternalTest.java index 5ebae7ed4361e8..9cfb03ae14d2d0 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/common/cache/NereidsSortedPartitionsCacheManagerExternalTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/common/cache/NereidsSortedPartitionsCacheManagerExternalTest.java @@ -297,24 +297,32 @@ public void testPluginDrivenMvccExternalTableRealOriginPartitionsAndVersion() th } } - // ──────────────────── Hive sentinel snapshot (-1) must skip Cache B ──────────────────── + // ──────────────────── Hive sentinel snapshot (-1) now uses Cache B via a NAME-SET version ───── // - // getPartitionMetaVersion's token is "@". Hive's beginQuerySnapshot always pins - // snapshotId == -1 (no real MVCC snapshot), so that token is a CONSTANT across queries and cannot - // detect a partition-set change. PluginDrivenMvccExternalTable#getSortedPartitionRanges must therefore - // gate on the pinned connector snapshot's snapshotId and return empty (skip Cache B, build fresh every - // query) rather than delegating to the cache manager. + // getPartitionMetaVersion's token is "@" for a REAL snapshot id, but hive's + // beginQuerySnapshot always pins the sentinel snapshotId == -1 (hive has no MVCC snapshot), so that + // token would be a CONSTANT across queries. Instead, for a snapshot-less pin the version is derived + // from the frozen partition NAME SET (getOriginPartitions(scan).keySet(), copied) -- the SAME map the + // ranges are built from, so version == exact content and the cache rebuilds precisely when the + // partition set changes. PluginDrivenMvccExternalTable no longer overrides getSortedPartitionRanges + // (the -1 short-circuit added in 5c17b748880 was removed): a snapshot-less pin now goes through the + // inherited ExternalTable#getSortedPartitionRanges -> the cache manager, exactly like iceberg/paimon. - @Test - public void testGetSortedPartitionRangesEmptyForSentinelSnapshotId() throws Exception { - Map pinnedParts = Maps.newHashMap(); - pinnedParts.put("id=1", listItem(1)); - // Hive's sentinel: no real MVCC snapshot id. - ConnectorMvccSnapshot sentinelSnapshot = ConnectorMvccSnapshot.builder() - .snapshotId(-1L).schemaId(0L).build(); - PluginDrivenMvccSnapshot pin = new PluginDrivenMvccSnapshot( - sentinelSnapshot, pinnedParts, Maps.newHashMap()); + /** + * Builds a live Env whose {@code getSortedPartitionsCacheManager()} returns {@code rangesCacheMgr}, + * mockStatic-scoped so {@code table.getSortedPartitionRanges(scan)} (now purely inherited from + * {@link ExternalTable}, no override) can resolve {@code Env.getCurrentEnv()} on the real dispatch path. + */ + private static MockedStatic mockEnvWithRangesCacheManager(NereidsSortedPartitionsCacheManager rangesCacheMgr) { + Env env = Mockito.mock(Env.class); + Mockito.when(env.getSortedPartitionsCacheManager()).thenReturn(rangesCacheMgr); + MockedStatic envStatic = Mockito.mockStatic(Env.class); + envStatic.when(Env::getCurrentEnv).thenReturn(env); + return envStatic; + } + /** Builds a {@code PluginDrivenMvccExternalTable} mock wired for the real getOriginPartitions/getPartitionMetaVersion/pinnedSnapshot bodies (same technique as the other real-wiring tests in this file). */ + private static PluginDrivenMvccExternalTable newRealWiringTable() { PluginDrivenMvccExternalTable table = Mockito.mock(PluginDrivenMvccExternalTable.class, Mockito.CALLS_REAL_METHODS); ExternalDatabase database = Mockito.mock(ExternalDatabase.class); @@ -325,25 +333,95 @@ public void testGetSortedPartitionRangesEmptyForSentinelSnapshotId() throws Exce Mockito.when(database.getCatalog()).thenReturn((CatalogIf) catalog); Mockito.when(catalog.getName()).thenReturn(CTL); Mockito.doReturn(SelectedPartitions.NOT_PRUNED).when(table).initSelectedPartitions(Mockito.any()); + return table; + } + /** Pins {@code pin} onto a FRESH ConnectContext/StatementContext ("a new query") and returns the scan built over it. */ + private static LogicalFileScan pinLatestAndBuildScan(PluginDrivenMvccExternalTable table, PluginDrivenMvccSnapshot pin) { ConnectContext ctx = new ConnectContext(); StatementContext stmtCtx = new StatementContext(ctx, null); ctx.setStatementContext(stmtCtx); ctx.setThreadLocalInfo(); - try { - // B5a implicit query-begin (latest) pin -- mirrors a plain (no @tag/@branch/time-travel) hive scan. - Mockito.doReturn(pin).when(table).loadSnapshot(Optional.empty(), Optional.empty()); - stmtCtx.loadSnapshots(table, Optional.empty(), Optional.empty()); + // B5a implicit query-begin (latest) pin -- mirrors a plain (no @tag/@branch/time-travel) hive scan. + Mockito.doReturn(pin).when(table).loadSnapshot(Optional.empty(), Optional.empty()); + stmtCtx.loadSnapshots(table, Optional.empty(), Optional.empty()); + return new LogicalFileScan(new RelationId(1), table, + Collections.singletonList(DB), Collections.emptyList(), + Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty()); + } - LogicalFileScan scan = new LogicalFileScan(new RelationId(1), table, - Collections.singletonList(DB), Collections.emptyList(), - Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty()); + private static PluginDrivenMvccSnapshot sentinelPin(Map parts) { + ConnectorMvccSnapshot sentinelSnapshot = ConnectorMvccSnapshot.builder() + .snapshotId(-1L).schemaId(0L).build(); + return new PluginDrivenMvccSnapshot(sentinelSnapshot, parts, Maps.newHashMap()); + } + + @Test + public void testGetSortedPartitionRangesPresentForSnapshotLessNonEmptyPartitions() throws Exception { + Map pinnedParts = Maps.newHashMap(); + pinnedParts.put("id=1", listItem(1)); + pinnedParts.put("id=2", listItem(2)); + PluginDrivenMvccExternalTable table = newRealWiringTable(); + + NereidsSortedPartitionsCacheManager rangesCacheMgr = new NereidsSortedPartitionsCacheManager(); + try (MockedStatic envStatic = mockEnvWithRangesCacheManager(rangesCacheMgr)) { + LogicalFileScan scan = pinLatestAndBuildScan(table, sentinelPin(pinnedParts)); + + Optional> ranges = table.getSortedPartitionRanges(scan); + + Assertions.assertTrue(ranges.isPresent(), + "snapshotId == -1 (hive sentinel) with a non-empty partition set must now use Cache B, " + + "keyed by the partition NAME-SET version token"); + } finally { + ConnectContext.remove(); + } + } + + @Test + public void testGetSortedPartitionRangesRebuildsWhenSnapshotLessNameSetChanges() throws Exception { + PluginDrivenMvccExternalTable table = newRealWiringTable(); + NereidsSortedPartitionsCacheManager rangesCacheMgr = new NereidsSortedPartitionsCacheManager(); + try (MockedStatic envStatic = mockEnvWithRangesCacheManager(rangesCacheMgr)) { + Map partsAtQuery1 = Maps.newHashMap(); + partsAtQuery1.put("id=1", listItem(1)); + partsAtQuery1.put("id=2", listItem(2)); + LogicalFileScan scan1 = pinLatestAndBuildScan(table, sentinelPin(partsAtQuery1)); + SortedPartitionRanges first = table.getSortedPartitionRanges(scan1).orElse(null); + Assertions.assertNotNull(first, "ranges built and cached at the first (2-partition) name set"); + Assertions.assertEquals(2, first.sortedPartitions.size()); + + // A second query ("ALTER ADD PARTITION" between queries): the pin's partition NAME SET grew. + // The manager's Objects.equals compares the two HashSet version tokens by CONTENT, so it must + // detect this change even though snapshotId is still the constant -1 on both pins. + Map partsAtQuery2 = Maps.newHashMap(); + partsAtQuery2.put("id=1", listItem(1)); + partsAtQuery2.put("id=2", listItem(2)); + partsAtQuery2.put("id=3", listItem(3)); + LogicalFileScan scan2 = pinLatestAndBuildScan(table, sentinelPin(partsAtQuery2)); + SortedPartitionRanges rebuilt = table.getSortedPartitionRanges(scan2).orElse(null); + + Assertions.assertNotNull(rebuilt, "ranges rebuilt at the second (3-partition) name set"); + Assertions.assertNotSame(first, rebuilt, + "name-set change at a constant snapshotId==-1 must still trigger a rebuild " + + "(the version token is content-derived from the partition names, not the snapshot id)"); + Assertions.assertEquals(3, rebuilt.sortedPartitions.size(), "rebuilt from the new partition set"); + } finally { + ConnectContext.remove(); + } + } + + @Test + public void testGetSortedPartitionRangesEmptyForSnapshotLessEmptyPartitions() throws Exception { + PluginDrivenMvccExternalTable table = newRealWiringTable(); + NereidsSortedPartitionsCacheManager rangesCacheMgr = new NereidsSortedPartitionsCacheManager(); + try (MockedStatic envStatic = mockEnvWithRangesCacheManager(rangesCacheMgr)) { + LogicalFileScan scan = pinLatestAndBuildScan(table, sentinelPin(Maps.newHashMap())); Optional> ranges = table.getSortedPartitionRanges(scan); Assertions.assertFalse(ranges.isPresent(), - "snapshotId == -1 (hive sentinel) must skip Cache B and yield empty, so the caller " - + "(PruneFileScanPartition) builds ranges fresh from this query's pin"); + "an empty partition set (snapshot-less or not) yields no ranges to cache " + + "(SortedPartitionRanges.build(emptyMap) == null)"); } finally { ConnectContext.remove(); } From cd1d080fe620463b89cebce1db4c1f684cf786b7 Mon Sep 17 00:00:00 2001 From: morningman Date: Tue, 21 Jul 2026 04:09:24 +0800 Subject: [PATCH 15/19] [perf](catalog) wire cross-query derived partition-view cache A into the hive connector (S6) Layer the generic ConnectorPartitionViewCache (S2 framework) above hive's raw per-name HMS listing (already cached by CachingHmsClient) so a repeated query on a partitioned hive table skips the derived-view BUILD -- the per-name HiveWriteUtils.toPartitionValues parse + ConnectorPartitionInfo construction in HiveConnectorMetadata#listPartitions -- not just the metastore round-trip. Mirrors the paimon wiring (S4, e3be4ad9b00): ONE typed field (hive's getMvccPartitionView returns the SPI default Optional.empty() for a real hive handle, so listPartitions is the only enumeration hook to wrap). - Key = (db, table, -1, -1). Hive is snapshot-less (beginQuerySnapshot always pins -1) and HiveTableHandle carries no schema version, so both axes are pinned "unversioned" -- simpler than paimon (no per-catalog snapshot lookup needed to build the key) and there is no cross-caller keying concern since every query for a table always shares one key. - listPartitions: extracted the existing body into listPartitionsUncached (the derivation seam); the public method bypasses the cache (compute directly, never populated) when partitionViewCache is null (the convenience/test ctors) or filter is present. - Hive has NO iceberg.rest.session=user-style per-user cache-disabling convention (its per-catalog caches -- CachingHmsClient, HiveFileListingCache -- are already built unconditionally), so the cache is constructed unconditionally on HiveConnector. - Invalidation routed from invalidateTable/invalidateDb/invalidateAll to the new cache alongside CachingHmsClient/HiveFileListingCache; invalidatePartition (hive overrides the SPI no-op default) also routes to invalidateTable -- cache A's key carries no partition-name axis, so a partition-level change can only invalidate the whole table's single entry. Tests: new HiveConnectorMetadataPartitionViewCacheTest (5) proves same-key-hit enumerates once (via a call-counting HmsClient double, the same FakeHmsClient idiom HiveConnectorMetadataPartitionListTest uses), invalidateTable/invalidateAll re-enumerate, filter bypass (both directions), and a null cache re-enumerates every call; HiveConnectorPartitionViewCacheTest (4) proves the cache is always built (no session=user gate), the REFRESH TABLE/DATABASE/CATALOG hooks route correctly on the real field, and invalidatePartition drops the whole table's entry. Confirmed RED without the wiring (compile failure: no matching ctor / no partitionViewCacheForTest) before restoring the implementation. Full fe-connector-hive suite 360/360 green; checkstyle clean. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_014ZngdXGBAXNDRe7jA42mAS --- .../doris/connector/hive/HiveConnector.java | 33 ++- .../connector/hive/HiveConnectorMetadata.java | 48 ++++ ...nnectorMetadataPartitionViewCacheTest.java | 251 ++++++++++++++++++ .../HiveConnectorPartitionViewCacheTest.java | 144 ++++++++++ 4 files changed, 475 insertions(+), 1 deletion(-) create mode 100644 fe/fe-connector/fe-connector-hive/src/test/java/org/apache/doris/connector/hive/HiveConnectorMetadataPartitionViewCacheTest.java create mode 100644 fe/fe-connector/fe-connector-hive/src/test/java/org/apache/doris/connector/hive/HiveConnectorPartitionViewCacheTest.java diff --git a/fe/fe-connector/fe-connector-hive/src/main/java/org/apache/doris/connector/hive/HiveConnector.java b/fe/fe-connector/fe-connector-hive/src/main/java/org/apache/doris/connector/hive/HiveConnector.java index 1ba13db82c1b64..b11a66c3d8e348 100644 --- a/fe/fe-connector/fe-connector-hive/src/main/java/org/apache/doris/connector/hive/HiveConnector.java +++ b/fe/fe-connector/fe-connector-hive/src/main/java/org/apache/doris/connector/hive/HiveConnector.java @@ -20,6 +20,7 @@ import org.apache.doris.connector.api.Connector; import org.apache.doris.connector.api.ConnectorCapability; import org.apache.doris.connector.api.ConnectorMetadata; +import org.apache.doris.connector.api.ConnectorPartitionInfo; import org.apache.doris.connector.api.ConnectorSession; import org.apache.doris.connector.api.ConnectorTestResult; import org.apache.doris.connector.api.DorisConnectorException; @@ -28,6 +29,7 @@ import org.apache.doris.connector.api.procedure.ConnectorProcedureOps; import org.apache.doris.connector.api.scan.ConnectorScanPlanProvider; import org.apache.doris.connector.api.write.ConnectorWritePlanProvider; +import org.apache.doris.connector.cache.ConnectorPartitionViewCache; import org.apache.doris.connector.hms.CachingHmsClient; import org.apache.doris.connector.hms.HmsClient; import org.apache.doris.connector.hms.HmsClientConfig; @@ -96,6 +98,19 @@ public class HiveConnector implements Connector { // enters SPI_READY_TYPES. Its metastore-metadata sibling is the CachingHmsClient wrapping the HmsClient. private final HiveFileListingCache fileListingCache; + // PERF-06 (S6): cross-query DERIVED partition-view cache ("cache A", the generic ConnectorPartitionViewCache + // from fe-connector-cache), layered ABOVE the raw per-name HMS listing served by CachingHmsClient: it + // memoizes the BUILT List (HiveConnectorMetadata#listPartitionsUncached's per-name + // HiveWriteUtils.toPartitionValues parse + ConnectorPartitionInfo construction), keyed by + // (db, table, -1, -1) — hive is snapshot-less (beginQuerySnapshot always pins -1) and its handle carries no + // schema version, so every query for a table shares ONE entry, invalidated by the hooks below + TTL. ONE + // typed field (like paimon): hive's getMvccPartitionView returns the SPI default (Optional.empty()) for a + // real hive handle, so listPartitions is the only enumeration hook to wrap. Unlike iceberg, hive has NO + // session=user / per-user credential-isolation cache-disabling convention (its per-catalog caches — + // CachingHmsClient, HiveFileListingCache — are already built unconditionally below), so this is constructed + // unconditionally too. + private final ConnectorPartitionViewCache> partitionViewCache; + // Embedded iceberg SIBLING connector: a flipped hms gateway delegates its iceberg-on-HMS tables to it. Built // once per gateway connector (lazily) in the iceberg plugin's OWN child-first classloader via // context.createSiblingConnector — never co-packaged into the hive zip (a second AWS SDK would poison S3 @@ -116,6 +131,9 @@ public HiveConnector(Map properties, ConnectorContext context) { this.properties = Collections.unmodifiableMap(properties); this.context = context; this.fileListingCache = new HiveFileListingCache(this.properties); + // Reads its own meta.cache.hive.partition_view.(enable|ttl-second|capacity) from the catalog properties + // via the framework's CacheSpec (default ON / 24h / 1000). + this.partitionViewCache = new ConnectorPartitionViewCache<>("hive", this.properties); } @Override @@ -170,7 +188,7 @@ private static String rootCauseMessage(Throwable t) { HiveConnectorMetadata newMetadata(HmsClient client) { return new HiveConnectorMetadata(client, properties, context, this::getOrCreateIcebergSibling, this::getOrCreateHudiSibling, this::resolveSiblingOwnerLabeled, - fileListingCache); + fileListingCache, partitionViewCache); } /** @@ -351,6 +369,9 @@ void invalidateTable(HmsClient client, String dbName, String tableName) { ((CachingHmsClient) client).flush(dbName, tableName); } fileListingCache.invalidateTable(dbName, tableName); + // PERF-06: also drop this table's cached derived partition-view entry, so the next listPartitions + // re-derives live. + partitionViewCache.invalidateTable(dbName, tableName); } /** @@ -373,6 +394,7 @@ void invalidateDb(HmsClient client, String dbName) { ((CachingHmsClient) client).flushDb(dbName); } fileListingCache.invalidateDb(dbName); + partitionViewCache.invalidateDb(dbName); } /** @@ -393,6 +415,7 @@ void invalidateAll(HmsClient client) { ((CachingHmsClient) client).flushAll(); } fileListingCache.invalidateAll(); + partitionViewCache.invalidateAll(); } /** @@ -422,6 +445,9 @@ void invalidatePartition(HmsClient client, String dbName, String tableName, List ((CachingHmsClient) client).invalidatePartitions(dbName, tableName, partitionValues); } fileListingCache.invalidatePartitions(dbName, tableName, partitionValues); + // PERF-06: cache A's key carries no partition-name axis (only db/table/-1/-1), so a partition-level + // change cannot be scoped finer than the whole table's single cached entry — invalidate it wholesale. + partitionViewCache.invalidateTable(dbName, tableName); } /** @@ -473,6 +499,11 @@ HiveFileListingCache fileListingCacheForTest() { return fileListingCache; } + /** Test-only: the derived listPartitions view cache (PERF-06). Never null (hive has no session=user gate). */ + ConnectorPartitionViewCache> partitionViewCacheForTest() { + return partitionViewCache; + } + private HmsClient getOrCreateClient() { if (hmsClient == null) { synchronized (this) { diff --git a/fe/fe-connector/fe-connector-hive/src/main/java/org/apache/doris/connector/hive/HiveConnectorMetadata.java b/fe/fe-connector/fe-connector-hive/src/main/java/org/apache/doris/connector/hive/HiveConnectorMetadata.java index 07f50ca76ca4a2..00cb6b9ee8c1ec 100644 --- a/fe/fe-connector/fe-connector-hive/src/main/java/org/apache/doris/connector/hive/HiveConnectorMetadata.java +++ b/fe/fe-connector/fe-connector-hive/src/main/java/org/apache/doris/connector/hive/HiveConnectorMetadata.java @@ -55,6 +55,8 @@ import org.apache.doris.connector.api.pushdown.ConnectorLiteral; import org.apache.doris.connector.api.pushdown.FilterApplicationResult; import org.apache.doris.connector.api.scan.ConnectorPartitionValues; +import org.apache.doris.connector.cache.ConnectorPartitionViewCache; +import org.apache.doris.connector.cache.PartitionViewCacheKey; import org.apache.doris.connector.hms.HiveShowCreateTableRenderer; import org.apache.doris.connector.hms.HmsClient; import org.apache.doris.connector.hms.HmsClientException; @@ -225,6 +227,17 @@ public class HiveConnectorMetadata implements ConnectorMetadata { // default (harmless for the direct-construction tests, which inject their file sizes and never list). private final HiveFileListingCache fileListingCache; + // PERF-06 (S6): cross-query DERIVED partition-view cache A (generic ConnectorPartitionViewCache), injected by + // the owning HiveConnector; null = no cross-query derived layer (the convenience/test ctors below pass null, + // matching every existing direct-construction test). Layered ABOVE the raw per-name HMS listing served by + // CachingHmsClient: a hit skips both the derived-view BUILD (the per-name HiveWriteUtils.toPartitionValues + // parse + ConnectorPartitionInfo construction in listPartitions) and the collectPartitionNames call, keyed by + // (db, table, snapshotId=-1, schemaId=-1) — hive is snapshot-less (beginQuerySnapshot always pins -1) and its + // handle carries no schema version, so both axes are pinned "unversioned". Consumed only by listPartitions: + // getMvccPartitionView returns Optional.empty() for a real hive handle (the SPI default), so fe-core's generic + // MTMV model already falls back to listPartitions for hive — there is no second enumeration hook to wrap. + private final ConnectorPartitionViewCache> partitionViewCache; + public HiveConnectorMetadata(HmsClient hmsClient, Map properties, ConnectorContext context) { this(hmsClient, properties, context, NO_ICEBERG_SIBLING, NO_HUDI_SIBLING, NO_SIBLING_OWNER); } @@ -237,11 +250,28 @@ public HiveConnectorMetadata(HmsClient hmsClient, Map properties new HiveFileListingCache(properties)); } + /** Convenience ctor without the PERF-06 derived partition-view cache (null -> listPartitions always live). */ public HiveConnectorMetadata(HmsClient hmsClient, Map properties, ConnectorContext context, Supplier icebergSiblingSupplier, Supplier hudiSiblingSupplier, Function siblingOwnerResolver, HiveFileListingCache fileListingCache) { + this(hmsClient, properties, context, icebergSiblingSupplier, hudiSiblingSupplier, siblingOwnerResolver, + fileListingCache, null); + } + + /** + * Full ctor used by {@link HiveConnector#newMetadata}, adding the PERF-06 derived partition-view cache + * (cache A): {@code partitionViewCache} memoizes {@link #listPartitions}'s built + * {@code List}, keyed by {@code (db, table, -1, -1)}. {@code null} for the + * convenience/test ctors (no cross-query derived layer -> compute directly every call). + */ + public HiveConnectorMetadata(HmsClient hmsClient, Map properties, ConnectorContext context, + Supplier icebergSiblingSupplier, + Supplier hudiSiblingSupplier, + Function siblingOwnerResolver, + HiveFileListingCache fileListingCache, + ConnectorPartitionViewCache> partitionViewCache) { this.hmsClient = hmsClient; this.properties = properties; this.context = context; @@ -249,6 +279,7 @@ public HiveConnectorMetadata(HmsClient hmsClient, Map properties this.hudiSiblingSupplier = hudiSiblingSupplier; this.siblingOwnerResolver = siblingOwnerResolver; this.fileListingCache = fileListingCache; + this.partitionViewCache = partitionViewCache; } /** @@ -1134,6 +1165,13 @@ public List listPartitionNames(ConnectorSession session, ConnectorTableH * (until then a hive MTMV base table's per-partition freshness is UNKNOWN, harmless while hive is * dormant). {@code rowCount}/{@code sizeBytes}/{@code fileCount} are likewise UNKNOWN — hive does not * declare {@code SUPPORTS_PARTITION_STATS} (legacy SHOW PARTITIONS lists names only).

+ * + *

PERF-06 cache A: the BUILT {@code List} is memoized across queries in + * {@link #partitionViewCache}, keyed by {@code (db, table, -1, -1)} (hive is snapshot-less and carries no + * schema version — see the field javadoc) — a hit skips both {@link #collectPartitionNames} (which is itself + * already CACHED by {@code CachingHmsClient}) and the per-name value-parse/derive loop below. The cache is + * BYPASSED (compute directly, never populated) when {@code partitionViewCache} is {@code null} (the + * convenience/test ctors) or {@code filter} is present (not the pruning path, and not keyed by filter). */ @Override public List listPartitions(ConnectorSession session, @@ -1142,6 +1180,16 @@ public List listPartitions(ConnectorSession session, return siblingMetadata(session, handle).listPartitions(session, handle, filter); } HiveTableHandle hiveHandle = (HiveTableHandle) handle; + if (partitionViewCache == null || filter.isPresent()) { + return listPartitionsUncached(hiveHandle); + } + PartitionViewCacheKey key = new PartitionViewCacheKey( + hiveHandle.getDbName(), hiveHandle.getTableName(), -1L, -1L); + return partitionViewCache.get(key, () -> listPartitionsUncached(hiveHandle)); + } + + /** The derivation seam PERF-06 cache A wraps: builds the full partition-view list, uncached. */ + private List listPartitionsUncached(HiveTableHandle hiveHandle) { List partKeyNames = hiveHandle.getPartitionKeyNames(); // Query partition pruning: CACHED listing (use_meta_cache contract; legacy pruned off HiveExternalMetaCache). List partitionNames = collectPartitionNames(hiveHandle, false); diff --git a/fe/fe-connector/fe-connector-hive/src/test/java/org/apache/doris/connector/hive/HiveConnectorMetadataPartitionViewCacheTest.java b/fe/fe-connector/fe-connector-hive/src/test/java/org/apache/doris/connector/hive/HiveConnectorMetadataPartitionViewCacheTest.java new file mode 100644 index 00000000000000..3b375114572190 --- /dev/null +++ b/fe/fe-connector/fe-connector-hive/src/test/java/org/apache/doris/connector/hive/HiveConnectorMetadataPartitionViewCacheTest.java @@ -0,0 +1,251 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.doris.connector.hive; + +import org.apache.doris.connector.api.ConnectorPartitionInfo; +import org.apache.doris.connector.api.ConnectorType; +import org.apache.doris.connector.api.pushdown.ConnectorColumnRef; +import org.apache.doris.connector.api.pushdown.ConnectorComparison; +import org.apache.doris.connector.api.pushdown.ConnectorExpression; +import org.apache.doris.connector.api.pushdown.ConnectorLiteral; +import org.apache.doris.connector.cache.ConnectorPartitionViewCache; +import org.apache.doris.connector.hms.HmsClient; +import org.apache.doris.connector.hms.HmsDatabaseInfo; +import org.apache.doris.connector.hms.HmsPartitionInfo; +import org.apache.doris.connector.hms.HmsTableInfo; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.stream.Collectors; + +/** + * PERF-06 (S6) tests for the cross-query DERIVED partition-view cache ("cache A", the generic + * {@link ConnectorPartitionViewCache}) wired into {@link HiveConnectorMetadata#listPartitions}. Hive does NOT + * override {@code getMvccPartitionView} for a real hive handle (it returns the SPI default + * {@code Optional.empty()} — fe-core's generic MTMV model already falls back to {@code listPartitions}), so — + * like paimon's single typed field — there is exactly ONE enumeration hook to wrap. + * + *

Uses a {@link CountingHmsClient} double (no Mockito, no docker): the derivation seam + * {@link HiveConnectorMetadata#listPartitions} wraps is {@code collectPartitionNames} -> + * {@code hmsClient.listPartitionNames}, so a cache HIT skips the whole loader (including the per-name + * {@link HiveWriteUtils#toPartitionValues} parse + {@link ConnectorPartitionInfo} construction) and the call + * count on {@code listPartitionNames} is the enumeration counter — the same seam + * {@link HiveConnectorMetadataPartitionListTest} already asserts seam-touch on. + * + *

Hive is snapshot-less (its {@code beginQuerySnapshot} always pins {@code -1}) and its + * {@link HiveTableHandle} carries no schema version, so cache A's key is always + * {@code (db, table, -1, -1)} for a given table — unlike iceberg/paimon there is no "distinct snapshot" + * dimension to re-key on; only the explicit invalidation hooks (+ TTL) can force a re-derive. + */ +public class HiveConnectorMetadataPartitionViewCacheTest { + + private static final List PART_KEYS = Arrays.asList("year", "month"); + private static final List PARTITIONS = Arrays.asList( + "year=2023/month=12", + "year=2024/month=01"); + + private static HiveConnectorMetadata metadataWithCache(CountingHmsClient client, + ConnectorPartitionViewCache> cache) { + return new HiveConnectorMetadata(client, Collections.emptyMap(), new FakeConnectorContext(), + () -> { + throw new UnsupportedOperationException(); + }, + () -> { + throw new UnsupportedOperationException(); + }, + handle -> { + throw new UnsupportedOperationException(); + }, + new HiveFileListingCache(Collections.emptyMap()), cache); + } + + private static ConnectorPartitionViewCache> partitionViewCache() { + return new ConnectorPartitionViewCache<>("hive", Collections.emptyMap()); + } + + private static HiveTableHandle handle() { + return new HiveTableHandle.Builder("db1", "t1", HiveTableType.HIVE) + .partitionKeyNames(PART_KEYS) + .build(); + } + + private static List names(List infos) { + return infos.stream().map(ConnectorPartitionInfo::getPartitionName).collect(Collectors.toList()); + } + + @Test + public void listPartitionsCachesDerivedListAcrossQueries() { + // WHY: cache A must memoize the BUILT List keyed by (db, table, -1, -1), so a + // repeated query on the same table skips the derived rebuild AND the hmsClient.listPartitionNames + // round-trip. MUTATION: not consulting the cache (compute directly every call) -> the seam runs twice + // -> red. + CountingHmsClient client = new CountingHmsClient(PARTITIONS); + ConnectorPartitionViewCache> cache = partitionViewCache(); + HiveConnectorMetadata md = metadataWithCache(client, cache); + HiveTableHandle h = handle(); + + List first = md.listPartitions(null, h, Optional.empty()); + List second = md.listPartitions(null, h, Optional.empty()); + + Assertions.assertEquals(PARTITIONS, names(first)); + Assertions.assertEquals(names(first), names(second), "the cached list is returned verbatim"); + Assertions.assertEquals(1, client.listPartitionNamesCalls, + "a cache hit must not re-enumerate (listPartitionNames once)"); + } + + @Test + public void listPartitionsInvalidateTableForcesReEnumeration() { + // WHY: REFRESH TABLE (HiveConnector.invalidateTable -> cache.invalidateTable) must drop the cached list + // so the next query re-enumerates live. MUTATION: invalidateTable not wired -> the second call hits the + // stale entry -> listPartitionNamesCalls stays 1 -> red. + CountingHmsClient client = new CountingHmsClient(PARTITIONS); + ConnectorPartitionViewCache> cache = partitionViewCache(); + HiveConnectorMetadata md = metadataWithCache(client, cache); + HiveTableHandle h = handle(); + + md.listPartitions(null, h, Optional.empty()); + cache.invalidateTable("db1", "t1"); + md.listPartitions(null, h, Optional.empty()); + Assertions.assertEquals(2, client.listPartitionNamesCalls, "invalidateTable must force a re-enumeration"); + } + + @Test + public void listPartitionsInvalidateAllForcesReEnumeration() { + // WHY: REFRESH CATALOG (HiveConnector.invalidateAll -> cache.invalidateAll) must drop the cached list. + // MUTATION: invalidateAll not wired -> the second call hits -> listPartitionNamesCalls stays 1 -> red. + CountingHmsClient client = new CountingHmsClient(PARTITIONS); + ConnectorPartitionViewCache> cache = partitionViewCache(); + HiveConnectorMetadata md = metadataWithCache(client, cache); + HiveTableHandle h = handle(); + + md.listPartitions(null, h, Optional.empty()); + cache.invalidateAll(); + md.listPartitions(null, h, Optional.empty()); + Assertions.assertEquals(2, client.listPartitionNamesCalls, "invalidateAll must force a re-enumeration"); + } + + @Test + public void listPartitionsWithFilterBypassesCache() { + // WHY: a present filter must BYPASS the cache and compute directly -- and must NOT populate it either, so + // a later empty-filter (pruning) call still misses. Hive already ignores the filter value entirely + // (returns the full set regardless), but the cache-A key carries no filter dimension, so caching a + // filtered call's result would be indistinguishable from caching an unfiltered one; bypassing keeps the + // two paths independent, mirroring the iceberg/paimon pattern. MUTATION: caching regardless of filter -> + // the second filtered call hits (count stays 1) -> red; or a bypassed call populating the cache -> the + // following empty-filter call hits instead of missing -> red. + CountingHmsClient client = new CountingHmsClient(PARTITIONS); + ConnectorPartitionViewCache> cache = partitionViewCache(); + HiveConnectorMetadata md = metadataWithCache(client, cache); + HiveTableHandle h = handle(); + + ConnectorExpression filter = new ConnectorComparison(ConnectorComparison.Operator.EQ, + new ConnectorColumnRef("year", ConnectorType.of("STRING")), + new ConnectorLiteral(ConnectorType.of("STRING"), "2024")); + md.listPartitions(null, h, Optional.of(filter)); + md.listPartitions(null, h, Optional.of(filter)); + Assertions.assertEquals(2, client.listPartitionNamesCalls, + "a present filter must bypass the cache (compute every call)"); + md.listPartitions(null, h, Optional.empty()); + Assertions.assertEquals(3, client.listPartitionNamesCalls, + "the empty-filter call must miss (filtered calls never populate)"); + } + + @Test + public void listPartitionsNullCacheEnumeratesEveryCall() { + // The convenience/test-ctor analogue: a null cache means compute directly every call -> the seam runs on + // every query (no cross-query sharing). MUTATION: defaulting to a shared cache when null was intended -> + // listPartitionNamesCalls stays 1 -> red. + CountingHmsClient client = new CountingHmsClient(PARTITIONS); + HiveConnectorMetadata md = metadataWithCache(client, null); + HiveTableHandle h = handle(); + + md.listPartitions(null, h, Optional.empty()); + md.listPartitions(null, h, Optional.empty()); + Assertions.assertEquals(2, client.listPartitionNamesCalls, "a null (disabled) cache must re-enumerate every call"); + } + + /** + * Minimal {@link HmsClient} double: {@code listPartitionNames} returns a fixed list and counts calls; + * {@code getPartitions} fails loud (the per-partition round-trip {@link HiveConnectorMetadata#listPartitions} + * must never make — mirrors {@link HiveConnectorMetadataPartitionListTest}'s FakeHmsClient). + */ + private static final class CountingHmsClient implements HmsClient { + private final List partitionNames; + int listPartitionNamesCalls; + + CountingHmsClient(List partitionNames) { + this.partitionNames = partitionNames; + } + + @Override + public List listPartitionNames(String dbName, String tableName, int maxParts) { + listPartitionNamesCalls++; + return partitionNames; + } + + @Override + public List getPartitions(String dbName, String tableName, List partNames) { + throw new AssertionError("get_partitions_by_names must not be called by partition listing"); + } + + @Override + public List listDatabases() { + throw new UnsupportedOperationException(); + } + + @Override + public HmsDatabaseInfo getDatabase(String dbName) { + throw new UnsupportedOperationException(); + } + + @Override + public List listTables(String dbName) { + throw new UnsupportedOperationException(); + } + + @Override + public boolean tableExists(String dbName, String tableName) { + throw new UnsupportedOperationException(); + } + + @Override + public HmsTableInfo getTable(String dbName, String tableName) { + throw new UnsupportedOperationException(); + } + + @Override + public Map getDefaultColumnValues(String dbName, String tableName) { + throw new UnsupportedOperationException(); + } + + @Override + public HmsPartitionInfo getPartition(String dbName, String tableName, List values) { + throw new UnsupportedOperationException(); + } + + @Override + public void close() { + } + } +} diff --git a/fe/fe-connector/fe-connector-hive/src/test/java/org/apache/doris/connector/hive/HiveConnectorPartitionViewCacheTest.java b/fe/fe-connector/fe-connector-hive/src/test/java/org/apache/doris/connector/hive/HiveConnectorPartitionViewCacheTest.java new file mode 100644 index 00000000000000..57c765f9482e90 --- /dev/null +++ b/fe/fe-connector/fe-connector-hive/src/test/java/org/apache/doris/connector/hive/HiveConnectorPartitionViewCacheTest.java @@ -0,0 +1,144 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.doris.connector.hive; + +import org.apache.doris.connector.api.ConnectorPartitionInfo; +import org.apache.doris.connector.cache.ConnectorPartitionViewCache; +import org.apache.doris.connector.cache.PartitionViewCacheKey; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.function.Supplier; + +/** + * PERF-06 (S6) tests for {@link HiveConnector}'s partition-view cache A wiring: that the cache is ALWAYS built + * (no session=user gate — hive has no per-query-identity cache-disabling convention; its caches, like + * {@link HiveFileListingCache}, are already built unconditionally per-catalog) and that the REFRESH hooks + + * {@code invalidatePartition} route to it. + */ +public class HiveConnectorPartitionViewCacheTest { + + private static Map props() { + Map m = new HashMap<>(); + m.put("hive.metastore.uris", "thrift://host:9083"); + return m; + } + + @Test + public void partitionViewCacheAlwaysBuilt() { + // Unlike iceberg, hive has NO session=user / per-user credential-isolation cache-disabling convention (a + // hive catalog authenticates at catalog-creation time, not per-query session identity), so the connector + // must construct cache A unconditionally. MUTATION: a stray session-like gate leaving the field null on + // some property combination -> red. + HiveConnector connector = new HiveConnector(props(), new FakeConnectorContext()); + Assertions.assertNotNull(connector.partitionViewCacheForTest(), + "a fresh hive connector must always build the partition-view cache"); + } + + @Test + public void refreshHooksInvalidatePartitionViewCache() { + // The REFRESH hooks must clear cache A too (else external DDL/writes stay invisible beyond the TTL): + // REFRESH TABLE drops that table's entry, REFRESH DATABASE that db's, REFRESH CATALOG everything. + // Asserted via a counting loader (the framework's size() is package-private): after invalidation the + // loader must run again. MUTATION: an invalidate* hook not routed to the view cache -> the entry + // survives -> loader not re-run -> a loads assert below red. + HiveConnector connector = new HiveConnector(props(), new FakeConnectorContext()); + ConnectorPartitionViewCache> cache = connector.partitionViewCacheForTest(); + Assertions.assertNotNull(cache); + int[] loads = {0}; + Supplier> loader = () -> { + loads[0]++; + return Collections.emptyList(); + }; + PartitionViewCacheKey db1t1 = new PartitionViewCacheKey("db1", "t1", -1L, -1L); + PartitionViewCacheKey db1t2 = new PartitionViewCacheKey("db1", "t2", -1L, -1L); + PartitionViewCacheKey db2t1 = new PartitionViewCacheKey("db2", "t1", -1L, -1L); + + // REFRESH TABLE db1.t1 -> only db1.t1 re-loads. Uses the public no-client hook (mirrors a never-scanned + // catalog's REFRESH — hmsClient is null here). + cache.get(db1t1, loader); + cache.get(db1t1, loader); + Assertions.assertEquals(1, loads[0], "second get is a hit"); + connector.invalidateTable("db1", "t1"); + cache.get(db1t1, loader); + Assertions.assertEquals(2, loads[0], "REFRESH TABLE forces a reload of db1.t1"); + + // REFRESH DATABASE db1 -> db1.t2 re-loads; db2.t1 unaffected. + cache.get(db1t2, loader); // loads=3 (miss) + cache.get(db2t1, loader); // loads=4 (miss) + cache.get(db1t2, loader); // hit + cache.get(db2t1, loader); // hit + Assertions.assertEquals(4, loads[0]); + connector.invalidateDb("db1"); + cache.get(db2t1, loader); // db2 untouched -> hit + Assertions.assertEquals(4, loads[0], "REFRESH DATABASE db1 must NOT drop db2's entries"); + cache.get(db1t2, loader); // db1.t2 dropped -> miss + Assertions.assertEquals(5, loads[0], "REFRESH DATABASE db1 drops db1's entries"); + + // REFRESH CATALOG -> everything re-loads. + connector.invalidateAll(); + cache.get(db2t1, loader); + Assertions.assertEquals(6, loads[0], "REFRESH CATALOG drops everything"); + } + + @Test + public void invalidatePartitionDropsTheWholeTablesCachedView() { + // Cache A's key carries no partition-name axis (only db/table/-1/-1), so a partition add/drop/alter + // (HiveConnector.invalidatePartition) cannot be scoped finer than the table's single cached entry -- + // it must invalidate that whole entry. MUTATION: invalidatePartition not routed to the view cache -> the + // stale entry survives -> the reload assert below red. + HiveConnector connector = new HiveConnector(props(), new FakeConnectorContext()); + ConnectorPartitionViewCache> cache = connector.partitionViewCacheForTest(); + int[] loads = {0}; + Supplier> loader = () -> { + loads[0]++; + return Collections.emptyList(); + }; + PartitionViewCacheKey db1t1 = new PartitionViewCacheKey("db1", "t1", -1L, -1L); + PartitionViewCacheKey db1t2 = new PartitionViewCacheKey("db1", "t2", -1L, -1L); + + cache.get(db1t1, loader); + cache.get(db1t2, loader); + Assertions.assertEquals(2, loads[0]); + + connector.invalidatePartition("db1", "t1", Collections.singletonList("dt=2024-01-01")); + + cache.get(db1t1, loader); + Assertions.assertEquals(3, loads[0], "invalidatePartition must drop the whole cached view of that table"); + cache.get(db1t2, loader); + Assertions.assertEquals(3, loads[0], "invalidatePartition must NOT drop another table's cached view"); + } + + @Test + public void publicHooksAreNoThrowWithoutBuildingAClient() { + // A fresh connector never built its metastore client (hmsClient == null). The public REFRESH hooks must + // not force-build one just to route to the view cache, and must not throw. + HiveConnector connector = new HiveConnector(props(), new FakeConnectorContext()); + Assertions.assertDoesNotThrow(() -> connector.invalidateTable("db", "t")); + Assertions.assertDoesNotThrow(() -> connector.invalidateDb("db")); + Assertions.assertDoesNotThrow(() -> connector.invalidateAll()); + Assertions.assertDoesNotThrow(() -> { + connector.invalidatePartition("db", "t", Collections.singletonList("dt=2024-01-01")); + }); + } +} From 2efa9f070d8f270d37eb3ab8e4e9e7ee8a6e8209 Mon Sep 17 00:00:00 2001 From: morningman Date: Tue, 21 Jul 2026 04:46:48 +0800 Subject: [PATCH 16/19] [fix](catalog) make Cache B version the frozen partition name-set for all engines + drop Cache B on db/catalog invalidation FIX 1 (correctness): PluginDrivenMvccExternalTable.getPartitionMetaVersion now unconditionally returns the frozen partition NAME-SET (new HashSet<>(getOriginPartitions(scan).keySet())); the O(1) "@" branch is removed. That token is only sound where a connector's listPartitions content is a pure function of the snapshot id, which is NOT true for iceberg NON-RANGE tables (identity/bucket/truncate/multi-field): their nameToPartitionItem (Cache A, keyed (-1,-1)) and the snapshot-id token (IcebergLatestSnapshotCache) expire independently, so a snapshot-id token could serve SortedPartitionRanges staler than the pin's own partitions -> within-query disagreement -> silent under-inclusive pruning. Deriving the version from the frozen name set makes it uniform across all engines (the scheme snapshot-less hive already used). FIX 2 (completeness): ExternalMetaCacheMgr.invalidateDb / invalidateCatalog / removeCatalog now also drop the NereidsSortedPartitionsCacheManager (Cache B), mirroring invalidateTable. The manager has no db/catalog-scoped eviction key, so a null-safe helper calls invalidateAll() (coarse but correct; lazy rebuild). Tests: retarget the token-string assertion to the name-set; add a real-snapshot (snapshotId=42 held constant) rebuild/hit test proving the iceberg non-RANGE coherence; add db/catalog/removeCatalog Cache-B-drop tests. fe-core only. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_014ZngdXGBAXNDRe7jA42mAS --- .../datasource/ExternalMetaCacheMgr.java | 27 ++++ .../mvcc/PluginDrivenMvccExternalTable.java | 33 +++-- ...tedPartitionsCacheManagerExternalTest.java | 140 ++++++++++++++++-- 3 files changed, 175 insertions(+), 25 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalMetaCacheMgr.java b/fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalMetaCacheMgr.java index ee4017a49eb698..115fb6becb0d0a 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalMetaCacheMgr.java +++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalMetaCacheMgr.java @@ -20,6 +20,7 @@ import org.apache.doris.catalog.Env; import org.apache.doris.common.Config; import org.apache.doris.common.ThreadPoolManager; +import org.apache.doris.common.cache.NereidsSortedPartitionsCacheManager; import org.apache.doris.datasource.doris.DorisExternalMetaCache; import org.apache.doris.datasource.metacache.AbstractExternalMetaCache; import org.apache.doris.datasource.metacache.ExternalMetaCache; @@ -184,6 +185,10 @@ public void invalidateCatalog(long catalogId) { routeCatalogEngines(catalogId, cache -> safeInvalidate( cache, catalogId, "invalidateCatalog", () -> cache.invalidateCatalogEntries(catalogId))); + // Cache B (Nereids sorted-partition-ranges) has no db/catalog-scoped eviction, so a catalog-level + // REFRESH must drop ALL of its entries -- else binary-search pruning could serve ranges older than + // the refreshed metadata. Coarse but correct (a rebuild is cheap and lazy). Mirrors invalidateTable. + invalidateSortedPartitionsCache(); } public void invalidateCatalogByEngine(long catalogId, String engine) { @@ -196,6 +201,9 @@ public void removeCatalog(long catalogId) { routeCatalogEngines(catalogId, cache -> safeInvalidate( cache, catalogId, "removeCatalog", () -> cache.invalidateCatalog(catalogId))); + // Drop ALL Cache B entries: the catalog is going away and Cache B has no catalog-scoped eviction. + // (invalidateAll needs no catalog name, so this is safe even after the catalog was already removed.) + invalidateSortedPartitionsCache(); } public void removeCatalogByEngine(long catalogId, String engine) { @@ -207,6 +215,9 @@ public void removeCatalogByEngine(long catalogId, String engine) { public void invalidateDb(long catalogId, String dbName) { routeCatalogEngines(catalogId, cache -> safeInvalidate( cache, catalogId, "invalidateDb", () -> cache.invalidateDb(catalogId, dbName))); + // Cache B has no db-scoped eviction key, so a db-level REFRESH drops ALL entries (coarse but + // correct -- a rebuild is cheap and lazy). Mirrors invalidateTable's Cache B wiring. + invalidateSortedPartitionsCache(); } public void invalidateTable(long catalogId, String dbName, String tableName) { @@ -221,6 +232,22 @@ public void invalidateTable(long catalogId, String dbName, String tableName) { } } + /** + * Drops ALL entries of the Nereids sorted-partition-ranges cache (Cache B). Used by the db/catalog-level + * invalidations, which have no finer-grained (db/catalog-scoped) eviction key on that cache. Null-safe: + * during early startup / checkpoint replay {@code Env.getCurrentEnv()} or its cache manager may be unset. + */ + private void invalidateSortedPartitionsCache() { + Env env = Env.getCurrentEnv(); + if (env == null) { + return; + } + NereidsSortedPartitionsCacheManager mgr = env.getSortedPartitionsCacheManager(); + if (mgr != null) { + mgr.invalidateAll(); + } + } + public void invalidateTableByEngine(long catalogId, String engine, String dbName, String tableName) { routeSpecifiedEngine(engine, cache -> safeInvalidate( cache, catalogId, "invalidateTableByEngine", diff --git a/fe/fe-core/src/main/java/org/apache/doris/datasource/mvcc/PluginDrivenMvccExternalTable.java b/fe/fe-core/src/main/java/org/apache/doris/datasource/mvcc/PluginDrivenMvccExternalTable.java index 2a48a6a6072088..41bd5ad2b618de 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/datasource/mvcc/PluginDrivenMvccExternalTable.java +++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/mvcc/PluginDrivenMvccExternalTable.java @@ -634,17 +634,23 @@ public Map getOriginPartitions(CatalogRelation scan) { @Override public Object getPartitionMetaVersion(CatalogRelation scan) { - ConnectorMvccSnapshot cs = getOrMaterialize(pinnedSnapshot(scan)).getConnectorSnapshot(); - if (cs.getSnapshotId() != -1L) { - // Opaque version token: iceberg/paimon expose an immutable (snapshotId, schemaId) pair. - return cs.getSnapshotId() + "@" + cs.getSchemaId(); - } - // Snapshot-less connector (e.g. hive): no real MVCC snapshot id to version against. Derive the - // token from the frozen partition NAME SET instead -- getOriginPartitions(scan) reads the SAME - // pin, so version == exact content the ranges are built from: the cache rebuilds precisely when - // the partition set changes, never on a stale set. A compact copy of just the name strings (NOT - // the live keySet view), so the cross-query cache entry does not retain the whole pin's - // partition-item map. + // The version token MUST equal the EXACT partition content the ranges are built from + // (getOriginPartitions reads the SAME pin), so Cache B can never disagree with the pin's own + // nameToPartitionItem regardless of how/when a connector's listPartitions cache (Cache A) + // refreshes: the cache rebuilds precisely when the partition set changes, never on a stale set. + // A compact copy of just the name strings (NOT the live keySet view), so the cross-query cache + // entry does not retain the whole pin's partition-item map. + // + // The former "@" O(1) token was REMOVED: it is only sound where a + // connector's listPartitions content is a pure function of the snapshot id, which is NOT true for + // iceberg NON-RANGE tables (identity / bucket / truncate / multi-field partitioning). There the + // pin's nameToPartitionItem is served by listPartitions (Cache A, keyed (-1,-1), enumerated at + // currentSnapshot with its own TTL, ignoring the pin) while the snapshot-id token comes from a + // DIFFERENT cache (IcebergLatestSnapshotCache) with its own TTL; the two expire independently, so + // a snapshot-id token could serve SortedPartitionRanges STALER than the pin's own partitions -> + // within-query disagreement -> silent under-inclusive pruning. Deriving the version from the + // frozen name set makes it uniform across ALL engines -- the same rigorous scheme snapshot-less + // hive already relied on -- at the cost of an O(partitions) set copy per lookup. return new HashSet<>(getOriginPartitions(scan).keySet()); } @@ -657,8 +663,9 @@ public long getPartitionMetaLoadTimeMillis(CatalogRelation scan) { // getSortedPartitionRanges is now a pure pass-through to ExternalTable's cache-manager delegation // (5c17b748880's snapshotId==-1 short-circuit was removed: getPartitionMetaVersion above derives a - // content-comparable version for snapshot-less connectors, so Cache B is now correct for them too) -- - // no override needed here. + // content-comparable version from the frozen partition name set for ALL engines, so Cache B is + // correct uniformly -- snapshot-less hive and real-snapshot iceberg/paimon alike) -- no override + // needed here. private Optional pinnedSnapshot(CatalogRelation scan) { if (scan instanceof LogicalFileScan) { diff --git a/fe/fe-core/src/test/java/org/apache/doris/common/cache/NereidsSortedPartitionsCacheManagerExternalTest.java b/fe/fe-core/src/test/java/org/apache/doris/common/cache/NereidsSortedPartitionsCacheManagerExternalTest.java index 9cfb03ae14d2d0..7e810f54a00c9d 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/common/cache/NereidsSortedPartitionsCacheManagerExternalTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/common/cache/NereidsSortedPartitionsCacheManagerExternalTest.java @@ -86,6 +86,7 @@ public class NereidsSortedPartitionsCacheManagerExternalTest { private static final String CTL = "ctl"; private static final String DB = "db"; private static final String TBL = "t"; + private static final long CATALOG_ID = 7L; @AfterEach public void tearDown() { @@ -289,24 +290,28 @@ public void testPluginDrivenMvccExternalTableRealOriginPartitionsAndVersion() th Assertions.assertEquals(pinnedPartsT1, origin, "getOriginPartitions must dispatch through the LogicalFileScan branch of pinnedSnapshot " + "and return exactly the t1 pin's partition map (not t2's, not latest's)"); - Assertions.assertEquals("42@7", version, - "getPartitionMetaVersion must return @ from the t1 pin's connector " - + "snapshot, dispatched via the same LogicalFileScan branch"); + Assertions.assertEquals(new java.util.HashSet<>(pinnedPartsT1.keySet()), version, + "getPartitionMetaVersion must return the FROZEN partition NAME-SET of the t1 pin " + + "(a real snapshotId=42 no longer yields the @ token) -- " + + "resolved via the same LogicalFileScan branch, so it is t1's name set, not t2's " + + "(empty) or latest's"); } finally { ConnectContext.remove(); } } - // ──────────────────── Hive sentinel snapshot (-1) now uses Cache B via a NAME-SET version ───── + // ──────────────────── Cache B version is the frozen partition NAME-SET for ALL engines ───── // - // getPartitionMetaVersion's token is "@" for a REAL snapshot id, but hive's - // beginQuerySnapshot always pins the sentinel snapshotId == -1 (hive has no MVCC snapshot), so that - // token would be a CONSTANT across queries. Instead, for a snapshot-less pin the version is derived - // from the frozen partition NAME SET (getOriginPartitions(scan).keySet(), copied) -- the SAME map the - // ranges are built from, so version == exact content and the cache rebuilds precisely when the - // partition set changes. PluginDrivenMvccExternalTable no longer overrides getSortedPartitionRanges - // (the -1 short-circuit added in 5c17b748880 was removed): a snapshot-less pin now goes through the - // inherited ExternalTable#getSortedPartitionRanges -> the cache manager, exactly like iceberg/paimon. + // getPartitionMetaVersion always returns the frozen partition NAME SET + // (getOriginPartitions(scan).keySet(), copied) -- the SAME map the ranges are built from, so + // version == exact content and Cache B rebuilds precisely when the partition set changes, never on a + // stale set. This is uniform across hive (snapshotId == -1 sentinel), paimon and iceberg: the old + // "@" O(1) token was removed because it is unsafe wherever a connector's + // listPartitions content is not a pure function of the snapshot id (iceberg non-RANGE: identity / + // bucket / truncate / multi-field partitioning), whose nameToPartitionItem (Cache A) and snapshot-id + // token (a DIFFERENT cache) expire independently. PluginDrivenMvccExternalTable no longer overrides + // getSortedPartitionRanges (the -1 short-circuit added in 5c17b748880 was removed): every pin goes + // through the inherited ExternalTable#getSortedPartitionRanges -> the cache manager. /** * Builds a live Env whose {@code getSortedPartitionsCacheManager()} returns {@code rangesCacheMgr}, @@ -356,6 +361,18 @@ private static PluginDrivenMvccSnapshot sentinelPin(Map p return new PluginDrivenMvccSnapshot(sentinelSnapshot, parts, Maps.newHashMap()); } + /** + * A pin carrying a REAL (non-sentinel) connector snapshot id -- e.g. iceberg. Used to prove Cache B's + * version token is the frozen partition NAME-SET even for {@code snapshotId != -1}: the snapshot id is + * held CONSTANT across queries while the partition set changes, exactly the iceberg non-RANGE hazard + * where Cache A (listPartitions) and the snapshot-id token expire independently. + */ + private static PluginDrivenMvccSnapshot realSnapshotPin(Map parts) { + ConnectorMvccSnapshot realSnapshot = ConnectorMvccSnapshot.builder() + .snapshotId(42L).schemaId(7L).build(); + return new PluginDrivenMvccSnapshot(realSnapshot, parts, Maps.newHashMap()); + } + @Test public void testGetSortedPartitionRangesPresentForSnapshotLessNonEmptyPartitions() throws Exception { Map pinnedParts = Maps.newHashMap(); @@ -410,6 +427,49 @@ public void testGetSortedPartitionRangesRebuildsWhenSnapshotLessNameSetChanges() } } + @Test + public void testGetSortedPartitionRangesRebuildsWhenRealSnapshotNameSetChanges() throws Exception { + // Real-snapshot (snapshotId != -1) coherence: the version token is the frozen partition NAME-SET + // for ALL engines, not just hive. This is the iceberg non-RANGE hazard -- the pin's + // nameToPartitionItem (served by listPartitions / Cache A) can advance while the connector snapshot + // id stays CONSTANT, because the two are backed by INDEPENDENTLY-expiring caches. Under the removed + // "@" token both queries would key "42@7" and serve a STALE HIT (silent + // under-inclusive pruning); under the name-set token the grown partition set forces a rebuild. + PluginDrivenMvccExternalTable table = newRealWiringTable(); + NereidsSortedPartitionsCacheManager rangesCacheMgr = new NereidsSortedPartitionsCacheManager(); + try (MockedStatic envStatic = mockEnvWithRangesCacheManager(rangesCacheMgr)) { + Map partsAtQuery1 = Maps.newHashMap(); + partsAtQuery1.put("id=1", listItem(1)); + partsAtQuery1.put("id=2", listItem(2)); + LogicalFileScan scan1 = pinLatestAndBuildScan(table, realSnapshotPin(partsAtQuery1)); + SortedPartitionRanges first = table.getSortedPartitionRanges(scan1).orElse(null); + Assertions.assertNotNull(first, "ranges built and cached at the first (2-partition) name set"); + Assertions.assertEquals(2, first.sortedPartitions.size()); + + // Same query again: IDENTICAL name set at the SAME real snapshot id => cache HIT (same instance). + LogicalFileScan scanHit = pinLatestAndBuildScan(table, realSnapshotPin(partsAtQuery1)); + SortedPartitionRanges hit = table.getSortedPartitionRanges(scanHit).orElse(null); + Assertions.assertSame(first, hit, + "unchanged name set at a real snapshotId=42 => cache HIT returns the SAME instance"); + + // A later query whose Cache A refreshed to include a new partition while the connector snapshot + // id is STILL 42: the name set grew, so Cache B must rebuild even though "42@7" is unchanged. + Map partsAtQuery2 = Maps.newHashMap(); + partsAtQuery2.put("id=1", listItem(1)); + partsAtQuery2.put("id=2", listItem(2)); + partsAtQuery2.put("id=3", listItem(3)); + LogicalFileScan scan2 = pinLatestAndBuildScan(table, realSnapshotPin(partsAtQuery2)); + SortedPartitionRanges rebuilt = table.getSortedPartitionRanges(scan2).orElse(null); + Assertions.assertNotNull(rebuilt, "ranges rebuilt at the second (3-partition) name set"); + Assertions.assertNotSame(first, rebuilt, + "name-set change at a CONSTANT real snapshotId=42 must still trigger a rebuild " + + "(the removed @ token would have served a stale hit)"); + Assertions.assertEquals(3, rebuilt.sortedPartitions.size(), "rebuilt from the new partition set"); + } finally { + ConnectContext.remove(); + } + } + @Test public void testGetSortedPartitionRangesEmptyForSnapshotLessEmptyPartitions() throws Exception { PluginDrivenMvccExternalTable table = newRealWiringTable(); @@ -462,4 +522,60 @@ public void testExternalMetaCacheMgrInvalidateTableDropsRangesCacheEntry() throw "ExternalMetaCacheMgr.invalidateTable must also drop the " + "NereidsSortedPartitionsCacheManager entry (ExternalMetaCacheMgr.java:217-220)"); } + + // ── db/catalog-level invalidation also drops Cache B (§10 completeness) ── + // + // invalidateTable (above) already drops the ranges cache; invalidateDb / invalidateCatalog / + // removeCatalog previously did NOT, so a db- or catalog-level REFRESH left STALE Cache B entries. + // Cache B has no db/catalog-scoped eviction key, so those coarse invalidations drop ALL entries + // (invalidateAll). Each test drives the REAL ExternalMetaCacheMgr method end-to-end. + + /** + * Populates a live {@link NereidsSortedPartitionsCacheManager} with one entry, then runs {@code + * invalidation} against a REAL {@link ExternalMetaCacheMgr} (with a mocked {@code Env} wiring + * {@code getCatalogMgr}/{@code getSortedPartitionsCacheManager}) and asserts Cache B is emptied. + */ + private void assertDropsAllRangesCache(java.util.function.Consumer invalidation) + throws Exception { + newLiveConnectContext(); + NereidsSortedPartitionsCacheManager rangesCacheMgr = new NereidsSortedPartitionsCacheManager(); + FakeExternalTable t = new FakeExternalTable(true); + t.parts.put("id=1", listItem(1)); + Assertions.assertTrue(rangesCacheMgr.get(t.table, (CatalogRelation) null).isPresent(), + "ranges built and cached before invalidation"); + Assertions.assertEquals(1, rangesCacheMgr.getPartitionCaches().estimatedSize(), + "one entry cached before invalidation"); + + Env env = Mockito.mock(Env.class); + CatalogMgr catalogMgr = Mockito.mock(CatalogMgr.class); + CatalogIf catalog = Mockito.mock(CatalogIf.class); + Mockito.when(catalog.getName()).thenReturn(CTL); + Mockito.when(catalogMgr.getCatalog(CATALOG_ID)).thenReturn(catalog); + Mockito.when(env.getCatalogMgr()).thenReturn(catalogMgr); + Mockito.when(env.getSortedPartitionsCacheManager()).thenReturn(rangesCacheMgr); + + ExternalMetaCacheMgr metaCacheMgr = new ExternalMetaCacheMgr(true); + try (MockedStatic envStatic = Mockito.mockStatic(Env.class)) { + envStatic.when(Env::getCurrentEnv).thenReturn(env); + invalidation.accept(metaCacheMgr); + } + + Assertions.assertEquals(0, rangesCacheMgr.getPartitionCaches().estimatedSize(), + "db/catalog-level invalidation must also drop the NereidsSortedPartitionsCacheManager entries"); + } + + @Test + public void testInvalidateDbDropsRangesCache() throws Exception { + assertDropsAllRangesCache(m -> m.invalidateDb(CATALOG_ID, DB)); + } + + @Test + public void testInvalidateCatalogDropsRangesCache() throws Exception { + assertDropsAllRangesCache(m -> m.invalidateCatalog(CATALOG_ID)); + } + + @Test + public void testRemoveCatalogDropsRangesCache() throws Exception { + assertDropsAllRangesCache(m -> m.removeCatalog(CATALOG_ID)); + } } From 6c1506428260efff9f26e581748ae8287bdc8a46 Mon Sep 17 00:00:00 2001 From: morningman Date: Tue, 21 Jul 2026 10:23:07 +0800 Subject: [PATCH 17/19] [test](catalog) stub getStatementScope in PhysicalIcebergMergeSinkTest connector mock The per-statement metadata funnel (777a61671ab) routed the merge sink's connector partition-field branch through PluginDrivenMetadata.get, which dereferences session.getStatementScope() before memoizing getMetadata. That commit did not update PhysicalIcebergMergeSinkTest, whose pluginTable helper mocks ConnectorSession without stubbing getStatementScope(), so the two tests that reach the connector branch (postFlipMergeBuildsDistributionFromConnectorSpec, postFlipAbsentTableHandleDegradesToUnpartitioned) hit a null scope and NPE'd. Stub the mock session to return ConnectorStatementScope.NONE, matching the established convention across the plugin-driven FE tests. NONE runs the loader on every call (byte-identical to a direct getMetadata), so the connector branch proceeds to the mocked handle/provider/spec without changing what is asserted. All 9 tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01SMtYwYyyubZZiC1odLZTG3 --- .../trees/plans/physical/PhysicalIcebergMergeSinkTest.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/plans/physical/PhysicalIcebergMergeSinkTest.java b/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/plans/physical/PhysicalIcebergMergeSinkTest.java index 9aa92915a96e13..68ed38f3df2b07 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/plans/physical/PhysicalIcebergMergeSinkTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/plans/physical/PhysicalIcebergMergeSinkTest.java @@ -23,6 +23,7 @@ import org.apache.doris.connector.api.Connector; import org.apache.doris.connector.api.ConnectorMetadata; import org.apache.doris.connector.api.ConnectorSession; +import org.apache.doris.connector.api.ConnectorStatementScope; import org.apache.doris.connector.api.handle.ConnectorTableHandle; import org.apache.doris.connector.api.write.ConnectorWritePartitionField; import org.apache.doris.connector.api.write.ConnectorWritePartitionSpec; @@ -313,6 +314,9 @@ private static PluginDrivenExternalTable pluginTable(ConnectorWritePartitionSpec ConnectorTableHandle handle = Mockito.mock(ConnectorTableHandle.class); ConnectorMetadata metadata = Mockito.mock(ConnectorMetadata.class); ConnectorSession session = Mockito.mock(ConnectorSession.class); + // The sink resolves metadata through PluginDrivenMetadata.get, which memoizes on the session's + // per-statement scope; NONE runs the loader every call (byte-identical to a direct getMetadata). + Mockito.when(session.getStatementScope()).thenReturn(ConnectorStatementScope.NONE); Connector connector = Mockito.mock(Connector.class); Mockito.when(connector.getWritePlanProvider()).thenReturn(provider); // Production selects the write provider per-handle; a plain mock does not run the interface default. From 1176d0f568fce5d5beb9fd6a425bdc0840f8c811 Mon Sep 17 00:00:00 2001 From: morningman Date: Tue, 21 Jul 2026 10:37:26 +0800 Subject: [PATCH 18/19] [fix](ci) allowlist gitleaks generic-api-key false positive on "token: iceberg/paimon" The Gitleaks PR check failed with "leaks found: 3", all of them the generic-api-key rule firing on plain prose. In code comments and a plan-doc the phrase "version token: iceberg/paimon ..." appears; the rule reads "token: " as key=secret, and the value "iceberg/paimon" has Shannon entropy 3.52, just above gitleaks' 3.5 cutoff, so it is reported as a generic API key. It is a cache version-token discussion, not a credential. Gitleaks scans each commit's diff over the PR range (git . --log-opts="--no-merges BASE..HEAD"), so rewording only the branch tip does not help: the commits that introduced the phrase remain in range and keep triggering. Allowlisting the exact benign match is the correct, history-robust fix and matches the existing false-positive allowlists in this config. Verified with the pinned gitleaks 8.30.0 over the full PR commit range: findings drop from 3 to 0. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01SMtYwYyyubZZiC1odLZTG3 --- .gitleaks.toml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.gitleaks.toml b/.gitleaks.toml index 8fb5f5626121aa..c01003b5a6c548 100644 --- a/.gitleaks.toml +++ b/.gitleaks.toml @@ -6,6 +6,12 @@ useDefault = true [[rules]] id = "generic-api-key" +[[rules.allowlists]] +description = "Ignore prose 'version token: iceberg/paimon' in code comments/plan-doc — a cache version-token discussion, not a credential (generic-api-key false positive: 'iceberg/paimon' entropy 3.52 barely clears the 3.5 threshold)" +condition = "AND" +regexTarget = "match" +regexes = ['''token:\s*iceberg/paimon'''] + [[rules.allowlists]] description = "Ignore the fake OIDC token fixture in AuthenticatorManagerTest" condition = "AND" From b20ffb3bd5ce94359730df8b82c386b59a1dea82 Mon Sep 17 00:00:00 2001 From: morningman Date: Tue, 21 Jul 2026 11:53:02 +0800 Subject: [PATCH 19/19] [fix](catalog) remove superseded file-scoped parquet position deletes on iceberg v3 DELETE readExistingFileScopedDeletes gated candidates with the bounds-aware ContentFileUtil.isFileScoped() but then matched them with the raw DeleteFile.referencedDataFile() (manifest field referenced_data_file, id 143). That field is set only by the v3 puffin DV writer (BaseDVFileWriter); a Spark-written file-scoped parquet/orc position delete leaves it null and carries its referenced data file in the file_path column bounds instead (Spark's default write.delete.granularity is FILE). Such a delete therefore passed isFileScoped() yet was dropped by the null check, so a v3 DELETE on a v2->v3 upgraded table wrote the new DV (unioning the old positions) but never removeDeletes'd the superseded parquet delete -- leaving a stale parquet delete file live (RowDelta removedDeleteFiles=null). Switch line 1053 to the bounds-aware ContentFileUtil.referencedDataFileLocation(), consistent with the isFileScoped() gate; DV / partition-scoped / equality-delete handling is unchanged. Also align the v3 row-lineage INSERT-rejection assertion in test_iceberg_v3_row_lineage_complex_query with the connector-agnostic SPI wording ("invisible column", from #64688), replacing the retired iceberg-specific "row lineage column" message. Fixes the two external_table_p0 iceberg regressions in Doris_External_Regression #1001567: test_iceberg_v3_row_lineage_continuous_dml (puffin-vs-parquet delete layout) and test_iceberg_v3_row_lineage_complex_query (stale exception message). Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01SMtYwYyyubZZiC1odLZTG3 --- .../connector/iceberg/IcebergConnectorTransaction.java | 8 +++++++- .../test_iceberg_v3_row_lineage_complex_query.groovy | 4 ++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergConnectorTransaction.java b/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergConnectorTransaction.java index b850f84c63fdc4..14502c4ff077e3 100644 --- a/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergConnectorTransaction.java +++ b/fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergConnectorTransaction.java @@ -1050,7 +1050,13 @@ private List readExistingFileScopedDeletes( || !ContentFileUtil.isFileScoped(deleteFile)) { continue; } - String referenced = deleteFile.referencedDataFile(); + // Use the bounds-aware ContentFileUtil accessor, consistent with the isFileScoped() gate + // above: a Spark-written file-scoped parquet/orc position delete carries its referenced + // data file in the file_path column bounds, NOT in the manifest referenced_data_file field + // (id 143) — only a v3 puffin DV sets that field. The raw DeleteFile.referencedDataFile() + // therefore returns null for such deletes, so they would pass isFileScoped() yet be dropped + // here, leaving the superseded parquet delete live after a v3 DELETE. + String referenced = ContentFileUtil.referencedDataFileLocation(deleteFile); if (referenced == null || !touchedDataFilePaths.contains(referenced)) { continue; } diff --git a/regression-test/suites/external_table_p0/iceberg/test_iceberg_v3_row_lineage_complex_query.groovy b/regression-test/suites/external_table_p0/iceberg/test_iceberg_v3_row_lineage_complex_query.groovy index 95de1c4ea1a464..62cbf86497fad0 100644 --- a/regression-test/suites/external_table_p0/iceberg/test_iceberg_v3_row_lineage_complex_query.groovy +++ b/regression-test/suites/external_table_p0/iceberg/test_iceberg_v3_row_lineage_complex_query.groovy @@ -195,7 +195,7 @@ suite("test_iceberg_v3_row_lineage_complex_query", "p0,external,iceberg,external select rid, id, tag, 0, date '2024-01-01' from ${dimTable} """ - exception "Cannot specify row lineage column '_row_id' in INSERT statement" + exception "Cannot specify invisible column '_row_id' in INSERT statement" } test { @@ -204,7 +204,7 @@ suite("test_iceberg_v3_row_lineage_complex_query", "p0,external,iceberg,external select seq, id, tag, 0, date '2024-01-01' from ${dimTable} """ - exception "Cannot specify row lineage column '_last_updated_sequence_number' in INSERT statement" + exception "Cannot specify invisible column '_last_updated_sequence_number' in INSERT statement" } } finally { sql """drop table if exists ${dimTable}"""