RussellSpitzer commented on PR #9546:
URL: https://github.com/apache/iceberg/pull/9546#issuecomment-1919357852
Based on our discussions last night on slack, I believe that we are
basically in the following situation
```java
commit() {
// Failure here means we throw CommitFailedException everything is broken no
commit
Rename to N+1 Metadata.json // Exceptions here are where we have a Commit
State Unknown because we can't be sure whether or not the rename actually
suceeded
// Failure after rename here means all clients will still read the wrong
hint out of version hint and not be able to see the n+1 meatdata json until
they attempt to commit. This is a Java-Impl bug imho but does not violate spec
as they will not break the history although the commit will remain invisible
until a subsequent commit. This will be painful for some operations like COW
that are heavy but not a violation of spec. We can throw either a
Commit-Success or Commit-StateUnknown Here based on how we feel about this
Write new version Hint
// Failure here is completely fine, commit has succeeded. We successfully
wrote a new version file so even this client will read the new state
Remove old metadata
// Failure here is also fine, Commit has succeeded we just didn't clean up
nice
}
```
So this basically means we need to ignore all exceptions that occur after
the "rename" and for exceptions that occur "during" the rename those have the
possibility of being commit state unknown if we can't check for the new
metadata existing.
This means that the following tests need to pass
// For all tests assume we start with a table at n-metadata.json
1. During the commit renameFile throws an exception but the n+1 metadata
json is written
a. Unknown State Exception must be thrown if we can't find
n+1-metadata.json or Commit Succeeded if we can
b. A subsequent commit must produce n+2-metadata.json
2. During the commit renameFile throws an exception but the n+1
metadata.json is not written
a. Unknown State Exception must be thrown if we can't check that
n+1.metadata.json exists, Commit Failed otherwise
b. Subsequent commit must produce n+1-metadata.json
3. Any Exceptions are thrown after renameFile
a. All exceptions should be caught
b. Commit Succeeded should be returned
c. Subsequent commits should produce n+2-metadata.json
I think that covers everything, we can always split out the 1.a and 2.a into
different cases where the check for n+1 fails or succeeds.
--
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: issues-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
