[perf](catalog) fe-connector-iceberg hot-path caching + fe-core per-statement metadata funnel, with session=user cache isolation#65785
Merged
morningman merged 47 commits intoJul 20, 2026
Conversation
morningman
requested review from
924060929,
englefly,
morrySnow and
starocean999
as code owners
July 19, 2026 01:18
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
morningman
force-pushed
the
branch-catalog-spi
branch
from
July 19, 2026 02:50
b2dff68 to
7b3821f
Compare
morningman
requested review from
CalvinKirs,
Gabriel39,
dataroaring,
gavinchou,
liaoxin01,
luwei16,
mymeiyi and
seawinde
as code owners
July 19, 2026 02:50
morningman
force-pushed
the
branch-catalog-spi
branch
2 times, most recently
from
July 20, 2026 05:52
3fb9cae to
eb0921d
Compare
morningman
force-pushed
the
catalog-spi-review-16
branch
from
July 20, 2026 07:35
699bd5d to
4563f2d
Compare
morningman
force-pushed
the
branch-catalog-spi
branch
from
July 20, 2026 08:52
eb0921d to
442f23d
Compare
…nector-iceberg perf audit report Generalize DORIS-27138 issue 1 (per-split whole-table planFiles() from PR apache#64134's getFileFormat fallback) into a problem-class doc, then audit the new fe-connector-iceberg framework against it: 7-lens multi-agent sweep + 2 adversarial verifiers per finding, 23 confirmed / 1 refuted. Top clusters: no Table-object cache (3-7 remote loadTable per planning pass), resurrected planFiles() file-format fallback (1-2 whole-table scans per query on gated tables), per-query PARTITIONS metadata-table scan for partitioned tables. Report is for review only; no code changes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Set up an independent task workspace (plan-doc/perf-hotpath-iceberg/) to drive the fixes for the 2026-07-17 fe-connector-iceberg hot-path heavy-op audit. It groups the 23 confirmed findings (C1-C23) into 11 tasks at cluster granularity (one cluster = one task), ordered by the audit's §5 fix priority. Contents: - README.md: orientation, per-task workflow (aligned with step-by-step-fix), and the iron-rule constraints (fe-core source-only-shrinks, connector-side memo, no source-specific code in generic nodes). - tasklist.md: 11 PERF-NN tasks with an at-a-glance overview table and full per-finding traceability. - HANDOFF.md: rolling per-session handoff, entry point set to PERF-01. - progress.md: append-only log. - designs/: placeholder for per-task design/summary docs (created per fix). Analysis is not duplicated here; it points to the existing audit report, findings.json, and problem-class doc. No product code changed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Re-verify + adversarial red-team + multi-round design for PERF-01 (the 3~7x remote loadTable amplification per query). No product code touched. Key findings folded into designs/FIX-PERF-01-table-memo-design.md: - loadTable is snapshot-agnostic -> memo key is TableIdentifier only, not (id, snapshotId) as the audit draft assumed. - Legacy IcebergMetadataCache (pre-apache#60937) already cached the Table object cross-query, so "behavior parity" means cross-query caching, not the audit's per-planning-pass framing. - Red-team refuted Part B (convertPredicate invalidation narrowing): in doFinalize, convertPredicate runs before any property build, so clearing the (still-null) caches is a no-op. The real duplication is the dual build path (getSplits + properties), which Part A covers. Part B dropped. - Red-team BLOCKER: the cross-query disable gate must be isUserSessionEnabled() || restVendedCredentialsEnabled() (vended tokens expire ~60min and would 403 mid-scan on a 24h-TTL cache hit). Finalized design (aligned with Trino, user-approved): - Fat handle: transient resolvedTable on IcebergTableHandle -> one Table instance per query, auto-reclaimed with the plan, zero per-query state on the connector (the Doris analog of Trino's per-transaction handle). - Cross-query IcebergTableCache on the long-lived IcebergConnector, consumed in the read helper (not a catalog-seam decorator, so DDL stays isolated), gated off for session=user / vended-credentials. Next: implement PERF-01 (TDD, RecordingIcebergCatalogOps count gate first). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…in and across queries (PERF-01) A plain WHERE-filtered iceberg SELECT re-loaded the same table 3~7 times per planning pass: getColumnHandles, getTableStatistics, the scan provider's resolveTable (getScanNodeProperties / planScan / streamingSplitEstimate) and the analysis-phase getMvccPartitionView / listPartitions each issued their own remote loadTable (a metastore RPC + a metadata.json read). The SPI cutover split the legacy IcebergExternalMetaCache and kept only the latest-snapshot pin (IcebergLatestSnapshotCache), dropping the cached-Table half. Restore it in two connector-side layers (fe-core untouched): - Query-scoped fat handle: IcebergTableHandle carries a transient raw Table memo (not serialized, not part of identity), carried forward by withSnapshot / withRewriteFileScope / withTopnLazyMaterialize. Reads resolve the table once and reuse the same instance. PluginDrivenScanNode.currentHandle is per-query, so the memo is GC'd with the plan (zero per-query accumulation) => exactly one remote loadTable per table per planning pass. - Cross-query IcebergTableCache (new; mirrors IcebergLatestSnapshotCache, value = raw Table): 24h TTL, REFRESH-invalidated, backs the fat handle's first miss and restores legacy cross-query caching. Disabled (null) for iceberg.rest.session=user and REST vended-credentials, where a cached raw Table's FileIO carries a per-user / soon-expiring token (cross-user leak / 403 mid-scan); the fat handle stays on there (token fresh within one query). Reads go through resolveTableForRead (fat handle -> cache -> raw loadTable), which throws NoSuchTableException unwrapped so listPartitions / getMvccPartitionView keep their concurrent-drop degradation. The scan provider re-applies wrapTableForScan (Kerberos FileIO) per call, never freezing it into the memo. DDL / write / procedure / sys-table paths bypass the helper (fresh remote base). Part B (convertPredicate narrowing) is intentionally dropped (red-team proved it a no-op). Metric gate: RecordingIcebergCatalogOps asserts one planning pass (getColumnHandles + planScan on a single handle) issues exactly one remote loadTable (was two). Full iceberg module suite: 932 pass / 0 fail / 1 skip; checkstyle clean. Behavior parity: pure de-duplication -- snapshot semantics, schema visibility and time travel are unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SMtYwYyyubZZiC1odLZTG3
…s; next = PERF-02 Record PERF-01 (484f0e0) as done: add the implementation summary, mark it complete in tasklist, append the session-2 progress entry (impl + verification + the maven build gotchas), and repoint HANDOFF at PERF-02 (partition-view cross-query cache + MTMV refresh pin), which reuses PERF-01's cache/handle-pin pattern. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SMtYwYyyubZZiC1odLZTG3
…cross queries (PERF-02) For a partitioned table, every query pays an analysis-phase PARTITIONS metadata-table scan (IcebergPartitionUtils.loadRawPartitions -> partitionsTable.newScan().useSnapshot(id).planFiles()+rows()), which the iceberg SDK materializes by reading EVERY data+delete manifest of the snapshot (O(#manifests) remote IO, independent of query selectivity). It ran once per query (memoized only within the statement) and 4~6 times per MTMV refresh, with no cross-query cache -- the partition-info half of the legacy IcebergExternalMetaCache that the SPI cutover dropped. PERF-01 already deduped the loadTable in this cluster; this fixes the remaining scan. Add a connector-side IcebergPartitionCache keyed by (TableIdentifier, snapshotId), value = the raw partition list. The three consumers -- buildMvccPartitionView (MVCC/MTMV view), listPartitions (selectedPartitionNum), listPartitionNames (SHOW PARTITIONS) -- all funnel through loadRawPartitions, so they share one scan per (table, snapshot). The snapshot id resolved inside those methods is held stable across queries (and across one refresh's enumeration points) by IcebergLatestSnapshotCache, so the key is stable and the MTMV 4~6x re-enumeration collapses onto one scan too. A snapshot is immutable, so the cached value is a pure function of the key (always correct; a new commit -> new snapshot id -> new key -> a live scan). The value is pure metadata with NO FileIO/credential, so unlike the PERF-01 table cache it needs no credential gate and is built for every catalog (only the meta.cache.iceberg.table.ttl-second knob disables it). The cached list is unmodifiable; the loader's exception propagates verbatim so listPartitions keeps its dropped-partition-source-column degradation, and a failed scan is not cached. REFRESH TABLE/DB/CATALOG invalidate it alongside the other caches. The MTMV refresh-level MvccSnapshot pin the audit also suggested is intentionally NOT done: IcebergLatestSnapshotCache already stabilizes the snapshot across a refresh (ttl>0), so this cache collapses the repeats without a fe-core change (the only residual is the ttl=0 no-cache catalog's pre-existing enumeration skew, which is out of scope for a connector-side perf fix). Metric gate: a real partitioned table + a real cache; repeated buildMvccPartitionView plus listPartitions at one snapshot run the PARTITIONS scan exactly once (cache.loadCountForTest()==1, was per-call). Full iceberg module suite: 943 pass / 0 fail / 1 skip; checkstyle clean. Behavior parity: cached enumeration equals a live one; snapshot/schema visibility unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SMtYwYyyubZZiC1odLZTG3
…er updates; next = PERF-03 Record PERF-02 (518d059) as done: add the partition-view-cache design and summary, mark it complete in tasklist, append the session-2 progress entry, and repoint HANDOFF at PERF-03 (apache#64134 planFiles format-inference fallback). Notes the stale PERF-01 dependency (Part B was dropped) so PERF-03 targets the scan itself. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SMtYwYyyubZZiC1odLZTG3
…ce across queries (PERF-03) Root cause: every SELECT/EXPLAIN of an iceberg table resolves the scan-level file_format_type through IcebergScanPlanProvider.getScanNodeProperties -> IcebergWriterHelper.getFileFormat. When the table carries neither the 'write-format' nickname nor 'write.format.default' (migrated tables, and any table whose writer never set the property -- parquet is an implicit default, not stored), resolution falls back to inferFileFormatFromDataFiles, an unfiltered table.newScan().planFiles() -- a whole-table manifest scan (remote FileIO; iceberg's ParallelIterable eagerly fans manifest readers) just to read the first data file's format. That apache#64134 fallback ran on every query with zero cross-query reuse: hundreds of ms to seconds of remote IO per query on large tables, multiplied by QPS. Fix (connector-side, no fe-core change): add IcebergFormatCache keyed by (TableIdentifier, currentSnapshotId), value = the inferred format-name string, owned by the long-lived IcebergConnector. A snapshot is immutable, so its first data file's format is a pure function of the key; the snapshot id is held stable across queries by IcebergLatestSnapshotCache. Only the inference fallback is cached -- the two cheap property probes stay uncached. No credential gate (like the PERF-02 partition cache): the value is bare metadata with no FileIO/token, so it is shared across users and built for every catalog (incl. session=user / REST vended). REFRESH TABLE/DB/CATALOG invalidate it. Route note: deriving the format from planScan's own file enumeration was rejected -- FileQueryScanNode.createScanRangeLocations computes file_format_type (getFileFormatType, line 325) BEFORE getSplits/planScan (line 422), and the inference is unfiltered while planScan is predicate-filtered, so a query that prunes all files would break the legacy "unfiltered fallback always finds a first file" parity. Memoization keeps strict parity. Parity: same currentSnapshot-derived inference as legacy IcebergUtils.getFileFormat (never the handle's time-travel pin); the orc/parquet mapping (and its throw on an unsupported format) stays OUTSIDE the cache, so an unsupported first file is inferred once yet re-throws every call as before. A failed inference is NOT cached (the loader propagates unchecked; the CloseableIterable.close IOException is wrapped UncheckedIOException), so a transient remote-IO failure retries on the next query. The scan-level value still only selects BE's V1/V2 scanner; per-file format still travels per range. Write-path getFileFormat callers are unchanged (left for PERF-07). Measurement gate: IcebergWriterHelperTest asserts the whole-table inference runs exactly once across repeated queries at one snapshot (loadCountForTest == 1) and matches a live resolution; property-path tables never populate the cache. IcebergFormatCacheTest covers TTL stability, the ttl<=0 disable, invalidation, and not-caching-on-failure. IcebergConnectorCacheTest asserts the no-gate build across plain/vended/session catalogs and REFRESH invalidation. Full iceberg module UT: 957 run / 0 fail / 0 error / 1 skip; checkstyle green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SMtYwYyyubZZiC1odLZTG3
…er updates; next = PERF-04 Records the file-format inference cache work (PERF-03, commit 0b96f2e): design doc (with the red-team fold-ins R1/R2/R3), summary, tasklist check-off, progress log, and a fresh HANDOFF pointing at PERF-04 (the two IcebergManifestCache bypasses, C17/C18) with the OOM-risk open question flagged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SMtYwYyyubZZiC1odLZTG3
… streaming + COUNT(*) planning, lazily (PERF-04) Root cause: the opt-in IcebergManifestCache (meta.cache.iceberg.manifest.enable, default off) was consulted only on the synchronous planning path (planFileScanTaskWithManifestCache). Two paths bypassed it: (C17) the streaming split path for tables >= num_files_in_batch_mode -- the OOM-protection lazy path, which is exactly the large tables the cache targets -- read scan.planFiles() (SDK, uncached); (C18) COUNT(*) pushdown read scan.planFiles() just to grab one placeholder file (the count itself comes from the snapshot summary), while iceberg's ParallelIterable eagerly submitted every manifest reader. On a cache-enabled catalog the cache delivered 0% hit rate on its target tables. Legacy contrast (verified against 6fef25709d3^ fe-core IcebergScanNode): batch mode DID honor the cache -- but the cache-on path MATERIALIZED the full task list (no OOM protection); OOM safety existed only with the cache off. The SPI cutover traded the cache for unconditional streaming (OOM-safe, uncached). Rather than restore legacy (reintroducing the OOM risk) or leave it, this makes the cache-backed planning itself LAZY so both paths get cache hits AND stay bounded. Fix (connector-side, no fe-core change): extract cacheBackedFileScanTasks(), a lazy CloseableIterable<FileScanTask> shared by all three consumers. Phase 1 (eager) reads the delete manifests through the cache and builds the DeleteFileIndex -- required before any data task and bounded like the SDK planFiles(), which also reads all delete manifests up front. Phase 2 (lazy) is a flat-map iterator over the data manifests, reading each through the cache on demand and yielding pruned whole-file BaseFileScanTasks WITHOUT materializing the list (peak heap = delete index + one manifest's files + the split queue). - planFileScanTaskWithManifestCache now materializes that iterable (byte-identical to before; the sync/small-table path keeps the per-table heuristic split size). - streamSplits, when the cache is enabled, feeds the lazy iterable to the existing IcebergStreamingSplitSource at a fixed slice size (unchanged for big tables) -- now cache-hitting and still OOM-safe. - planCountPushdown, when enabled, iterates the lazy iterable and takes the FIRST file (lazy early stop, no ParallelIterable fan-out). The batch/stream DECISION (streamingSplitEstimate) is unchanged. A cache-read failure records it and falls back to scan.planFiles() (catch Exception, since the cache rethrows failures as RuntimeException). statsQueryId is nullable: streaming passes null (no-stats overload) because its Phase 2 runs on the engine pump thread while Phase 1 ran on the caller -- tallying the per-query ScanStats from two threads would race; sync/count are single-threaded and keep stats. Parity: the lazy path reuses the exact per-file prune/task-construction of the old materialized path (which already claims legacy planFiles parity), so no new planning is invented. The COUNT placeholder file may differ from the SDK path's first file (ParallelIterable order is non-deterministic) but the count (snapshot summary) is identical and BE ignores the file. Big-table streaming now populates the cache (its purpose; capacity-bounded, no OOM). Tests: streaming cache-parity vs SDK + cache consumed; partition-prune parity; flat-map across multiple data manifests; COUNT cache count-parity + lazy early stop (reads < all manifests); empty null-snapshot COUNT guard. Sync cache-path tests stay green. Full iceberg module UT: 962 run / 0 fail / 0 error / 1 skip; checkstyle 0 violations. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SMtYwYyyubZZiC1odLZTG3
…er updates; next = PERF-05 Records the lazy manifest-cache reconnect (PERF-04, commit 2e5f393): design doc (with the resolved legacy-parity conflict + red-team fold-ins), summary, tasklist check-off, progress log, and a fresh HANDOFF pointing at PERF-05 (the information_schema.tables per-table loadTable-for-comment, C9) with the fe-core loop constraint and the first-query N-load floor flagged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SMtYwYyyubZZiC1odLZTG3
…anifest reads across snapshots (PERF-08 / C21) Root cause: IcebergExpireSnapshotsAction.buildDeleteFileContentMap built the delete-file -> content classification map by looping over every snapshot and reading all of that snapshot's delete manifests, with no visited-path dedup. Iceberg manifests are immutable and adjacent snapshots carry the same delete manifests forward unchanged, so a single manifest is re-read once per referencing snapshot -- S snapshots x M manifests of serial remote reads, largely redundant. Fix: add a method-scoped visited set and skip a delete manifest whose path was already read (if (!visitedDeleteManifests.add(manifest.path())) continue). Reading each DISTINCT manifest exactly once collapses the reads from O(S*M) to O(distinct M). The result map is byte-identical: an immutable manifest yields the same DeleteFile set on every read, and putIfAbsent first-writer-wins is order- independent because a delete file's content type is immutable. visited MUST stay at method scope (a comment guards this) -- inside the snapshot loop it would reset each iteration and defeat the cross-snapshot dedup. Gate: new @VisibleForTesting lastDeleteManifestReadCount records the distinct manifests read; buildDeleteFileContentMap made package-visible. New test seeds a table whose delete manifest is carried across multiple snapshots (position + equality deletes), asserts reads == distinct manifests (strictly fewer than the per-snapshot total) and that both delete files stay classified as POSITION_DELETES / EQUALITY_DELETES. IcebergExpireSnapshotsActionTest 9/9 green; 0 checkstyle violations. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SMtYwYyyubZZiC1odLZTG3
…er updates; next = P2 remaining Records the maintenance-path fixes (rewrite_data_files union registration C19, expire_snapshots delete-manifest dedup C21): the design + red-team verdict, the summary with both commit hashes, tasklist marked done, a progress entry, and the HANDOFF rewritten to point at the remaining P2 tasks (PERF-09/10/11). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SMtYwYyyubZZiC1odLZTG3
…ariants across a file's byte-slices (PERF-11 / C12+C15a+C13) Root cause: TableScanUtil.splitFiles cuts one data file into k byte-slice FileScanTasks, and buildRange ran per slice, recomputing values that are identical for the whole file every time: partition_data_json (date/time format + Jackson), the ordered identity partitionValues, and the merge-on-read delete carriers. On a partitioned MOR table at 100k splits that is ~0.5-2s of extra planning CPU plus large garbage; the k ranges also each held their own copies (FE heap), and a v3 scan re-put an identical rewritable-delete supply per slice. Fix: thread a 1-entry per-file cache (PerFileScratch) through buildRangeForTask, the shared choke point of the eager planScanInternal loop and the streaming IcebergStreamingSplitSource. computePerFileInvariants runs once per file (keyed by the shared DataFile instance, which all k slices return from task.file()); the file's remaining slices reuse the cached values, and the k ranges share the same immutable partitionValues Map and deleteCarriers List. Only start/length and the size-proportional selfSplitWeight stay per-slice. The v3 rewritableDeleteSupply put is moved to once per file, on the file's FIRST slice keyed to the new file (covering the last file in the stream — a flush-on-evict would drop it and silently resurrect deleted rows). The streaming scratch is a per-source (= per-scan) field, single-threaded, so it stays O(1) memory (streaming OOM safety preserved). count-pushdown passes a null scratch (single range, no caching). Byte-identical: every cached value is a deterministic function of task.file() and scan-invariant inputs; distinct logical files never share a DataFile instance (the SDK and manifest-cache enumeration both copy per entry), so the identity key never returns a stale value; splitFiles emits a file's slices consecutively, so the 1-entry cache hits 100% (a hypothetical non-consecutive arrival would only cost perf, never correctness); IcebergScanRange is immutable and every consumer reads only. Verified by a 3-lens adversarial byte-parity review (0 breaks). Out of scope (separate follow-ups): the wire-side per-range TFileRangeDesc payload duplication needs a thrift delete-file dictionary + BE reader change (protocol evolution, C15b); the fe-core generic-node per-split hoists are a framework-layer change (C14). Gate: @VisibleForTesting perFileInvariantComputeCount + two tests (a split file computes its invariants once, not per slice; two split files compute twice with no cross-file staleness). IcebergScanPlanProviderTest 105/105, IcebergConnectorTransactionTest 66/66, full iceberg module 1064 pass / 1 skip / 0 fail; 0 checkstyle violations. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SMtYwYyyubZZiC1odLZTG3
…ral: design/summary/tracker PERF-11 done partially (C12 per-file invariant memo + C15a instance sharing + C13 plan-side put-once, commit 10b7d29): design + 3-lens byte-parity red-team verdict + summary; C15b (wire-bytes thrift+BE protocol change) and C14 (fe-core generic-node hoists) recorded as deferred with their prerequisites. PERF-10 (WHERE conjunct conversion) re-verified as low-value and not cleanly fixable within the no-framework-touch constraint; marked deferred with the revival condition. Tasklist rows, progress log, and HANDOFF (next = PERF-09 or PERF-11 remainder) updated. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SMtYwYyyubZZiC1odLZTG3
…er handle (PERF-11 / C14) Root cause: PluginDrivenScanNode.resolveScanProvider() = connector.getScanPlanProvider(currentHandle), and the SPI contract builds providers fresh/stateless per call. It is invoked on the per-split / per-range hot path (getFileCompressType per split, getDeleteFiles per range, plus ~10 planning sites), so every split re-allocated a scan provider + did two TCCL classloader swaps to run what is an identity no-op for iceberg. All three enumeration paths hit it (eager coordinator loop, concurrent partition-batch appendBatch threads, single streaming pump). Lowest-magnitude tier (~100-200ns/split, no remote IO), but pure waste: the provider is a function of currentHandle, which is only refined during early pushdown/pin, before split enumeration. Fix: memoize the resolved provider keyed on currentHandle IDENTITY, held in an immutable (handle, provider) holder published via a single volatile write. A hit reuses the instance; an identity miss (first call, or currentHandle swapped by pushdown/pin) re-resolves and republishes. A null provider (no scan capability) is cached and returned correctly. Because it is keyed on the exact currentHandle each caller would pass, it is byte-identical to calling connector.getScanPlanProvider(currentHandle) every time - no reliance on any connector's provider-selection semantics. The final fields + single volatile write make it safe for the concurrent partition-batch appendBatch threads (no torn new-key/old-provider read); sharing one provider instance across the scan is already the established pattern on the heavier planScan path (startSplit/startStreamingSplit capture and share one provider across concurrent async tasks). Connector-agnostic (no source branching); connectors that field-cache their provider (jdbc/maxcompute) see a strict no-op, connectors that new-per-call (iceberg/paimon/hive) lose the per-split allocation - no connector regresses. Measurement / gate: PluginDrivenScanNodeScanProviderSelectionTest. memoizesProviderForStableHandleAndReResolvesOnHandleChange asserts getScanPlanProvider is invoked exactly once for a stable handle across repeated resolves (was N-per-split; observed red "Wanted 1 time ... but was 3" before the fix) and re-resolves once on a handle change. Full PluginDrivenScanNode*Test family: 103 pass / 0 fail / 0 skip; fe-core Checkstyle 0 violations. fe-core-only change; the iceberg connector does not depend on fe-core and is unaffected. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SMtYwYyyubZZiC1odLZTG3
…P2-remainder recon; tracker/handoff updates - Design + summary for the resolveScanProvider() per-handle memo (fix committed in 87ff73b). - Records the session-4 recon of the three remaining P2 candidates (all evidence re-verified live, not stale): the streaming-pump micro-batch (fe-core framework, needs sign-off because micro-batching changes the backend split distribution - not byte-identical), the C15b/C13 wire-byte dedup (protocol evolution: thrift delete-file dictionary + BE reader + mixed-version compat, needs sign-off), and the C14 generic-node slices #1/#2/#4 (deferred: cross-connector Hadoop-Path byte risk / new SPI capability surface). User chose only C14 slice #3 (provider memo). - tasklist/HANDOFF: mark C14 provider-memo done; HANDOFF next-step = the two sign-off-gated blocks only. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SMtYwYyyubZZiC1odLZTG3
…ing spaces - plan-doc/master-todo/: a cross-task register of deferred, decision-gated big items (needs user sign-off or protocol evolution), so they are not forgotten. Seeds two entries with full background/current-call-stack/solution-call-stack/example writeups: the streaming split-dispatch micro-batch (fe-core; not byte-identical -> changes backend split distribution) and the wire-side delete-file dictionary (thrift + BE reader + mixed-version compat). - plan-doc/per-statement-table-owner-port/: tracking space for porting iceberg's PERF-07 per-statement table-load owner pattern to other connectors. Key finding recorded: the neutral fe-core/fe-connector-api scaffolding (ConnectorStatementScope + ConnectorSession.getStatementScope + StatementContext plumbing) is already in place, so porting is connector-side-only work. Candidate map (to be scoped next session): paimon (high) / hive-hms (mid-high) / hudi (mid); maxcompute/es/jdbc/trino likely excluded (no per-statement metastore loadTable fan-out). No product code touched. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SMtYwYyyubZZiC1odLZTG3
…conclusions + Trino-refactor groundwork Recon (recon + adversarial cross-check, double-signed) of every catalog-backed connector for porting iceberg's per-statement table-load-owner pattern. Verdict: no connector is worth porting now. iceberg is unique in being the only connector with a migrated row-level write path (DELETE/MERGE) — the only capability that triggers the multi-arm reload storm the pattern targets. Every other connector is read-only (paimon/hudi/es/trino) or INSERT/append-only (hive/hms, jdbc, maxcompute), and existing cross-query caches already collapse loads to ~1. Also recorded the architecture-unification analysis: the uniform SPI standard already exists (neutral getStatementScope() + ConnectorStatementScope); Trino's per-transaction-metadata model is not directly portable (Doris connectors are shared singletons; the only per-statement span is StatementContext). Altitude tiers L0-L3 laid out; paimon's future row-level-write migration is the correct trigger to extract a shared helper (L1) into fe-connector-api (no iron-rule-A breach). Per user decision: this round only records conclusions to a standalone doc; the Trino-architecture refactor (L2/L3) is deferred to a dedicated next-session discussion (groundwork staged in the doc's section 7). No product code changed. - designs/recon-findings-and-trino-refactor-groundwork.md (new, full findings) - tasklist.md: PORT-01..04 all marked recon-judged-unnecessary (paimon = future candidate) - progress.md / HANDOFF.md: session 1 recon + next-session Trino-refactor discussion Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SMtYwYyyubZZiC1odLZTG3
…oundation) Foundation for the per-statement ConnectorMetadata redesign: an engine-owned funnel that memoizes one ConnectorMetadata per (statement, catalog) and can close it deterministically at statement end. Read-side rerouting, close wiring, HMS sibling, write sharing, and cache isolation land in later commits. - ConnectorStatementScope (SPI): add getOrCreateMetadata(key, factory) typed memo over computeIfAbsent + closeAll() default no-op. NONE runs the factory every call (byte-identical to today). - ConnectorStatementScopeImpl: idempotent best-effort closeAll() closing each AutoCloseable value once (close-once guard, log-and-continue). - PluginDrivenMetadata: the single funnel - get(session, connector) memoizes connector.getMetadata(session) on the session's per-statement scope, keyed by catalog id. - Tests: memo-once-per-statement, NONE-each-call, cross-catalog isolation, close-once idempotency. No production seam routes through the funnel yet; this commit is byte-neutral and independently compilable. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SMtYwYyyubZZiC1odLZTG3
…adata scope (close wiring) Wire the per-statement ConnectorStatementScope to be closed deterministically at statement end, so a later commit that gives ConnectorMetadata.close() real work (releasing FileIO / tables) releases it exactly once, not on GC. P1 close is still a no-op, so this commit is behavior-neutral; it establishes the lifecycle. Two-tier close (a single register-at-scope-creation point was rejected: it would leak the callback for DDL / SHOW / DESCRIBE / EXPLAIN / foreground ANALYZE, which create a scope via Command.run but never reach unregisterQuery, and the callback registry has no TTL): - PRIMARY (coordinated scans/writes/cursor-fetch/internal): getSplits registers scope::closeAll on the existing query-finish callback, same query-id key as the read-transaction release beside it. Object-capture; skip NONE. Fires after off-thread pump quiescence; no registry leak. - FALLBACK (non-coordinated Command statements): StatementContext.close() closes the scope, guarded by isReturnResultFromLocal so arrow-flight defers to its own query-finish close. Runs in ConnectProcessor's per-statement finally (direct) and a new finally in proxyExecute (master-side forwarded DDL/SHOW). - resetConnectorStatementScope() closes-before-null, and each retry attempt resets, so a reused StatementContext (prepared EXECUTE / retry) never memoizes into an already-closed scope whose values would never close. closeAll is idempotent, so the dual primary+fallback triggers on a coordinated local query close the scope exactly once. Tests: reset-closes-before-drop, close()-closes-locally, close()-defers-for-async. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SMtYwYyyubZZiC1odLZTG3
…TEP 1 (C1 foundation + C2 close wiring) - expanded-scope-phasing-and-security-decisions.md: user decisions (phased, not big-bang; the "propagate the instance to background threads" ask was refuted by recon and corrected to read-through; cache isolation confirmed as a REAL security fix since list != load) + verified 3-area findings + STEP 1-4 sequencing + C1/C2 implementation progress + close-wiring residual risks. - trino-parity-metadata-redesign-design.md, P1-implementation-design.md: target architecture + seam-by-seam blueprint (from the prior session). P1-design section 4 corrected: the "register at scope creation" close plan was found to leak (DDL/SHOW/EXPLAIN/ANALYZE via Command.run create a scope but never reach unregisterQuery, and the callback registry has no TTL); replaced with the two-tier close landed in the code commits. - HANDOFF / progress / tasklist: C1 (5b7312f) + C2 (12f3e95) marked done; next session = STEP 1 C3 (read-side rerouting + background read-through + fix the ANALYZE row-count scope-binding hazard). Docs only. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SMtYwYyyubZZiC1odLZTG3
…tement metadata funnel + force read-through for cross-statement loaders Completes the read-side of the engine-held per-statement ConnectorMetadata keystone (C1 funnel + C2 close wiring already landed). Four mechanical parts, all in one commit so every intermediate state stays compilable and green (the sub-parts interleave within two shared files): 1. buildCrossStatementSession() helper on PluginDrivenExternalCatalog: mirrors buildConnectorSession() (same delegated-credential handling) but forces the per-statement scope to ConnectorStatementScope.NONE. Under NONE the funnel memoizes nothing, so a metadata resolved with this session is built fresh and never bound into -- nor closed with -- a live statement's scope. 2. Force read-through for the 9 cross-statement background loaders (database / table name caches, schema cache, column-statistic cache, chunk-size and row-count loaders, name-mapping resolvers, and the BE-driven metadata TVF): each now builds its session via buildCrossStatementSession() and resolves through PluginDrivenMetadata.get(...). This also fixes a latent hazard where fetchRowCount, reached synchronously from AnalysisManager.buildAnalysisJobInfo on the ANALYZE execution thread, would otherwise capture that statement's live scope; forcing NONE makes read-through a contract rather than an accident. 3. Reroute the 49 on-thread read / DDL / command-TVF / MVCC seams from connector.getMetadata(session) to PluginDrivenMetadata.get(session, connector), so a statement resolves one memoized ConnectorMetadata per (catalog) instead of rebuilding it at each resolver. 4. PluginDrivenScanNode: add a volatile cachedMetadata field + metadata() accessor (mirroring the existing resolvedScanProvider holder); the 8 per-method resolvers share it, and the static create() factory routes through the funnel directly. The write-path getMetadata seams (translator x2, BindSink x2, IcebergMergeSink, IcebergRowLevelDmlTransform, InsertExecutor, and the write-only resolveWriteTargetHandle) are deliberately left unchanged for the write-sharing step. Tests: adapt the plugin/scan/mvcc/command/TVF unit tests -- their mocked ConnectorSession now flows through the funnel, so getStatementScope() must return NONE (a Mockito mock returns null for the default method), and test doubles that override buildConnectorSession() also override buildCrossStatementSession(). No assertions or verify() call-counts were weakened. Adds ConnectorSessionImplTest.explicitNoneStatementScopeWinsOverLiveContext pinning that an explicit NONE scope wins over a live ConnectContext capture. Verification: fe-core main compile BUILD SUCCESS; 265 targeted tests green; checkstyle 0 violations. Byte-neutral under NONE (offline/no ConnectContext), matching pre-funnel behavior. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SMtYwYyyubZZiC1odLZTG3
…st + progress/handoff Adds the verified seam-by-seam checklist for the read-side rerouting step (66 connector-metadata factory seams partitioned into 51 reroute / 7-9 force-NONE loaders / 8 write-deferred, cross-checked against current line numbers by an adversarial recon), records the user's two decisions (name-mapping seams -> force-NONE; loaders route through the funnel with a NONE session for a zero-exception anti-drift gate), and updates progress / tasklist / handoff to point the next session at the anti-drift gate that closes the read keystone and then the HMS heterogeneous-gateway sibling fan-out. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SMtYwYyyubZZiC1odLZTG3
…e per-statement funnel Close out the read keystone of the per-statement metadata redesign with a build-time gate so the read/scan/DDL/MVCC seams cannot drift back to bare connector.getMetadata(session) calls. tools/check-fecore-metadata-funnel.sh (+ self-test) greps fe-core main sources and fails the build on any Connector#getMetadata(session) call outside the funnel PluginDrivenMetadata.get(...). Kept silent: the funnel file itself; the no-arg getMetadata() (a different method); comment mentions; and write-path seams carrying a `getMetadata-funnel-exempt` marker (on the call line or the line above it). Wired as a validate-phase exec in fe-core/pom.xml, mirroring tools/check-connector-imports.sh. The 8 write-path seams (INSERT/DELETE/MERGE sink, bind, row-level DML, write target handle) still call getMetadata directly and are marked exempt; the marker is removed and the gate auto-tightens onto them when the write-sharing step reroutes writes through the funnel. Verified: self-test PASS; gate green on the current tree and red on the 8 unmarked write sites; fe-core checkstyle 0 violations; `mvn -pl fe-core validate` fires the gate with BUILD SUCCESS. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SMtYwYyyubZZiC1odLZTG3
…rift gate) + STEP 2 handoff RD-1 (read keystone) marked complete: C1 foundation + C2 close wiring + C3 reroute + scan-node field + background read-through + the anti-drift gate. Progress log records the gate landing (grep universe, rule set, verification); handoff re-points the next session at the HMS heterogeneous-gateway sibling fan-out (STEP 2 / RD-2), including the note that the sibling funnel lives in fe-connector-hive and must go through session.getStatementScope().getOrCreateMetadata (not fe-core's PluginDrivenMetadata) to stay off the connector-imports gate. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SMtYwYyyubZZiC1odLZTG3
…ta per statement
The Hive heterogeneous gateway forwards iceberg/hudi-on-HMS operations to embedded
sibling connectors. It obtained a fresh sibling ConnectorMetadata on every forward,
so one statement rebuilt a table's sibling metadata a dozen-plus times. Route the
four sibling-metadata acquisition sites through the existing per-statement scope so
a statement reuses ONE sibling metadata per owner (mirroring fe-core's own funnel).
- Add SiblingOwner{connector,label}. HiveConnector.resolveSiblingOwnerLabeled yields
the owner label from its matched ownsHandle arm (no force-build, no identity
comparison). resolveSiblingOwner now delegates to it, so the three connector-level
provider seams are unchanged.
- HiveConnectorMetadata.memoizedSiblingMetadata keys the scope as
"metadata:"+catalogId+":"+label. The three connectors share one catalogId, so the
label keeps their metadata entries distinct (a plain catalogId key would collapse
them and misroute). The three helpers (icebergSiblingMetadata / hudiSiblingMetadata
by TYPE, siblingMetadata by HANDLE) and the getTableSchema capability-reflection
stray route through it; beginTransaction(session,handle) is covered for free because
it forwards via siblingMetadata. By-TYPE and by-HANDLE mint the same label for one
owner, so the getTableHandle divert and later per-handle forwards reuse one instance.
- Under NONE scope the factory runs on every call: byte-identical to before. Only
fe-connector-api types are used (no fe-core import); the foreign sibling is never cast.
Tests: five funnel assertions (one build across many forwards incl. the write txn;
NONE rebuilds each call; by-TYPE == by-HANDLE key; cross-catalog isolation; iceberg /
hudi isolated by label). Existing sibling suites now pass a NONE-scope session (the
forward path dereferences the scope); no forwarding / routing / fail-loud assertion is
weakened. Copies TestStatementScope + a ScopeSession test double into the hive test
tree (connector tests cannot import fe-core / iceberg).
The heterogeneous-gateway e2e (INSERT/DELETE/MERGE/ALTER/EXECUTE on an iceberg-on-HMS
table vs a standalone iceberg catalog) is deferred to the later unified delegation-e2e
pass.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SMtYwYyyubZZiC1odLZTG3
…oseout + STEP 3 handoff Record the HMS heterogeneous-gateway sibling-metadata funnel landing (commit 5fd55d0): grounding correction (4 acquisition sites, not ~43), the design confirmed with the user, e2e deferred to the unified delegation pass, 348 tests green + gates green + adversarial review clean. Flip RD-2 to done in tasklist and rewrite HANDOFF to point at STEP 3 (write-side sharing: reroute the 8 write seams through the funnel, uphold the hive start-of-write refresh + read/write identity gates, lift ConnectorTransaction ownership). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SMtYwYyyubZZiC1odLZTG3
… the per-statement funnel The 8 write-arm getMetadata call sites -- the INSERT/DELETE/MERGE sink translators, the insert executor's connector setup, the write-target handle resolver, the two BindSink partition validators, the row-level DML mode check, and the merge-sink partition-field derivation -- were the last direct Connector#getMetadata calls in fe-core (each carried a funnel-exempt marker). Reroute them through PluginDrivenMetadata.get so read and write share the ONE memoized ConnectorMetadata instance per statement, aligning with Trino's CatalogTransaction (which memoizes exactly one ConnectorMetadata per transaction+catalog and shares it across read planning and writes), and delete the 8 markers so the anti-drift gate now covers all of fe-core with no exemptions. Guard the shared reuse with a fail-loud identity check in the funnel: a session=user connector bakes the querying user's delegated credential into the metadata instance at build time, so reusing it under a different identity would execute one user's write with another's credentials. A statement resolves a catalog under exactly one identity (one statement = one user = one credential), so the check never fires in practice; it turns any future violation of that invariant into a hard error instead of a silent cross-user leak. It uses the Doris principal (getUser), never a credential token, so fe-core still parses no credentials; under NONE it stores nothing and the check is vacuous. The hive begin-write refresh (its own getTable under the write auth context for the ACID reject) is downstream of beginTransaction and untouched. Tests: two new funnel identity assertions (same user reuses, different user fails loud); the three write-path suites that now flow through the funnel get the NONE statement-scope stub (the read-side convention). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SMtYwYyyubZZiC1odLZTG3
…to a per-statement co-holder Give the write transaction a first-class statement-level owner, CatalogStatementTransaction, co-held on the per-statement scope next to the one memoized ConnectorMetadata the read and write arms now share -- mirroring Trino's CatalogTransaction (one metadata + one transaction per statement + catalog). The insert executor opens the transaction through the co-holder's begin(), which mints it from that shared write-ops facet (so the write inherits the read arm's client/ops) and registers it with the manager exactly as before. The tx<->session binding, the global registration for the BE block-allocation / commit-data RPCs, and the executor's own commit/rollback at onComplete/onFail are all unchanged. The co-holder's own job is deterministic statement-end teardown. The scope's closeAll now runs in two ordered passes: pass 1 finalizes the transaction (finalizeAtStatementEnd rolls back a transaction the executor never committed -- only a mid-flight abort leaves one active), pass 2 closes the remaining AutoCloseable values (the memoized metadata). A transaction is therefore always finished before the instance it was minted from is closed. The backstop is idempotent and can never undo a committed write: PluginDrivenTransactionManager removes a transaction from its map on both commit and rollback, so the new isActive(id) is the single source of truth -- on every normal path the executor already finished the transaction, so the backstop finds nothing active and does nothing. Under NONE (offline / no live statement) the scope stores nothing, so the co-holder is transient and the executor's own commit/rollback is the only lifecycle -- byte-identical to the pre-co-holder path. Tests: CatalogStatementTransaction (begin registers active; finalize rolls back an orphan, never undoes a committed txn, is idempotent after rollback, no-ops with no txn opened); the scope's two-pass ordering (txn finalized before the metadata is closed). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SMtYwYyyubZZiC1odLZTG3
…+ STEP 4 handoff Record the read/write-shares-one-metadata-instance step as done: the P3 implementation design + as-built (3a reroute + identity gate, 3b co-holder + two-pass closeAll, side-car orthogonality), the RD-3 tasklist row (two commits), the session-7 progress entry, and a rewritten HANDOFF pointing at the cache isolation step (STEP 4, the independent security track: getIdentityShardKey() SPI, iceberg projection-cache key sharding, fe-core schema-cache bypass, the heterogeneous-gateway hole, anti-drift gate, and the required threat-model sign-off). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SMtYwYyyubZZiC1odLZTG3
…che leak fix) Records the signed-off design for the cache-isolation security track, with the three grounding corrections the adversarial verification surfaced: - The only ACTIVE cross-user leaks under iceberg.rest.session=user are the latestSnapshotCache (beginQuerySnapshot reads it without a preceding per-user loadTable) and the fe-core name-keyed schema cache (no shouldBypassSchemaCache). partitionCache/formatCache are safe-but-fragile (guarded only by an upstream per-user loadTable that rests on tableCache being null). - The heterogeneous HMS gateway is a LATENT, not active, hole: the embedded iceberg sibling is forced iceberg.catalog.type=hms and can never be session=user; the "propagate owner label to fe-core" fix-vehicle was refuted. Signed-off route: DISABLE authorization-sensitive projection caches under session=user (no new SPI, no stale-authz window, mirrors tableCache/commentCache), all three projection caches unified; fe-core schema-cache bypass mirroring the db/table-name-cache bypass; a fail-loud gateway guard; an anti-drift gate. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SMtYwYyyubZZiC1odLZTG3
…ion caches under session=user The latestSnapshotCache / partitionCache / formatCache were built unconditionally, so under iceberg.rest.session=user (per-user delegated authorization) they were shared across users with no user dimension in the key. beginQuerySnapshot reads latestSnapshotCache WITHOUT a preceding per-user loadTable, so a "can-list-cannot- load" principal hitting a cached entry received another user's snapshotId/schemaId with the per-user authorization (which lives inside loadTable) bypassed. partition/ format were only safe because their readers happen to run a per-user loadTable first (tableCache being null under session=user) -- a fragile dependency. Gate all three null under isUserSessionEnabled() (mirroring the existing tableCache/ commentCache discipline), so session=user keeps NO live cross-query metadata cache and every projection is re-loaded live per-user -- re-running authorization every call with no stale-authz window. beginQuerySnapshot gains a null-cache fallback (loadLatestSnapshotPin, mirroring resolveTableForRead's tableCache ternary); the partition/format readers already tolerate a null cache. The invalidate* hooks gain null-guards on the three (they were unconditional and would NPE on a nulled cache). Non-session catalogs are byte-unchanged. Tests: latest-snapshot/partition/format each null for session=user (non-null for plain + vended); invalidate hooks no-throw with all three nulled. 22 cache-test + 29 mvcc + 51 metadata green; checkstyle 0. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SMtYwYyyubZZiC1odLZTG3
…ser catalogs The generic table-schema cache is keyed by table NAME only (SchemaCacheKey wraps a NameMapping, no user/identity dimension), so under a SUPPORTS_USER_SESSION connector whose remote loadTable enforces per-user authorization (Iceberg REST session=user) a shared hit served one user's schema to another who could LIST the table but was NOT authorized to LOAD it — the same "list != load" metadata disclosure the db/table-name caches already bypass, but the schema cache was missed. Add shouldBypassSchemaCache(SessionContext) (default false) as the schema-level analog of shouldBypassTableNameCache/DbNameCache, overridden in PluginDrivenExternalCatalog with the identical predicate (supportsUserSession() && ctx.hasDelegatedCredential()). ExternalTable.getSchemaCacheValue() consults it and, on bypass, reads schema live via initSchema (exactly what the cache miss-loader would do) instead of the shared cache — so schema is re-read per user with no stale-authz window. A credential-less session keeps the shared cache; the fail-closed rejection stays connector-side. This covers the MVCC "latest" path too (it funnels through super.getSchemaCacheValue()). Tests: predicate bypasses only for capable+credentialed sessions (empty/null keep the cache; non-user-session never bypasses); read-site reads live via initSchema and never touches the shared cache under bypass. 6 tests green; checkstyle 0; funnel gate exit 0. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SMtYwYyyubZZiC1odLZTG3
…ling is session=user fe-core keys its per-user schema/name cache bypass off the FRONT-DOOR connector's capabilities. A hive gateway front door never declares SUPPORTS_USER_SESSION, so for a delegated iceberg-on-HMS sibling the bypass would NOT trigger — a session=user sibling would silently leak cross-user metadata through the shared caches. Today this cannot happen: the sibling is built with iceberg.catalog.type forced to hms (IcebergSiblingProperties.synthesize), which can never satisfy isUserSessionEnabled() (REST-flavor only). Guard that invariant explicitly: getOrCreateIcebergSibling now throws if the freshly built sibling declares SUPPORTS_USER_SESSION, converting a future fail-open (someone letting the sibling be REST session=user) into a loud failure at sibling-build time instead of a silent leak. Tests: a plain (no-capability) sibling is accepted; a sibling declaring SUPPORTS_USER_SESSION fails loud with a message naming the invariant. FakeSibling gains an optional capability set (no-arg ctor unchanged). 16 tests green; checkstyle 0; connector-import gate exit 0. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SMtYwYyyubZZiC1odLZTG3
…on on IcebergConnector A new cross-query cache field added to IcebergConnector without gating it for iceberg.rest.session=user would silently reintroduce the "list != load" metadata disclosure this track just fixed (a shared, un-partitioned cache bypasses the per-user loadTable authorization). tools/check-authz-cache-sharding.sh turns that from a silent leak into a build failure: every final cache-typed holder field on IcebergConnector must carry, on its declaration line or the line directly above it, one of two markers — authz-cache-session-user-disabled (null under isUserSessionEnabled()) or authz-cache-exempt (justified as read-only-after-a-per-user-load, e.g. the default-off manifest cache). The field-declaration regex matches any "final <Type>Cache " / "final Cache<" holder regardless of visibility or static-ness, so a future authz cache added as a raw Caffeine/Guava Cache<...>/LoadingCache<...> is still caught (not only the shipped "private final Iceberg*Cache" convention). Marker-based like check-fecore-metadata-funnel .sh; a RED/GREEN mktemp self-test locks the behavior across all these forms and proves the markers are load-bearing and independent. Wired as a second exec-maven-plugin validate execution alongside check-connector-imports in fe/fe-connector/pom.xml (inherited=false). The five projection/table/comment caches carry the disabled marker; the manifest cache the exempt marker. The fe-core generic schema cache is a different name-keyed cache protected by shouldBypassSchemaCache, not by this gate. Verified: self-test PASS (incl. raw Cache/LoadingCache/static/visibility forms); real gate exit 0; both validate gates run (BUILD SUCCESS); checkstyle 0; iceberg cache test 22 green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SMtYwYyyubZZiC1odLZTG3
…main line done RD-4 (cache isolation, the "list != load" authorization-leak fix) landed across four commits: iceberg disables its three authz-sensitive projection caches under session=user, fe-core bypasses the shared schema cache, the hive gateway fails loud on a session=user sibling, and an anti-drift gate enforces the invariant. Grounding + adversarial verification corrected the plan (only latestSnapshotCache + fe-core schema cache are active leaks; the heterogeneous gateway is latent, not active), and the user signed off the disable route (no getIdentityShardKey SPI, no stale-authz window). Clean-room review returned 0 blocker/major. Updates tasklist (RD-4 -> done), appends progress session 8, and rewrites HANDOFF: the read/write refactor main line (RD-1..RD-4 / STEP 1-4) is complete; the only remaining debt is the consolidated e2e (heterogeneous gateway DML/DDL + can-list-cannot-load authorization) under external_table_p2/refactor_catalog_param. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SMtYwYyyubZZiC1odLZTG3
…rmalizer signature after rebase The rebase onto upstream branch-catalog-spi replayed our 46 review commits onto a base that already carries upstream's four "port fe-core external-table fixes into the connector" commits. All four survived the 3-way auto-merge and are functionally intact; the only breakage was a test API drift, not a code conflict. Upstream apache#65676's new test convertDeleteRejectsMalformedDeletionVector calls convertDelete(delete, Collections.emptyMap()) -- the pre-refactor signature. Our PERF-06 (635aee0) changed convertDelete's second parameter from a raw Map to a UnaryOperator<String> uriNormalizer (derive the vended storage config once per scan). The upstream test predates that refactor, so it no longer compiles. Align the test to the refactored signature with UnaryOperator.identity(), matching the sibling DV tests (1669/1685/1698/1773). identity() is correct here: the case asserts a malformed DV is rejected, where URI normalization is immaterial. This is the ONLY test migration the rebase required; no connector code changed. fe-connector-iceberg 488 tests green (IcebergScanPlanProviderTest 113/113, incl. upstream's projection-order + 6 DV-validation tests). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SMtYwYyyubZZiC1odLZTG3
morningman
force-pushed
the
catalog-spi-review-16
branch
from
July 20, 2026 09:14
4563f2d to
960bb33
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
The catalog-SPI cutover for Iceberg (P6, #64688) split the legacy
IcebergExternalMetaCacheand kept only the latest-snapshot pin, dropping thecached-
Table/ partition / file-format halves. A 2026-07-17 hot-path audit(7-lens multi-agent sweep + 2 adversarial verifiers per finding, 23 confirmed)
showed the fallout: a plain
WHERE-filtered IcebergSELECTre-loaded the sametable 3–7× per planning pass, partitioned tables re-scanned the
PARTITIONSmetadata table every query, and PR #64134's
getFileFormatfallback ran anunfiltered whole-table
planFiles()on every query of a migrated table.This PR closes that gap on three fronts, all held to behavior parity:
ConnectorMetadatafunnel infe-core — the Doris analog of Trino's
CatalogTransaction(one metadata + onewrite transaction per statement + catalog), so read / scan / DDL / MVCC / write
share one metadata (and one loaded table) per statement.
iceberg.rest.session=user,fixing a "list ≠ load" metadata-disclosure the shared caches would otherwise
introduce.
Two build-time anti-drift gates keep each invariant from silently regressing.
Part of the catalog-SPI migration tracked in #65185.
1. fe-connector-iceberg hot-path caching (PERF-01…11)
All connector-side (fe-core untouched except the one generic-node memo in §1.3);
each landed TDD-first behind a
loadCountForTest-style measurement gate thatasserts the redundant remote read now happens exactly once.
Cross-query caches restored on the long-lived
IcebergConnector:IcebergTableCache+ a query-scoped fat handle. Memoize theresolved
Tablewithin a planning pass (transientresolvedTableonIcebergTableHandle, GC'd with the plan) and across queries (24h TTL,REFRESH-invalidated). Collapses 3–7 remoteloadTableper pass to one.IcebergPartitionCache. Cache thePARTITIONSmetadata-tablescan (keyed by
(table, snapshotId)); shared by the MVCC view,listPartitions, andSHOW PARTITIONS, and across the 4–6 re-enumerations ofone MTMV refresh.
IcebergFormatCache. Memoize the scan-level file-formatinference, eliminating PR [bug](iceberg) fix can't get migrated Iceberg tables format type #64134's per-query unfiltered whole-table
planFiles()fallback on tables that store nowrite.format.default.IcebergManifestCacheback into the streaming andCOUNT(*)planning paths ithad bypassed since the cutover, via a lazy
CloseableIterableso the big tablesit targets get cache hits and stay OOM-bounded.
IcebergCommentCache. Cache the per-tablegetCommentload thatinformation_schema.tables/SHOW TABLE STATUSdrives N-per-query (BI hotpath), for vended-credentials catalogs (where the PERF-01 table cache is null).
Per-scan / per-file / per-split hoists:
scan-scoped SPI seam
ConnectorContext.newStorageUriNormalizer(token)bakes thetoken-derived
StoragePropertiesmap in once (was rebuilt for every data + deletefile,
O(N_files + N_deletes)).provider memo (C14). Compute a file's partition JSON / partition values /
delete carriers once and share them across the file's byte-slice splits; the k
ranges share one immutable map. In fe-core, memoize
PluginDrivenScanNode.resolveScanProvider()per handle identity (byte-identicalto re-resolving; connector-agnostic).
Per-statement table load:
planning, write shaping and
beginWritethrough the new per-statementConnectorStatementScope(see §2) so a statement loads each table once and everyresolver shares that one raw object; the snapshot pin and Kerberos
doAsFileIOwrap are applied per consumer, never frozen into the shared object. Removes the
fat-handle memo and the per-catalog
IcebergRewritableDeleteStashsingleton.Maintenance-path re-scan/dedup (PERF-08):
rewrite_data_files' per-group source registration into oneunion scan (was G+1 full-table
planFiles()against the same pinned snapshot).expire_snapshots' delete-manifest reads across snapshots(immutable manifests were re-read once per referencing snapshot).
2. Per-statement
ConnectorMetadatafunnel (fe-core, Trino parity)Doris connectors are shared singletons whose session is rebuilt ~26× per
statement, so Trino's per-transaction metadata cache cannot hang on the session —
but it can hang on the one per-statement
StatementContext.ConnectorStatementScope(neutral SPI,NONEno-op) memoizes one
ConnectorMetadataper(statement, catalog)viaPluginDrivenMetadata.get(session, connector), closed deterministically atstatement end by a two-tier close (primary: the
getSplitsquery-finishcallback; fallback:
StatementContext.close()), idempotent so a coordinatedlocal query closes exactly once.
9 cross-statement background loaders are forced through a
NONE-scoperead-through (which also fixes a latent hazard where
ANALYZE's synchronousfetchRowCountwould capture the live statement scope).sibling metadata per statement (keyed by owner label), so one statement reuses
one sibling metadata per owner instead of rebuilding it a dozen-plus times.
getMetadataseams through thefunnel so read and write share the one memoized metadata, guarded by a fail-loud
identity check (a
session=usermetadata bakes in the querying user's delegatedcredential; reuse under a different identity is turned into a hard error, never a
silent cross-user write). The funnel now covers all of fe-core with no exemptions.
statement-level
CatalogStatementTransactionco-held next to the shared metadata(Trino's one-metadata-one-transaction-per-statement model), with deterministic
two-pass teardown (finalize the transaction, then close the metadata).
3. Authorization-sensitive cache isolation (RD-4, "list ≠ load")
The restored caches (and the fe-core schema cache) are keyed by table name
only, with no user dimension. Under
iceberg.rest.session=user— where per-userauthorization lives inside the remote
loadTable— a shared hit could serve oneuser's metadata to a principal who can
LISTa table but is not authorized toLOADit. Grounding + adversarial verification narrowed the real leaks tolatestSnapshotCache(read bybeginQuerySnapshotwithout a preceding per-userloadTable) and the fe-core schema cache.latestSnapshotCache/partitionCache/formatCacheare nulled under
isUserSessionEnabled(), sosession=userkeeps no livecross-query metadata cache and re-loads every projection live per user.
shouldBypassSchemaCache(SessionContext)(the schema-levelanalog of the existing db/table-name-cache bypass), overridden in
PluginDrivenExternalCatalog, so a credentialedsession=userread re-readsschema live via
initSchemawith no stale-authz window.SUPPORTS_USER_SESSION(today impossible — the sibling is forcediceberg.catalog.type=hms; this converts a future fail-open into a build-timeloud failure).
4. Anti-drift gates
tools/check-fecore-metadata-funnel.sh— fails the build on anyConnector#getMetadata(session)call in fe-core outside the funnel.tools/check-authz-cache-sharding.sh— every cache-typed field onIcebergConnectormust carry anauthz-cache-session-user-disabledorauthz-cache-exemptmarker.Both are wired as
validate-phase execs (mirroringcheck-connector-imports.sh)and ship with RED/GREEN self-tests.
5. Rebase alignment (upstream #65676)
Rebasing this branch onto
branch-catalog-spireplayed the review commits onto abase already carrying upstream #65676's deletion-vector fixes. All four survived
the 3-way merge intact; the only breakage was a test-signature drift — #65676's
new
convertDeleteRejectsMalformedDeletionVectorcalls the pre-refactorconvertDelete(delete, Map), but PERF-06 changed the second parameter to aUnaryOperator<String> uriNormalizer. Aligned the test to the refactored signaturewith
UnaryOperator.identity()(URI normalization is immaterial to amalformed-DV-rejection assertion). No connector code changed.
Verification
Every step landed TDD-first with a measurement gate, then an adversarial byte-parity
review. The full
fe-connector-icebergmodule UT suite and the touchedfe-core/fe-connector-hivesuites are green; Checkstyle reports 0 violations; both newanti-drift gates pass (self-test + real run). All cache/funnel changes are
byte-identical under the
NONEscope (offline / no live statement), so the pre-existingbehavior is unchanged.
The design and audit trail lives under
plan-doc/(the 2026-07-17 hot-path audit +problem-class doc, the per-task PERF designs, and the per-statement-metadata
redesign). The remaining debt is a consolidated heterogeneous-gateway e2e
(DML/DDL + can-list-cannot-load authorization), deferred to the unified
delegation-e2e pass.
🤖 Generated with Claude Code
https://claude.ai/code/session_01SMtYwYyyubZZiC1odLZTG3