linliu-code opened a new pull request, #705:
URL: https://github.com/apache/hudi-rs/pull/705
## Description
A metadata partition was read from **exactly one** file slice, and anything
else was refused:
```rust
if file_slices.len() != 1 {
return Err(CoreError::MetadataTable(format!(
"Expected 1 file slice for {} partition, got {}", …)));
}
```
That holds for `files`, which is one file group in practice. It does not
hold for the partitions that **shard** — record index above all, and secondary
index — whose design is many file groups with keys hashed across them. The
fixture's own `record_index` has **ten** file groups,
`secondary_index_rider_idx` ten, `column_stats` two. The reader never cared how
many slices there were; only this check did.
**The fan-out is not new code.** `Table::read_file_slices_bounded` already
read slices with a ceiling, in order, chunk by chunk, and its reasoning was
worth preserving verbatim:
- order is preserved (`buffered`, not `buffer_unordered`) because a caller
concatenating batches should not see rows move with scheduling;
- it chunks rather than sliding because the futures borrow their
`FileSlice`, and the higher-ranked lifetimes inside `FuturesOrdered` then
defeat `Send` inference for any caller that spawns the read.
That logic now lives in `util::concurrency::bounded_in_order`, and **both**
the table scan and the metadata read call it — one implementation rather than
two that drift. Tying the closure's input lifetime to the slice is
load-bearing: a plain `Fn(&T) -> Fut` cannot name the borrow its future holds,
and the compiler rejects it with an unsatisfiable `'1 must outlive '2`.
The same `hoodie.read.file.slice.read.concurrency` bounds both paths, so a
sharded metadata partition cannot open more readers at once than a table scan
already would. Concatenating a single slice returns its batch untouched — no
copy added to the path that existed before sharding was supported.
**Scope.** This is the execution model only. The pruner is still pinned to
the `files` partition, so no new partition is served yet; that is ENG-47771's
scope. This exists so routing has somewhere to route to.
## How are the changes test-covered
- [ ] N/A
- [x] Automated tests (unit and/or integration tests)
- [ ] Manual tests
- [ ] Details are described below
**The multi-slice read**,
`a_files_partition_of_several_slices_reads_all_of_them`. The fixture's `files`
partition is made genuinely multi-slice by copying its base file under a second
file id. Slice discovery is a storage listing, so this needs no timeline edit —
which on table version 8 is Avro-encoded and not something a test should have
to write.
The assertion is a **doubled row count**, which separates three outcomes
that "the read succeeded" would not: the old refusal errors, a one-slice read
returns N, a correct two-slice read returns 2N.
The test asserts its own premise before asserting anything else, and that
caught a wrong premise: the partition holds **two base files in a single file
group** at different instants, not two groups. Without the premise check the
test would have been copying a file into a group that already existed.
**The ceiling**, three unit tests on `bounded_in_order`: results keep input
order; no more than `concurrency` operations run at once — asserted on the
*observed* peak, not the configured value, so it fails if the chunking is
removed; and a concurrency of 0 clamps to 1 rather than silently reading
nothing, which would look like an empty table.
**Mutation-checked with a positive control:**
| mutation | result |
| --- | --- |
| read only the first slice, as before | FAILED — 4 rows against 8 |
| restore the single-slice refusal | FAILED — with its error |
| unmutated | passes |
```
cargo test -p hudi-core --lib 1405
passed, 0 failed
cargo test -p hudi-core --lib --no-default-features 1385
passed, 0 failed
cargo test --workspace --all-targets --all-features 0
failing suites
cargo clippy -p hudi-core --all-targets -- -D warnings clean
cargo clippy -p hudi-core --lib --no-default-features -- -D warnings clean
cargo fmt --all -- --check clean
```
**Not covered, stated rather than implied.** No test reads a partition with
ten real shards, because none is reachable while the pruner is pinned to
`files`. The two-slice case proves the fan-out and the removal of the refusal;
it does not exercise routing a key to the shard that holds it, which arrives
with ENG-47771.
Separately measured while building this, and worth knowing before the
concurrency default is raised: on a merge-on-read table each concurrent slice
costs roughly 340 MB, and neither streaming nor `hoodie.memory.merge.max.size`
bounds it. Tracked in ENG-47885; the default of 1 is why this change does not
depend on it.
--
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]
