perf(table): load equality deletes lazily per scan task - #1963
Conversation
83b9c1f to
d2a6745
Compare
Signed-off-by: Minh Vu <vuhoangminh97@gmail.com>
Signed-off-by: Minh Vu <vuhoangminh97@gmail.com>
Signed-off-by: Minh Vu <vuhoangminh97@gmail.com>
Signed-off-by: Minh Vu <vuhoangminh97@gmail.com>
d2a6745 to
e692bac
Compare
zeroshade
left a comment
There was a problem hiding this comment.
This defers equality-delete I/O without changing which delete files each planned task receives or how their values are matched. Approving.
The sibling change for position deletes (#1938) had the same shape, so I verified each risk independently here rather than assuming parity.
- Semantics preserved. The planner's sequence and partition gates are untouched (
table/equality_delete_index.go:174-205— delete sequence > data sequence, matching partition/global scope), and the lazy loader consumes only the files already assigned to a task.buildEqualityDeleteSetsForTaskloads every task-assigned equality file, groups by equality-field IDs, and unions the complete key sets, so the delete set is whole when consulted. Equality deletes apply by value across files, so this was the thing that most needed checking. - Concurrency. Each delete file has a correctly scoped
sync.Oncepublishing both the decoded set and any error to every caller — not just the first (table/equality_delete_reader.go:281-382). The shared-combination cache uses async.Mapwith a per-combinationsync.Once, so tasks sharing a delete-file combination don't duplicate reads or merges. The files map is read-only after construction. - No leak on abandonment. The worker pipeline is created inside the returned iterator, so an iterator that is never ranged over performs no equality-delete I/O and has no reader to leak. On early stop or error,
createIteratorcancels workers, drains the sequencer, and releases queued Arrow batches viaMakeSequencedChanWithDiscard(table/internal/utils.go:92-106). - Errors and cancellation. Load failures are sent into the ordered iterator and cancel the scan with the same error (
table/arrow_scanner.go:1796-1865). The scan context threads through reader acquisition and record reading with cancellation checks while decoding, and both readers close on all return paths (:606-716). - Error timing is documented, which matters because it's observable:
table/scanner.go:1451-1454states setup vs iterator error behaviour, and the ambiguous-column test now assertsErrAmbiguousEqualityColumnsurfaces during iteration.
Tests cover on-demand and unread behaviour, concurrent single-read reuse, cached read errors, cancelled contexts, and queued-batch release. BenchmarkLazyEqualityDeleteLoading is committed; the body reports ~25–30× lower time and allocation for unread and partial scans with comparable full-scan cost — which is exactly the profile you'd want from this.
No findings. CI green (15/15), all 4 commits signed off.
This review was drafted by an AI-assisted tool and confirmed by an Apache Iceberg Go maintainer, who has read the findings and signed off. If something feels off, please reply on the PR and a maintainer will follow up.
More on how to contribute to Apache Iceberg Go: CONTRIBUTING.md
Summary
The old path read every unique equality-delete file before returning the iterator. This change only reads files needed by tasks that are actually processed.
Benchmark
Setup: 10,000 scan tasks, 1,000 equality-delete files, 10 keys per file, 16 workers. Three runs with
-benchtime=1son an Apple M1 Pro.Tests
go test ./table -count=1go test -race ./table -count=1go vet ./table