voonhous commented on code in PR #20028:
URL: https://github.com/apache/hudi/pull/20028#discussion_r4152485479
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/transaction/lock/StorageBasedLockProvider.java:
##########
@@ -367,22 +430,29 @@ public synchronized boolean tryLock() {
logInfoLockState(FAILED_TO_ACQUIRE);
switch (lockUpdateStatus.getLeft()) {
case ACQUIRED_BY_OTHERS:
- // failed to acquire the lock, indicates concurrent contention
+ // failed to acquire the lock, indicates concurrent contention.
Retrying immediately
+ // would not help — the other owner holds a valid lease.
hoodieLockMetrics.ifPresent(HoodieLockMetrics::updateLockAcquirePreconditionFailureMetric);
- break;
+ return AcquireAttemptResult.NOT_ACQUIRED;
case THROTTLED:
- // The write was rejected; we did not acquire. Transient — caller
may retry.
+ // The write was rejected; we did not acquire. Transient — worth a
bounded retry.
hoodieLockMetrics.ifPresent(HoodieLockMetrics::updateLockThrottledMetric);
- break;
+ return AcquireAttemptResult.RETRIABLE_FAILURE;
+ case TRANSIENT_ERROR:
+ // Storage returned a 5xx. The conditional write cannot clobber
another writer, so a
+ // bounded retry is safe. A 5xx does not prove the write was
rejected, though: if it
+ // landed, the retry finds our own untracked lock and reports it as
held until it lapses.
+
hoodieLockMetrics.ifPresent(HoodieLockMetrics::updateLockTransientErrorMetric);
Review Comment:
IIUC, if a 5xx acquire write actually landed, the retry reads our own valid
lock and gives up with "Lock already held by <self>". That lock has no
heartbeat, so all writers including us, are blocked for the full lease (300s by
default).
Other writers then report it as a dangling lock, which is the production
alert this PR targets.
So the behaviour prior to this PR and after is the same?
--
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]
