hudi-agent commented on code in PR #18439:
URL: https://github.com/apache/hudi/pull/18439#discussion_r3183885018
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/transaction/lock/StorageBasedLockProvider.java:
##########
@@ -424,22 +436,48 @@ private boolean believesLockMightBeHeld() {
* Unlock by marking our current lock file "expired": true.
*/
@Override
- public synchronized void unlock() {
- assertHeartbeatManagerExists();
- if (!believesLockMightBeHeld()) {
- return;
+ public void unlock() {
+ ExpireLockResult expireResult;
+ synchronized (this) {
+ assertHeartbeatManagerExists();
+ if (!believesLockMightBeHeld()) {
+ return;
+ }
+
+ // Try to stop the heartbeat first
+ if (heartbeatManager.hasActiveHeartbeat()) {
+ logger.debug("Owner {}: Gracefully shutting down heartbeat.", ownerId);
+ if (!heartbeatManager.stopHeartbeat(true)) {
+
hoodieLockMetrics.ifPresent(HoodieLockMetrics::updateLockReleaseFailureMetric);
+ throw new
HoodieLockException(generateLockStateMessage(FAILED_TO_RELEASE));
+ }
+ }
+
+ // Then expire the current lock.
+ expireResult = tryExpireCurrentLock(false);
}
- boolean believesNoLongerHoldsLock = true;
- // Try to stop the heartbeat first
- if (heartbeatManager.hasActiveHeartbeat()) {
- logger.debug("Owner {}: Gracefully shutting down heartbeat.", ownerId);
- believesNoLongerHoldsLock &= heartbeatManager.stopHeartbeat(true);
+ // If throttled, retry once after sleeping outside the monitor to avoid
blocking other threads.
+ // Note: when unlock() is called via close() -> shutdown(), the caller's
synchronized methods
+ // hold the monitor via reentrant locking, so the sleep will still block
in that path.
+ // This is acceptable since close() is a shutdown path, not the hot path.
+ if (expireResult == ExpireLockResult.THROTTLED) {
+ logger.warn("Owner {}: Lock expiration was throttled, retrying after 1
second.", ownerId);
+ try {
+ TimeUnit.SECONDS.sleep(1);
+ } catch (InterruptedException ie) {
Review Comment:
🤖 nit: the `1` here is a magic number — could you extract it as a named
constant (e.g. `THROTTLE_RETRY_DELAY_SECONDS`)? That way the retry delay lives
in one place rather than being split between this call and the log message
string above.
<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]
