jackye1995 commented on pull request #1633: URL: https://github.com/apache/iceberg/pull/1633#issuecomment-718143478
> Thanks, @jackye1995! This looks like a great start and mostly correct to me. > > What is the plan for atomic commits? I don't see locking in this PR, so it looks like that will be added later with some configuration to enable it and point a catalog to a dynamo instance? It would also be great to know what the timeline for an atomic swap in Glue is. Maybe we could avoid adding the external locking, although it may be useful to have an external locking option. Currently the `doCommit` method uses Glue's `ConcurrentModificationException` to detect commit conflicts. However, because we are doing a client-side compare-and-swap, there is still a potential race condition. But the code should still work for low concurrency use cases. I am actively discussing with Glue about a native support, but it's not a very common use case for them to support yet at this point of time. Yes, there will be another PR for adding the DynamoDB locking. When I add that, it will be enabled based on a config value, and user can choose to enable it or not based on the access pattern. As discussed in the original draft, I did not include this functionality to control the size of the PR. I already have the code, just waiting for this PR to be merged before adding that support. ---------------------------------------------------------------- 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. 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
