perf(rest): cache partition decode plans

[fallintoplace]

Summary

• Cache the resolved partition decode plan by spec ID while decoding REST scan tasks.
• Reuse the plan across inline and fetched task envelopes in one remote scan.
• Keep decoded partition values and maps independent for every file.
• Add focused reuse tests and a decoder benchmark.

Performance

Compared with the parent commit on an Apple M1 Pro, using GOMAXPROCS=1 and 5 benchmark samples:

• 64 tasks: 365 us -> 241 us, about 1.5x faster
• 1024 tasks: 4.23 ms -> 3.82 ms, about 1.1x faster
• Allocations: 2,049 -> 1,862 per operation, about 9% fewer

The cache is shared across inline and fetched task envelopes, so the setup cost is paid once per spec for a remote scan.

Testing

• go test ./... -count=1
• go test -race ./catalog/rest -count=1
• go vet ./catalog/rest
• go test ./catalog/rest -run "^$" -bench "^BenchmarkDecodeScanTasks$" -benchmem -benchtime=300ms -count=3

[zeroshade]

Since this is a draft, i'll hold off on further review until it's marked ready

[zeroshade]

Thanks for completing the end-to-end remote scan-planning path. I found two blocking lifecycle/response-validation issues in the newly activated path; details are inline. Separately, all 10 commits in apache/main..HEAD are missing the Signed-off-by trailer required by CONTRIBUTING.md, so the series needs DCO sign-off before merge. Please also update the PR title and description: they currently describe only partition decode-plan caching, while this patch activates and implements remote scan planning across 15 files and roughly 1,400 added lines.

[zeroshade]

Blocking: activating REST remote planning here makes the existing PlanIO owner leak reachable in normal use. planFilesRemote installs an owning planIOState, but exhausting the ReadTasks iterator releases only its reader lease; the owner is released only by replacement/local replanning through the private closePlanIO. Scan has no public Close and no finalizer, so a one-shot remote scan that reads successfully and is then discarded never invokes PlanIO.Close, leaving the cached prefix-scoped cloud filesystems/credential resources alive. Cloning an already planned scan adds more owners that public callers likewise cannot release. The package tests can clean up only by calling the private helper. Please provide a deterministic public lifetime mechanism or redesign ownership so completed/abandoned scans release their plan IO.

[zeroshade]

The null fetch-response blocker is fixed, and all ten commits now carry the required Signed-off-by trailer. The focused REST and table lifecycle suites pass under the race detector, package tests and vet are clean, and CI is green.

The PlanIO lifecycle issue is still not fully resolved. ReadTasks acquires a reader lease before returning its lazy iterator, but that lease is released only if the iterator is invoked. If the caller abandons the returned iterator without ranging over it and then calls Scan.Close, the owner is released while the reader count remains permanently nonzero, so PlanIO.Close is never called. The inline comment includes the focused reproduction.

Please also update the PR title and description to represent the end-to-end REST remote scan-planning implementation rather than only partition decode-plan caching, and reference issue #1178 as required by CONTRIBUTING.md.

---

This review was drafted by an AI-assisted tool and confirmed by an Apache Iceberg Go maintainer.

More on how Apache Iceberg Go handles contributions:
https://github.com/apache/iceberg-go/blob/main/CONTRIBUTING.md

[zeroshade]

The active-iterator guarantee leaves an unreleasable edge: ReadTasks increments the reader count before returning its lazy iter.Seq2, while releasePlanIOAfter runs only if that sequence is invoked. If the caller receives the iterator, never ranges over it, and calls Scan.Close, the owner is released but the reader count remains permanently nonzero; PlanIO.Close is never called, and iter.Seq2 has no separate close operation. I reproduced this with a focused test. Please defer acquisition until iteration starts or otherwise provide deterministic abandonment cleanup.

[zeroshade]

This PR was reduced to the cache-only change, so I want to be explicit about what happened to my earlier review: my PlanIO lifetime, null fetch-response, and end-to-end title/description findings were raised against the much larger earlier version of this PR. That implementation has since landed via merged and split-out work, and none of those files or lines are in this six-file diff. Those findings are therefore no longer in scope rather than outstanding, and I'm clearing my CHANGES_REQUESTED accordingly.

On the change that is actually here, the partition decode-plan cache looks correct:

• Key correctness — all envelopes in one remote scan share the same immutable table metadata, and Iceberg partition-spec IDs uniquely identify specs within that metadata, so specID is a collision-free key for this scope.
• Lifetime and memory — remoteScanTasks creates one map for the sequential envelope walk and DecodeScanTasks creates one per call. It is not process-global: the map is discarded when the decode operation ends and holds only the distinct spec IDs actually encountered, bounded by the finite spec set. No eviction or invalidation is needed for that lifetime.
• Concurrency — the cache never escapes the decode call and all access is sequential; concurrent scans get independent caches. No mutex or sync.Map required.
• Isolation — entries retain only immutable spec/type/logical metadata, while decoded partition values and output maps stay per-file allocations.

Benchmark evidence is present (Apple M1 Pro, GOMAXPROCS=1, five samples: 64 tasks 365 µs → 241 µs; 1024 tasks 4.23 ms → 3.82 ms; allocations 2,049 → 1,862) and the patch adds BenchmarkDecodeScanTasks. Checked-in raw output or a benchstat comparison would be a nice-to-have for reproducibility — explicitly not a condition of this approval.

CI is green across all 15 checks. Thanks for splitting this out; the narrowed scope made it much easier to review.

---

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