You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "fallintoplace (via GitHub)" <gi...@apache.org> on 2026/08/29 13:51:50 UTC
[PR] perf(table): stream data manifest entries into tasks [iceberg-go]
fallintoplace opened a new pull request, #1974:
URL: https://github.com/apache/iceberg-go/pull/1974
## Summary
- **Streams data manifest entries directly into scan tasks** instead of keeping a global data-entry slice.
- Reads delete manifests first and builds positional, equality, and deletion-vector indexes.
- Reads data manifests concurrently while keeping task order by manifest order.
- Uses one shared task buffer when manifest counts are known. Unknown counts use a safe fallback.
- Keeps the existing collector for incremental append scans.
## Tests
- `go test ./...`
- `go test -race ./table`
- `go vet ./table`
## Benchmark
Command:
`go test ./table -run '^$' -bench '^BenchmarkPlanDataManifestTasks/manifests=8/entries=10000/(buffered|streamed)$' -benchmem -benchtime=1s`
Apple M1 Pro, 8 manifests x 10,000 entries:
| Path | ns/op | B/op | allocs/op |
| --- | ---: | ---: | ---: |
| buffered | 28,409,791 | 86,939,909 | 905,562 |
| streamed | 23,727,901 | 80,683,642 | 905,561 |
The streamed path is about **16% faster** and uses about **7% fewer allocated bytes** on this workload.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] perf(table): stream data manifest entries into tasks [iceberg-go]
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade commented on PR #1974:
URL: https://github.com/apache/iceberg-go/pull/1974#issuecomment-5488005008
My approval above stands, but this went conflicted before I could merge it.
Same cause as #1965: the duplicated `schema.go` `MarshalJSON` lazy-cache hunk shared across this stack has now landed on `main`, so the identical hunk here conflicts. A rebase onto current `main` should drop it and leave just the streaming-planner change.
Once rebased and green I'll merge — no need to re-request review. The two minor follow-ups from my review (context check between entries, planner-level edge-case tests) are fine to pick up here or separately.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] perf(table): stream data manifest entries into tasks [iceberg-go]
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade commented on PR #1974:
URL: https://github.com/apache/iceberg-go/pull/1974#issuecomment-5488575154
Update: I rebased #1965 and #1976 onto current `main` and merged both. I stopped short on this one, because its rebase is not mechanical and I did not want to guess at your design.
The `schema.go` hunk drops out cleanly (already upstream). The real conflict is that `PlanFiles` has changed underneath this PR in three ways that interact with the streaming restructure:
1. **#1939 splits one entry into N tasks.** `splitParquetScanTask(task, splitTargetSize)` can emit multiple tasks per manifest entry. `planDataManifestTasks` sizes its `directBuffer` via `manifestTaskCapacity` at exactly one task per live entry, so that invariant no longer holds.
2. **#1971 computes per-task residuals.** Main keeps `residualEvaluators` as a plain `map[int]*partitionResidualEvaluator` populated per entry, which is fine sequentially but would race inside your `g.Go` workers.
3. **Result metrics accumulate per task.** `acc.resultDataFiles++` and `acc.addResultDeleteMetrics(task)` are unsynchronized for the same reason.
So reapplying this needs decisions that are yours to make: whether residuals are computed inside each worker (needing a concurrency-safe per-spec cache) or in a sequential pass after flattening; whether splitting happens inside the worker (giving up the exact-capacity buffer) or afterwards (costing a second pass, which partly offsets what the PR is optimizing); and how metrics accumulate safely.
My review and approval above still stand on the design as reviewed — this is purely about how it lands on top of what has since merged. Also note `openManifest` no longer exists, so the `scan_metrics.go` comment referencing it needs updating to match `streamManifest`.
Related: #1976 is now merged and reworks `collectManifestEntriesWithSchema` into per-manifest result slots, which your delete-manifest path still uses. Worth rebasing on top of that rather than around it.
Happy to review again once rebased. If you'd rather I take a pass at the merge and you review my choices, say so and I will.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] perf(table): stream data manifest entries into tasks [iceberg-go]
Posted by "laskoviymishka (via GitHub)" <gi...@apache.org>.
laskoviymishka merged PR #1974:
URL: https://github.com/apache/iceberg-go/pull/1974
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org