dancsi opened a new pull request, #4023: URL: https://github.com/apache/iceberg-python/pull/4023
Closes #4021 Closes #4022 # Rationale for this change When `commit_table` raises `CommitFailedException` after the catalog has already applied the commit (a lost response), `Transaction.commit_transaction` can delete the landed snapshot's manifest list and manifests, which leaves the table unreadable: - #4021: on the last attempt (including `commit.retry.num-retries=0`, or after the total timeout), the cleanup runs without checking whether the attempt landed. - #4022: the landed check requires a snapshot from every producer. A producer that adds no snapshot, such as the delete in an overwrite of an empty table, makes it always false. The retry then rebuilds the updates, the rebuilt delete sees its own landed append as a conflict, and the resulting `ValidationException` cleans up. This change: - bases the landed check on the `AddSnapshotUpdate` snapshot ids that were actually sent to the catalog. Each attempt is atomic, so finding any of them proves an attempt landed; - runs the check after every `CommitFailedException`, including on the last attempt, before any cleanup; - raises `CommitStateUnknownException` (chained to the original error) if the refresh for the check fails. That exception skips the cleanup, so files a landed snapshot may reference are never deleted when the outcome can't be verified; - still refreshes before rebuilding a retry that sent no snapshots, so its validation sees current metadata; - runs the post-commit cleanup of superseded attempts' manifests once, after the loop. ## Are these changes tested? Yes, in `tests/table/test_commit_retry.py`, parametrized over the existing catalog fixture: - a lost response on the last attempt, with no retries and with retries exhausted; - a lost response for an overwrite of an empty table; - a lost response whose landed check can't refresh the table. Each asserts that exactly one snapshot exists, that its manifest list is still on disk and that the table scans. All of them fail on `main`. ## Are there any user-facing changes? A lost response on the final attempt now returns successfully when the commit landed, instead of raising `CommitFailedException`. If the landed check itself can't reach the catalog, `CommitStateUnknownException` is raised instead of `CommitFailedException`. Generated-by: Claude Code (Claude Opus 5.5) -- 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]
