linliu-code opened a new pull request, #697:
URL: https://github.com/apache/hudi-rs/pull/697
## Description
Independent of the #691 to #696 stack: based on `main`, touching only
`crates/core/src/hfile/key.rs`.
Parsing one HFile key-value copied the entire uncompressed data block twice,
once into the `KeyValue` and once into the `Key` inside it. The buffer handed
to both is the block, not the record, so parsing a block of N records copied it
2N times and iterating a block cost records times block size. Measured on
generated blocks holding one data block each, per-record cost was 7.83us at 200
records, 16.06us at 1,000 and 66.15us at 5,000: growing with the block rather
than staying flat.
Both structures index everything relative to their `offset`, so each can
instead hold the bytes it actually spans with the offset at zero, leaving every
accessor's arithmetic as it was. Per-record cost is then 0.16us, 0.16us and
0.12us at those three sizes, and iterating a block is 49 to 531 times faster
across that range. The flatness matters more than the multiple: a fix aimed at
the wrong cause would have moved the level without changing the shape.
Two behaviour notes, both stated because they are not visible from the diff.
`Key::content` now bounds its slice against the key's own bytes rather than
against the rest of the block. On corrupt input where the inner length prefix
claims more content than the key holds, it yields empty instead of reading on
into the value that follows. A well-formed HFile cannot reach that, since a key
length is `2 + row + 1 + family + qualifier + 9` and so always covers its
content. Every other accessor is unchanged, checked by walking all 104,200
records of the nine fixtures and comparing against the previous implementation
field by field.
`Key::content_offset` becomes private. It is an offset into the key's own
bytes, so pairing it with a block buffer, which used to be correct, would now
index the wrong place. Nothing outside the module used it. `Key::bytes` stays
public but its meaning narrows from the enclosing block to the key itself; no
caller in `crates/`, `python/` or `cpp/` reads it.
Lengths come straight out of the file and nothing upstream validates them,
so the new slices clamp to the buffer. Corrupt lengths panic in the same places
with the same messages as before rather than becoming silently wrong output.
While reviewing this, a pre-existing bug turned up in the same file and is
filed separately rather than fixed here: `find_block_for_key` builds its search
key with `Key::from_bytes` from a raw lookup string with no length prefix, so
the block-index range query never resolves and every seek degrades to a linear
block walk. Results stay correct, which is why nothing caught it. Both `Key`
construction sites involved are byte-identical before and after this change.
## How are the changes test-covered
- [ ] N/A
- [x] Automated tests (unit and/or integration tests)
- [ ] Manual tests
- [ ] Details are described below
One test added, and it is the one the existing suite lacked. Every other
test in `key.rs` parses at offset 0, where a record and the buffer it came from
are nearly the same thing, so none of them would notice a parse that ignored
the offset or sliced from the start of the buffer. The new test lays two
records out as a data block does, parses the second at the offset the block
iterator would advance to, and asserts it reads its own key and its own value.
It then asserts the narrowing itself: the key holds exactly `key_length` bytes,
the record holds exactly header plus key plus value, and less than the whole
block.
That last group is what guards the regression, and it does so without timing
anything, so it cannot be flaky. Reverting the change fails it with "a parsed
key must hold exactly its own bytes, not the block's".
Equivalence was checked more broadly than the new test covers: a throwaway
differential harness reimplemented the previous semantics side by side and
compared `content`, `content_length`, `value`, `record_size`, `key_length` and
`value_length` for every record of every data block of all nine fixtures,
across GZ and NONE codecs, two and three level indexes, shortened first keys
and non-unique keys. 104,200 records, no mismatches. Six corrupt inputs were
run against both implementations and panic identically.
A timing benchmark was written for this and deliberately not included: it
needs a directory of generated blocks too large to commit, so it could never
fail in CI and carried no signal. The measurements above are reproducible from
the generator described in the ticket.
Run locally: 1300 pass with default features, 1280 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. No CI run has happened: fork pull
requests sit at `action_required` until a committer approves them.
--
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]
