tanmayrauth commented on issue #2341:
URL: https://github.com/apache/iceberg-go/issues/2341#issuecomment-6475257025

   +1 on the distinction. Treating every non-conflict error as unknown state 
makes a plain rejection unrecoverable, and catalogs often know nothing was 
written.                                                                        
                                                                                
     
                                                                                
                                                                                
                                                                                
                                                                               
     Java has a direct precedent: the CleanableFailure marker interface, 
documented as "commit exceptions where the state is known to be failure and 
uncommitted metadata can be cleaned up." TableOperations.requireStrictCleanup() 
defaults to true. SnapshotProducer and BaseTransaction only clean up when the 
failure is a  CleanableFailure. Anything else is treated as possibly committed, 
which matches what Go does for unclassified errors. So ErrCommitRejected would 
be the Go equivalent, and it might be worth naming or documenting it that way 
so the parity is obvious.                                                       
           
                                                                                
                                                                                
                                                                                
                                                                               
     That precedent also answers the opt-in question: both. Java's marker is 
opt-in by error type, but the REST exceptions already carry it. 
BadRequestException, ForbiddenException, NotAuthorizedException and 
NoSuchTableException all implement CleanableFailure. So Java's REST commit path 
already treats 400, 401, 403   and 404 as cleanable. I'd have the Go REST 
catalog classify the same codes, including 401 (ErrUnauthorized), which the 
issue doesn't list. One caveat: when the error body can't be decoded, an 
unmapped status falls back to plain ErrRESTError (rest.go ~684-690). If 
400/401/403 should be classified in that case too, they need to go into the 
commit override map.                                                            
                                                                                
                                                                                
                                                 
                                                                                
                                                                                
                                                                                
                                                                               
     On resumable vs. unusable: Java doesn't really keep it resumable. A 
cleanable failure in BaseTransaction deletes the staged files. Go deliberately 
keeps staged files for resume, though, and a deterministic rejection would just 
fail again quickly on the next Commit, so I'd lean resumable unless there's a 
case where resubmitting does harm.  


-- 
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]

Reply via email to