[GitHub] [iceberg] amogh-jahagirdar commented on pull request #4763: AWS : Use strongly consistent read while acquiring lock via DDBLockManager

[GitBox <gi...@apache.org>]

amogh-jahagirdar commented on PR #4763:
URL: https://github.com/apache/iceberg/pull/4763#issuecomment-1126509083

   Overall, I think this change should be fine, but I just want to verify a few things to make sure my understanding is correct:
   
   1.) When acquiring the lock we do conditional puts to validate that no other entity is holding the lock, or in the case where there is an existing lock found in the eventual consistent read, we validate via the conditional put that it's not a different lease id than expected. Could we elaborate on the case (time where where an eventual consistent read of the lock will lead to a correctness issue in different entities trying to grab the lock). The case that comes to mind is we read an outdated version and then in the conditional put to acquire the lock we fail to acquire it since the versions are different. Perhaps in streaming workloads where the lock is repeatedly getting acquired and released, there's also a chance that the version is out dated due to eventual consistency; but in those cases the strong consistent read may lead to longer times to acquire the lock as opposed to maybe failing faster on the put? I'll need to think more on the tradeoff here I suppose.
   
   2.) Considering Dynamo locking was originally built due to a lack of optimistic locking support from Glue, I am not sure how many new users would use this locking support and so we may not want to add to it unless there's a correctness issue in the locking (1); if there's a correctness issue, definitely makes sense to fix that.
   
   So in short, if there's a correctness issue in terms of acquiring the lock due to the eventual consistent read, makes sense and would be nice to see an example timeline of systems grabbing the lock.
   
   If it's more for reducing the chance of failure on the conditional put, would want to make sure it's a change that existing users for DDB Lock manager would benefit from in their workload considering new users could just use GlueCatalog and optimistic locking.


-- 
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-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org