AnirudhSriram opened a new issue, #2518: URL: https://github.com/apache/iceberg-go/issues/2518
### Apache Iceberg version v0.7.0, and still present on `main` ### Please describe the bug 🐞 `binPackRecords` calls `panic(err)` when the source record iterator yields an error. The iterator runs on the feeder goroutine that `internal.MapExec` starts, so the caller can't `recover()` it. A single unreadable data file kills the whole process instead of failing that write. https://github.com/apache/iceberg-go/blob/main/table/arrow_utils.go#L2187-L2192 ```go func binPackRecords(itr iter.Seq2[arrow.RecordBatch, error], recordLookback int, targetFileSize int64) iter.Seq[[]arrow.RecordBatch] { return internal.PackingIterator(func(yield func(arrow.RecordBatch) bool) { for rec, err := range itr { if err != nil { panic(err) } ``` Both unpartitioned delete writers use it: `positionDeleteRecordsToDataFiles` and the equality delete writer. The partitioned fanout writers already return iterator errors correctly. **How we hit it.** We run iceberg-go to merge CDC changes into tables that Spark wrote before. Spark had written some data files with one parquet page of 100 to 170 MB. When an update hit a row in one of those files, the position delete write read the file. arrow-go v18.8.0 rejected the page (`DefaultMaxCompressedPageSize` is 64 MiB), and the read error became a process crash: ``` panic: parquet: compressed page size 172113501 exceeds configured limit 67108864 goroutine 5072116 [running]: github.com/apache/iceberg-go/table.binPackRecords.func1-range1(...) [email protected]/table/arrow_utils.go:1985 +0xcd github.com/apache/iceberg-go/table.createIteratorWithCleanup.func5(...) [email protected]/table/arrow_scanner.go:2421 +0x482 github.com/apache/iceberg-go/table.binPackRecords.func1(...) [email protected]/table/arrow_utils.go:1983 +0x9f github.com/apache/iceberg-go/table.binPackRecords.PackingIterator[...].func4() [email protected]/internal/utils.go:159 +0x16e github.com/apache/iceberg-go/table.positionDeleteRecordsToDataFiles.func5(...) [email protected]/table/arrow_utils.go:2243 +0x1cc github.com/apache/iceberg-go/table/internal.MapExec[...].func2() [email protected]/table/internal/utils.go:685 +0x138 created by github.com/apache/iceberg-go/table/internal.MapExec[...] in goroutine 5068027 [email protected]/table/internal/utils.go:678 +0x2c9 ``` Our process restarted, replayed the same change, and crashed again. Every other table handled by that process stalled too. **Expected:** the write returns the read error through the `iter.Seq2[iceberg.DataFile, error]` it already returns, so the caller can fail or retry that one operation. **Repro.** Pass `positionDeleteRecordsToDataFiles` an `itr` that yields one batch and then an error. The process panics. **A possible fix**, which we run in a fork: `binPackRecords` stores the first error and stops, and the two callers yield it after `writeFiles` finishes. The feeder goroutine sets the error before `MapExec` closes its output channel, so the read after the loop is race-free. Commit: https://github.com/artie-labs/iceberg-go/commit/c6cc571. Happy to open a PR. **Related.** There's no way to raise the arrow-go page limits: `parquet.NewReaderProperties` is called internally in `table/internal/parquet_files.go`. Spark can write pages over 64 MiB, so a read option or table property for `MaxCompressedPageSize` / `MaxUncompressedPageSize` would help. We can file that 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: [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]
