You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hudi.apache.org by "linliu-code (via GitHub)" <gi...@apache.org> on 2026/08/26 17:59:21 UTC

[PR] feat(core): read the metadata table's files partition through the v2 reader [hudi-rs]

linliu-code opened a new pull request, #695:
URL: https://github.com/apache/hudi-rs/pull/695

   ## Description
   
   **Stacked on #694, which is stacked on #693, #692 and #691. Review those first.** Until they merge the diff here shows their commits too; the delta is `8cd7a9a..HEAD`, five files.
   
   There are two metadata-table read paths, and the one production uses never touches the version two file group reader: it opens the base file itself, scans the log files itself, and folds records with its own merger. So every fix to the merge or to the HFile path reaches ordinary reads and not metadata reads, and the metadata table stays on machinery that exists only for it.
   
   This adds the metadata layer that would end that, above the version two reader and owning every metadata concern: the partition, the record shape, the decoded type, the `.` to empty string normalisation a non-partitioned table needs. The reader below is handed a slice, a schema and a predicate, which is the layering Hudi already has between `HoodieBackedTableMetadata` and its generic file group reader. A named-key read pushes the predicate into the base file read, which seeks to the blocks that can hold those keys, and into the HFile log block read, where a log block's own index says which of its blocks to parse. Dropping log records for keys nobody asked for is sound because the merge is per key.
   
   **It is not on the read path, and that is the point of this PR being what it is.** `MetadataTable::read_files_partition` still calls the existing reader. The new layer matches it record for record and value for value, and is **3.4x slower end to end** — 4.24 ms against 1.25 ms on a base file plus eight log files, measured with both readers in one process. Three causes, none of them the layer itself: the ranged base-file open costs a head request and two range requests before any data byte, which is pure overhead on a 7.6 KB metadata file; version two's log scan walks every log file twice, once for headers and once for content, where the older scanner decoded each block as it walked, and that is 2.6 ms of the 3.0 ms difference; and the metadata payload merger rebuilds an Arrow map array per merged pair where the older merger mutated a map in place.
   
   Two of those predate this change, but switching the default metadata read onto that path is what would make them matter, so the switch waits. The layer lands with its parity tests so the comparison is standing and checkable while the cost is worked on, rather than being rewritten later from a description.
   
   Scope is the `files` partition, matching what the existing reader serves. The other five metadata partitions, the key to file group routing they need, and a valid instant filter are separate work.
   
   ## How are the changes test-covered
   
   - [ ] N/A
   - [x] Automated tests (unit and/or integration tests)
   - [ ] Manual tests
     - [ ] Details are described below
   
   Parity against the existing reader, record for record and value for value: file names, sizes and tombstone flags, not key sets. Both readers run in one test over one slice, on the `files` partition before its compaction so the comparison covers a real base file plus seven log files rather than a base alone. A key set comparison would not have been enough; that is how the fold bug fixed in #693 stayed hidden behind an earlier key set assertion. A named-key read is compared the same way.
   
   A named-key read is also asserted to put fewer records through the merge than a full read, measured with the merge map's peak entry count. That number is honest about one mechanism and blind to the rest, which is how the 3.4x above went unnoticed until it was measured end to end; it is kept because it is the only thing that pins the log-side pushdown, and with the predicate withheld from the log decoder both reads peak at four and the test fails on that number.
   
   Every rule has a mutation that fails a named test: the named-key filter, the record context rebuild, the key normalisation, the all-partitions rewrite, the tombstone flag, the size, the null-valued map entry, and the predicate withheld from the log decoder. Three of those were **not** caught on the first pass, all for one reason: the fixture's metadata table is partitioned and holds no tombstones. They are covered by unit tests on constructed batches now, and one of them exposed a real divergence rather than a gap, since the file map was normalising every key as though it were a partition when in a files record those keys are file names.
   
   Three findings from review are fixed here. The row filter built a set once instead of scanning the key list per row, which the two other filter sites in this crate already did and this one did not. A null map value is omitted rather than defaulted, since defaulting invents a zero sized undeleted file and sends a read at a path that does not exist. And the per-block record read reuses the existing helper rather than carrying a second copy of how a record is built from a key value.
   
   The non-partitioned branch, which asks for `.`, is not covered end to end and cannot be: no fixture here is a non-partitioned table with a metadata table. The normalisation it depends on is unit tested instead, and both places say so.
   
   Run locally on both legs: default features and `--no-default-features`, clippy clean on both with warnings denied, fmt clean, the ninety case table read suite green, and `cargo check --workspace --all-targets --all-features` clean including the Python and C++ bindings. No CI run has happened: fork pull requests sit at `action_required` until a committer approves them, so all of the above is local only.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(core): read the metadata table's files partition through the v2 reader [hudi-rs]

Posted by "yihua (via GitHub)" <gi...@apache.org>.
yihua closed pull request #695: feat(core): read the metadata table's files partition through the v2 reader
URL: https://github.com/apache/hudi-rs/pull/695


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] feat(core): read the metadata table's files partition through the v2 reader [hudi-rs]

Posted by "yihua (via GitHub)" <gi...@apache.org>.
yihua commented on PR #695:
URL: https://github.com/apache/hudi-rs/pull/695#issuecomment-5505365303

   Closing this PR since the set of changes has been reviewed in #704.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@hudi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org