alexandre-normand opened a new issue, #1666: URL: https://github.com/apache/iceberg-go/issues/1666
### Apache Iceberg version main (development) ### Please describe the bug 🐞 We have a streaming ingestion into iceberg that uses iceberg-go to write to iceberg tables and we saw repeated cases of tables getting corrupted due to snapshots referring to missing manifest list files (and potentially missing manifest files as well). At a high-level, what we're seeing in those cases of corrupted snapshots is: * We create a transaction that does [AddFiles](https://pkg.go.dev/github.com/apache/[email protected]/table#Transaction.AddFiles) * We call [transaction.Commit](https://pkg.go.dev/github.com/apache/[email protected]/table#Transaction.Commit) and get an error back. For example, we might get transient 503 errors on GCS that show up like this: ``` blob (key "<redacted>/metadata/snap-1200212384152206267-0-deb76786-3779-4042-b0d2-8e321fcb1929.avro") (code=Unknown): googleapi: got HTTP response code 503 with body: Service Unavailable ``` * Our application retries by refreshing the table and trying the commit again and it succeeds. However, the failed commit still produced a snapshot pointing to a missing manifest list file and this causes queries / reads on that table to fail. The only path to recovery is restoring from a previous snapshot and replaying write operations. ## Investigation I reviewed the implementation of [commitManifests](https://github.com/apache/iceberg-go/blob/main/table/snapshot_producers.go#L1160) and it looks like the issue has to do with how the errors on Close are all evaluated as deferred calls (example [here](https://github.com/apache/iceberg-go/blob/main/table/snapshot_producers.go#L1187) for the snapshot list writer: ```go out, err := sp.io.Create(manifestListFilePath) if err != nil { return nil, nil, err } defer internal.CheckedClose(out, &err) ... err = iceberg.WriteManifestList(sp.txn.meta.formatVersion, out, sp.snapshotID, parentSnapshot, &nextSequence, firstRowID, newManifests) if err != nil { return nil, nil, err } ``` The implementation assumes that errors writing files will be returned on the write operations *but* the gocloud.dev blob writer [states](https://github.com/google/go-cloud/blob/master/blob/blob.go#L432-L436) that writes can't be guaranteed to be successful until Close returns without errors: ```go // Close closes the blob writer. The write operation is not guaranteed to have succeeded until // Close returns with no error. // Close may return an error if the context provided to create the Writer is // canceled or reaches its deadline. func (w *Writer) Close() (err error) ``` This lines up with the behavior we're seeing where we get an error back from Commit because it happens on the deferred `Close` calls but the actual implementation had already gone through the full commit steps only checking the errors on `Write`. -- 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]
