mgao0 commented on a change in pull request #1077:
URL: https://github.com/apache/helix/pull/1077#discussion_r437773999



##########
File path: 
helix-lock/src/main/java/org/apache/helix/lock/helix/ZKDistributedNonblockingLock.java
##########
@@ -115,6 +113,11 @@ public boolean isCurrentOwner() {
         .getTimeout());
   }
 
+  @Override
+  public void close() {
+    _baseDataAccessor.close();

Review comment:
       Discussed offline. Won't unlock the lock inside close(), but will check 
to make sure the lock is unlocked when the user calls close().

##########
File path: helix-lock/src/main/java/org/apache/helix/lock/DistributedLock.java
##########
@@ -49,4 +49,9 @@
    * false if the user is not the lock owner or the lock doesn't have a owner
    */
   boolean isCurrentOwner();
+
+  /**
+   * Close the lock and release resources
+   */
+  void close();

Review comment:
       Discussed offline, for now we will let user to explicitly call the close 
method, since we cannot determine all the use cases where we should close the 
lock right now. After we have a clearer picture of all the features, we can try 
to do this for our users.

##########
File path: 
helix-lock/src/main/java/org/apache/helix/lock/helix/ZKDistributedNonblockingLock.java
##########
@@ -54,8 +53,8 @@
    * @param lockMsg the reason for having this lock

Review comment:
       Updated. Thanks for checking!

##########
File path: 
helix-lock/src/test/java/org/apache/helix/lock/helix/TestZKHelixNonblockingLock.java
##########
@@ -71,9 +73,14 @@ public void beforeMethod() {
     Assert.assertFalse(_gZkClient.exists(_lockPath));
   }
 
+  @AfterSuite
+  public void afterSuite() throws IOException {
+    _lock.close();

Review comment:
       I added two tests. One is trying to close the lock when it's unlocked, 
and one is trying to close it when it's locked.




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

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