MarkGaox commented on code in PR #2698:
URL: https://github.com/apache/helix/pull/2698#discussion_r1398535640


##########
helix-lock/src/main/java/org/apache/helix/lock/helix/ZKDistributedNonblockingLock.java:
##########
@@ -312,16 +312,20 @@ public ZNRecord update(ZNRecord current) {
         return _record;
       }
 
-      // higher priority lock request will try to  preempt current lock owner
-      if (!isCurrentOwner(curLockInfo) && _priority > 
curLockInfo.getPriority()) {
+      LockInfo unlockOrLockRequestLockInfo = new LockInfo(_record);
+      int unlockOrLockRequestPriority = 
unlockOrLockRequestLockInfo.getPriority();
+      // higher priority lock request will try to  preempt current lock owner. 
And any unlock request
+      // from non-lock owners will always be blocked because the default 
priority of unlock request
+      // is 0. And it would always be less than or equal to the priority value 
of the current lock.
+      if (!isCurrentOwner(curLockInfo) && unlockOrLockRequestPriority > 
curLockInfo.getPriority()) {

Review Comment:
   Make sense. Added a check that throws exception if the user is not the lock 
owner with descriptive messages.



##########
helix-lock/src/test/java/org/apache/helix/lock/helix/TestZKHelixNonblockingLock.java:
##########
@@ -141,6 +141,23 @@ public void testAcquireLockWhenExistingLockExpired() {
     Assert.assertFalse(_lock.isCurrentOwner());
   }
 
+  @Test
+  public void testOtherClientsAcquireLockFromExistingLockExpired() {
+    // Fake condition when the lock owner is not current user
+    String fakeUserID = UUID.randomUUID().toString();
+    ZNRecord fakeRecord = new ZNRecord(fakeUserID);
+    fakeRecord.setSimpleField(LockInfo.LockInfoAttribute.OWNER.name(), 
fakeUserID);
+    fakeRecord
+        .setSimpleField(LockInfo.LockInfoAttribute.TIMEOUT.name(), 
String.valueOf(Long.MAX_VALUE));
+    fakeRecord.setIntField(LockInfo.LockInfoAttribute.PRIORITY.name(), 0);

Review Comment:
   Thanks. Added a new test down below.



-- 
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]

Reply via email to