zeroshade commented on code in PR #1781:
URL: https://github.com/apache/iceberg-go/pull/1781#discussion_r3864468507
##########
table/scanner.go:
##########
@@ -789,6 +785,9 @@ func (scan *Scan) collectManifestEntriesWithSchema(
}
g.Go(func() error {
+ // FileIO factories may renew credentials between
manifest reads. Keep
+ // this load at the worker boundary instead of
retaining the IO used for
+ // the manifest list across the whole plan.
fs, err := scan.ioF(ctx)
Review Comment:
Each manifest worker now calls `scan.ioF(ctx)` again, so current head
measures 2, 11, and 101 factory calls/op for 1, 10, and 100 manifests—the same
values documented as “Before,” not the claimed one call/op. This fixes
credential renewal by restoring the original repeated factory behavior, but
removes the performance benefit that is the purpose of this PR. Please preserve
renewal checkpoints while retaining a measurable reuse benefit, or revise the
PR scope if this is intentionally only a refactor. The comment below saying one
FileIO is shared with all workers should also be updated.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
