voonhous commented on code in PR #20186: URL: https://github.com/apache/hudi/pull/20186#discussion_r4166783875
########## rfc/rfc-91/rfc-91.md: ########## @@ -76,6 +76,48 @@ Once a lock is acquired, a dedicated heartbeat task periodically calls renewLock - Clock drift: we allow for a maximum of 500ms of clock drift between nodes. A requirement of this lock provider is that all writers competing for the same lock must be writing from the same cloud provider (AWS/Azure/GCP). - This will not be configurable at this time. If a storage-specific implementation needs to customize this the config can be added at that time but it should never go below 500ms. +### Ambiguous write results + +The operations above treat each conditional write as having a definitive answer: +- success; +- 412, the file changed since we read or wrote it; +- 429, throttled and not applied, retried with backoff; +- on S3, 409 ConditionalRequestConflict, not applied and to be retried. + +A write can also come back with an *ambiguous* answer, where storage does not say whether it applied the write: a server-side error (HTTP 5xx), a client-side timeout, or a dropped connection. The write may have landed, so the provider must not assume either way. SDK-level retries are disabled in all three storage clients, so the provider sees every such answer. + +The lock file makes this resolvable. Each provider instance has a random `owner` UUID and no other instance writes it. A lock file that carries our owner was therefore written by this instance. + +**Rule.** After an ambiguous answer, the provider retries the operation with the same precondition (bounded attempts, with backoff). It also remembers that the write is unresolved until a definitive answer arrives. While a write is unresolved, a 412 on any later operation does not count as another writer's write straight away: that covers the in-cycle retry, the next heartbeat's renewal, and `unlock()`. The same goes for a lock that looks held on the next acquire attempt. The provider first reads the lock file back. Only if the read-back does not match one of the rows below is the result treated as another writer's. + +| Operation | The read-back is our owner, and... | Outcome | +|---|---|---| +| `tryLock()` | `expired = false`, still valid (expiration plus the clock drift buffer is in the future), and the expiration this call wrote | The acquire write landed. Adopt that lock file, including its version tag, and start the heartbeat. | Review Comment: +1, should we consider. a time budget for retries? -- 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]
