CTTY commented on code in PR #3560:
URL: https://github.com/apache/iceberg-rust/pull/3560#discussion_r4395155946
##########
crates/iceberg/src/transaction/mod.rs:
##########
@@ -193,16 +193,47 @@ impl Transaction {
let backoff = Self::build_backoff(table_props)?;
let tx = self;
- (|mut tx: Transaction| async {
+ let (tx, result) = (|mut tx: Transaction| async {
let result = tx.do_commit(catalog).await;
(tx, result)
})
.retry(backoff)
.sleep(tokio::time::sleep)
.context(tx)
.when(|e| e.retryable())
- .await
- .1
+ .await;
+
+ let status = match &result {
+ Ok(_) => CommitStatus::Committed,
+ Err(e) => Self::classify_failure(e),
+ };
+ // For `Committed`, the accepted table supports reachability checks.
For
+ // the others, the last base is execution context only.
+ let table = result.as_ref().unwrap_or(&tx.table).clone();
+ for entry in tx.actions {
+ entry.cleanup(&table, status).await;
+ }
+
+ result
+ }
+
+ /// Classifies a failed commit for cleanup, by error kind alone.
+ ///
+ /// Every kind but `Unexpected` is a definite rejection (a catalog
conflict,
+ /// a validation or precondition failure, an unsupported feature), so the
+ /// commit was not applied. `Unexpected` covers both I/O failures before
+ /// submission and catalog errors after it (a 5xx, a dropped connection),
+ /// and the two cannot be told apart, so it is treated as unresolved and
+ /// cleanup deletes nothing. This mirrors iceberg-java's strict-cleanup
+ /// mode, where only `CleanableFailure` exceptions trigger cleanup.
+ // TODO: let errors carry a "definitely not committed" marker so that
+ // pre-submission I/O failures and catalog rejections clean up instead of
+ // leaving orphans.
+ fn classify_failure(error: &Error) -> CommitStatus {
Review Comment:
I think a cleaner solution is probably a `cleanable` field to `Error`. So we
can have a finer granularity on cleanable failures。 Currently it's kept simple
--
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]
