zeroshade commented on code in PR #1225:
URL: https://github.com/apache/iceberg-go/pull/1225#discussion_r3523596359
##########
table/rolling_data_writer.go:
##########
@@ -429,6 +428,8 @@ func (r *RollingDataWriter) closeAndWait() error {
return fmt.Errorf("error in rolling data writer: %w", err)
}
+ r.cancel()
Review Comment:
Blocking: moving `cancel()` here does not fix the fanout drain path.
`RollingDataWriter` instances are created with the `errgroup.WithContext`
context in `table/partitioned_fanout_writer.go` around line 160, and
`fanoutWorkers.Wait()` cancels that context before `writerFactory.closeAll()`
drains writers around line 188. That means `closeAndWait()` can still drain
buffered records under a canceled context and hit `context.Canceled` in
`stream()` / `openFileWriter` / `ToRequestedSchema` / `SortRecordBatch`. Please
decouple the per-writer drain context from the fanout errgroup context on
success, and cancel writers only on the abort/error path. This is the approach
already implemented in PR #1368; please consolidate with or adopt that fix.
--
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]
