hudi-agent commented on code in PR #20129:
URL: https://github.com/apache/hudi/pull/20129#discussion_r4150598781
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/transaction/lock/StorageBasedLockProvider.java:
##########
@@ -618,20 +745,98 @@ synchronized ExpireLockResult
tryExpireCurrentLock(boolean fromShutdownHook) {
}
/**
- * Renews (heartbeats) the current lock if we are the holder, it forcefully
set
- * the expiration flag
- * to false and the lock expiration time to a later time in the future.
+ * Whether an earlier expire attempt that was answered with an error
actually landed: storage
+ * holds our expired lock, or another owner holds it while our lease has not
yet elapsed.
+ */
+ private boolean earlierExpireWriteLanded() {
+ final Pair<LockGetResult, Option<StorageLockFile>> current;
+ try {
+ current = storageLockClient.readCurrentLockFile();
+ } catch (RuntimeException e) {
+ // A failed reconcile read does not change the original precondition
failure. Let the
+ // normal ACQUIRED_BY_OTHERS handling clear our local lock and report
release failure.
+ logger.warn("Owner {}: Failed to reconcile lock expiration after a
retriable write error for {}.",
+ ownerId, lockFilePath, e);
+ return false;
+ }
+ if (current.getLeft() != LockGetResult.SUCCESS ||
!current.getRight().isPresent()) {
+ return false;
+ }
+ StorageLockFile stored = current.getRight().get();
+ if (ownerId.equals(stored.getOwner())) {
+ return stored.isExpired() && stored.getValidUntilMs() ==
getLock().getValidUntilMs();
+ }
+ // Others take over a live lease only after validUntil +
CLOCK_DRIFT_BUFFER_MS, so another
+ // owner holding it inside our lease means our expire landed and they
acquired after it.
+ return getCurrentEpochMs() < getLock().getValidUntilMs();
Review Comment:
🤖 This only holds if the other writer's clock is within
CLOCK_DRIFT_BUFFER_MS of ours. A writer whose clock runs ahead (the case the
ACQUIRED_BY_OTHERS log below calls out), or a lock file someone deleted, would
now be reported as a clean release with no acquired-by-others metric. Could
this branch log at WARN with the other owner and how much of our lease was
left, or count something, so a skew steal can still be diagnosed?
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
--
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]
