Repository navigation
perf(core): cache ROW sidecar size and reader metadata - #1196
Conversation
JingsongLi
left a comment
JingsongLi
left a comment
There was a problem hiding this comment.
Requirement fit: SUPPORTED. Implementation: FINDINGS at ad1c207. Repeated sparse ROW reads have end-to-end value, but the new cache must respect the existing metadata memory limit.
Local worktree validation: 537 related core tests passed (1 ignored), all 14 data_evolution_row_sidecar_test write/commit/read regressions passed, and formatting/diff checks passed. A temporary single-FileIO probe with file-format.metadata-cache.max-size=64 kb confirmed that Parquet and ROW are each given a separate 64 KiB retention budget (128 KiB combined). The probe was removed afterward. CI runtime/integration checks passed; the failing check job reports baseline TLS dependency advisories, already addressed by main's #1194.
There was a problem hiding this comment.
[P2] Keep ROW and Parquet within the configured metadata-cache budget
This creates an additional cache with min(max_bytes, 32 MiB), while get_or_init still gives Parquet the full max_bytes. The existing file-format.metadata-cache.max-size option promises the maximum parsed file-format metadata cache size. A Data Evolution table can warm both pools through ordinary Parquet scans and sparse ROW sidecar reads in the same FileIO, so the setting no longer bounds their combined retention. A local probe with one FileIO configured to 64 KiB receives 64 KiB for each pool; at the default, their budgets total 82 MiB against the configured 50 MiB. Please share accounting or divide the configured budget between formats; an intentionally additive ROW budget needs its own explicit setting.
XiaoHongbo-Hope
changed the title
Problem
Repeated reads of the same immutable ROW sidecar issue a storage
statfor its length and fetch and parse its footer and block index again. With a one-block fixture, each query makes three range reads afterstat(footer, index, block).Change
Cache the sidecar length and parsed ROW footer/block index in the existing FileIO metadata-cache context. A repeated one-block read reuses the length and parsed metadata, so it only reads the data block. Size entries use a lazy, 4,096-entry LRU; parsed ROW metadata is also entry-bounded. Both caches use the storage-namespaced immutable path; parsed metadata additionally keys by file size. They are disabled when
file-format.metadata-cache.max-size=0, bypass inherited state after fork, and avoid caching when a stable storage namespace is unavailable. FileIO deletions invalidate cached sizes.The configured parsed file-format metadata budget is now shared by Parquet and ROW: ROW receives up to one quarter of the configured limit, capped at 32 MiB, and Parquet receives the remainder. At the 50 MiB default this is 12.5 MiB for ROW and 37.5 MiB for Parquet. The separate size cache is bounded by its 4,096-entry limit. This addresses the review finding that the two parsed caches previously exceeded the configured budget when added together.
This PR supersedes #1195. It optimizes the existing
.rowsidecar path; it introduces no new option or file-format change and does not add the experimental scalar.srowformat.Validation
cargo test --locked -p paimon --features storage-memory row_and_parquet_metadata_share_configured_budget --lib(1 passed)cargo test --locked -p paimon --features storage-memory row_reader_ --lib(7 passed; includes counted footer/index reads)cargo test --locked -p paimon --features storage-memory immutable_row_sidecar_size --lib(2 passed; includes countedstatcalls and disabled-cache case)cargo test --locked -p paimon --test data_evolution_row_sidecar_test(14 passed)cargo clippy --locked -p paimon --lib --features storage-memory -- -D warnings -A clippy::nonminimal_bool(passed; suppression covers an unrelated existing lint)cargo fmt --all -- --check;git diff --cached --check(passed before commit)