RussellSpitzer commented on issue #18608:
URL: https://github.com/apache/iceberg/issues/18608#issuecomment-6421269906

   I'm still not following the jump from "Isolation level for delete commands" 
to "this applies to appends." The property name and the description both scope 
it to deletes. Isolation level is a table-level guarantee about what a commit 
is allowed to have observed, and in that sense appends are serializable too. An 
append reads nothing, so no concurrent commit can invalidate it, which is why 
there is nothing for it to validate. Explaining that properly means writing a 
guide to serializability in Iceberg, and I think that's out of scope for the 
configuration table. If you're changing these properties, you need to 
understand the guarantees you're giving up.
   
   I also think the proposed sentence is misleading:
   
   > Applies only to the DELETE, UPDATE or MERGE command being committed: it 
controls whether data files added by concurrent commits (including appends) 
count as conflicts. It has no effect on how appends are committed; an append 
that loses a commit race is retried according to commit.retry.*.
   
   Every commit that loses a race is retried according to `commit.retry.*` 
(This includes DELETE, UPDATE and MERGE.) On retry, the operation re-validates 
against the new table state, and the isolation level decides whether that 
validation passes:
   
   *serializable*: no concurrent commit added or removed data that matches what 
this operation read
   *snapshot*: no concurrent commit removed or deleted data that matches what 
this operation read
   
   Appends have no read set, so they always pass.
   
   For your workload, the 409s on the Flink side are plain commit contention. 
Thirty writers checkpointing every minute against one table means roughly 27 
commits a minute competing for the same metadata pointer. Isolation level can't 
help with that. Fewer commits (longer checkpoint intervals, or fewer writers 
per table) or more generous commit.retry.* settings can.
   
   
   So my vote would be no change to the documentation here. 
   


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