MarkGaox commented on code in PR #2698:
URL: https://github.com/apache/helix/pull/2698#discussion_r1470496226
##########
helix-lock/src/main/java/org/apache/helix/lock/helix/ZKDistributedNonblockingLock.java:
##########
@@ -312,7 +312,17 @@ public ZNRecord update(ZNRecord current) {
return _record;
}
- // higher priority lock request will try to preempt current lock owner
+ LockInfo unlockOrLockRequestLockInfo = new LockInfo(_record);
+ // Any unlock request from non-lock owners is blocked.
+ if
(unlockOrLockRequestLockInfo.getOwner().equals(LockConstants.DEFAULT_USER_ID)) {
Review Comment:
The behavior of current codebase is `requestor.unlock()` will return true
when it's priority is larger than the current lock owner. However, what
actually happened inside is the requestor didn't release the lock even if
unlock returns true, the current code just changed requestor lock's
pendingTimeOut and the requestor still has to call `tryLock()` to acquire the
lock, which is not a really efficient. Based on offline discussion with
@xyuanlu, the ideal behavior is that `unlock()` should return false when the
lock requestor is not the lock owner. And requestors should call `tryLock()` to
acquire the lock.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
