linliu-code opened a new pull request, #696:
URL: https://github.com/apache/hudi-rs/pull/696
## Description
**Stacked on #695, which is stacked on #694, #693, #692 and #691. Review
those first.** Until they merge the diff here shows their commits too; the
delta is `eede8d9..HEAD`, ten files.
A log block whose record count exceeds the decoder's batch size fails to
decode at all: `Offsets must be non-negative and within the length of the
Array`. The batch size is 1024 and a metadata table's log blocks exceed that
routinely, so the version two metadata read cannot handle a realistic metadata
table. Nothing in the test corpus saw it, because every metadata fixture block
holds two or three records and the largest data fixture of any kind is 458 KB.
The trigger is any Avro union with two or more non-null branches, which
Arrow represents as a dense union, and which the metadata record schema always
carries in its column statistics. Unions spelled `["null", T]` are unaffected:
Arrow makes those a nullable `T`, with no offsets buffer. `arrow-avro`'s union
decoder drains its offsets on flush but keeps the per-branch counts that
produced them, so the second batch emits offsets past children it has just
emptied. Reported upstream as apache/arrow-rs#10876 with a three-line
reproducer. Its decoder exposes no reset, so a clean one has to be built, which
now happens after a mid-block flush. A block that fits inside one batch never
reaches that path and pays nothing.
Rebuilding per batch would otherwise repeat schema-sized work, so the writer
schema's parse and fingerprint are cached and reused across blocks. Measured on
the metadata record schema, which is 7,969 bytes of JSON, that registration is
234us of the roughly 400us a construction costs. What remains is the two parses
inside `arrow-avro`'s own decoder construction, which its public API gives no
way to skip.
Separately, the header walk now keeps a block's content when its fetch
window already covered it, instead of seeking past and re-requesting the same
bytes in pass three. That is one request per log file rather than two, matching
what the eager reader has always cost. The bytes are copied rather than sliced,
and charged against a budget of one window shared across the whole scan: a
`Bytes` slice pins its entire window allocation, and the walk accumulates
blocks from every log file before the gates run, so residency would otherwise
grow with the slice instead of staying at the bound the headers-only walk
exists to buy.
Stage timings move from milliseconds to microseconds. A metadata read
completes in about a millisecond, so every stage rounded to zero and the
counters said nothing. Four timers that wrapped an empty span are removed: the
fetch they were written for had moved to pass three, leaving
`log_block_decode_ms` structurally zero. The log path is now split into fetch,
decode and upsert, which is what located the cost described above.
## How are the changes test-covered
- [ ] N/A
- [x] Automated tests (unit and/or integration tests)
- [ ] Manual tests
- [ ] Details are described below
Three additions and two re-derivations.
The decode fix has two tests over a three-line hand-written schema with a
two-branch union, encoding 100 records at a batch size of 16, so they need no
fixture and no generated data. One asserts every row returns and that more than
one batch was actually produced, so it cannot pass vacuously if a batch-size
change ever makes 100 records fit in one. The other alternates the union's
branches and checks every row's identity across the boundary, because a corrupt
offsets buffer returns wrong values rather than only crashing. A companion test
without a union pins that multi-batch decoding is not broken in general. With
the rebuild removed, both union tests fail with the upstream error and the
non-union test still passes.
The one-request-per-log-file change is asserted by request count through a
store that wraps the real one, not by rows returned: both paths return the same
records over the same file, which is the point. Inside one window every block
is resident and the file costs one get; with a 64 byte window every block
defers and it costs more than one. A separate test pins that the residency
budget is a bound: a generous budget lets the walk keep content, a zero budget
keeps none, and a budget below the total keeps less than everything.
Two existing tests changed subject and were re-derived rather than inverted.
`test_load_content_reports_a_block_running_past_the_file_end` truncated its
file after the walk, which no longer leaves a ranged read to come back short;
it now forces deferral with a one byte window and keeps its assertion, plus a
guard that deferral actually happened.
`test_prefetch_returns_content_for_every_admitted_block` had become vacuous,
since both of its blocks were resident and `fetch_window` was never called; it
now walks with a zero residency budget and asserts nothing is resident, so the
keying it exists to pin is exercised again.
Run locally: 1382 pass with default features, 1362 with
`--no-default-features`, clippy clean on both feature sets with warnings
denied, fmt clean, and `cargo check --workspace --all-targets --all-features`
clean including the Python and C++ bindings. That last one matters here:
`HoodieReadStats` is public and seven of its fields were renamed. 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.
Not covered, and stated rather than implied: the correctness fix makes a
large block readable, not fast. Decode is superlinear in records per block,
6.72us each at 200 records against 68.89us at 5,000, which is a separate defect
this fix made visible rather than caused. It is filed and not addressed here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
