You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@helix.apache.org by GitBox <gi...@apache.org> on 2020/01/23 19:36:47 UTC

[GitHub] [helix] mgao0 opened a new pull request #703: Created Helix distributed lock design (apache#702)

mgao0 opened a new pull request #703: Created Helix distributed lock design (apache#702)
URL: https://github.com/apache/helix/pull/703
 
 
   ### Issues
   
   - [ ] My PR addresses the following Helix issues and references them in the PR description:
   
   #702 
   
   ### Description
   
   - [ ] Here are some details about my PR, including screenshots of any UI changes:
   
   Created a Helix distributed lock design with acquire, release, and getInfo functionalities. 
   
   ### Tests
   
   - [ ] The following tests are written for this issue:
   
   (List the names of added unit/integration tests)
   
   - [ ] The following is the result of the "mvn test" command on the appropriate module:
   
   (Copy & paste the result of "mvn test")
   
   ### Commits
   
   - [ ] My commits all reference appropriate Apache Helix GitHub issues in their subject lines, and I have squashed multiple commits if they address the same issue. In addition, my commits follow the guidelines from "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)":
     1. Subject is separated from body by a blank line
     1. Subject is limited to 50 characters (not including Jira issue reference)
     1. Subject does not end with a period
     1. Subject uses the imperative mood ("add", not "adding")
     1. Body wraps at 72 characters
     1. Body explains "what" and "why", not "how"
   
   ### Documentation
   
   - [ ] In case of new functionality, my PR adds documentation in the following wiki page:
   
   (Link the GitHub wiki you added)
   
   ### Code Quality
   
   - [ ] My diff has been formatted using helix-style.xml

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org


[GitHub] [helix] mgao0 commented on a change in pull request #703: Created Helix distributed lock interface

Posted by GitBox <gi...@apache.org>.
mgao0 commented on a change in pull request #703: Created Helix distributed lock interface
URL: https://github.com/apache/helix/pull/703#discussion_r370398855
 
 

 ##########
 File path: helix-lock/src/main/java/org/apache/helix/lock/HelixLock.java
 ##########
 @@ -0,0 +1,43 @@
+package org.apache.helix.lock;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/**
+ * Generic interface for Helix distributed lock
+ */
+public interface HelixLock {
+  /**
+   * Acquire a lock
+   * @return true if the lock was acquired, false if could not be acquired
+   */
+  public boolean acquireLock();
+
+  /**
+   * Release a lock
+   * @return true if the lock was released, false if it could not be released
+   */
+  public boolean releaseLock();
+
+  /**
+   * Retrieve the lock information, e.g. lock timeout, lock message, etc.
+   * @return lock information
+   */
+  public Object getLockInfo();
 
 Review comment:
   Good point. I'll change the return type to LockInfo, which could these fields for now: owner ID, expiring time, lock message.
   Since the lock info already have the owner ID, user can check if they are the lock owner or not by comparing their ID to the ID returned in LockInfo. If we want to make it easier for users, we can take in the user ID in the lock constructor and when they call isOwner() API, we compare the user ID with  the owner ID, and return true or false.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org


[GitHub] [helix] mgao0 commented on a change in pull request #703: Created Helix distributed lock interface

Posted by GitBox <gi...@apache.org>.
mgao0 commented on a change in pull request #703: Created Helix distributed lock interface
URL: https://github.com/apache/helix/pull/703#discussion_r373140743
 
 

 ##########
 File path: helix-lock/src/main/java/org/apache/helix/lock/LockInfo.java
 ##########
 @@ -0,0 +1,46 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.helix.lock;
+
+import java.util.Map;
+
+
+/**
+ * Generic interface for a map contains the Helix lock information
+ * @param <T> The type of the LockInfo value
+ */
+public interface LockInfo<T> {
 
 Review comment:
   We will add some mandatory fields to the LockInfo once we identify what are needed for both blocking lock and nonblocking 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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org


[GitHub] [helix] dasahcc commented on a change in pull request #703: Created Helix distributed lock interface

Posted by GitBox <gi...@apache.org>.
dasahcc commented on a change in pull request #703: Created Helix distributed lock interface
URL: https://github.com/apache/helix/pull/703#discussion_r371267034
 
 

 ##########
 File path: helix-lock/src/main/java/org/apache/helix/lock/LockInfo.java
 ##########
 @@ -0,0 +1,44 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.helix.lock;
+
+import java.util.Map;
+
+
+/**
+ * Generic interface for a map contains the Helix lock information
+ * @param <T> The type of the LockInfo value
+ */
+public interface LockInfo<T> {
+
+  /**
+   * Create a single filed of LockInfo, or update the value of the field if it already exists
+   * @param key the key of the LockInfo field
+   * @param value the value of the LockInfo field
+   */
+  void setValue(String key, T value);
 
 Review comment:
   the method name is too generic.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org


[GitHub] [helix] jiajunwang commented on a change in pull request #703: Created Helix distributed lock interface

Posted by GitBox <gi...@apache.org>.
jiajunwang commented on a change in pull request #703: Created Helix distributed lock interface
URL: https://github.com/apache/helix/pull/703#discussion_r370791178
 
 

 ##########
 File path: helix-lock/src/main/java/org/apache/helix/lock/HelixLock.java
 ##########
 @@ -0,0 +1,43 @@
+package org.apache.helix.lock;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/**
+ * Generic interface for Helix distributed lock
+ */
+public interface HelixLock {
+  /**
+   * Acquire a lock
+   * @return true if the lock was acquired, false if could not be acquired
+   */
+  public boolean acquireLock();
+
+  /**
+   * Release a lock
+   * @return true if the lock was released, false if it could not be released
 
 Review comment:
   What's the return if we release a new lock object?
   Or we try to lock a lock twice?

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org


[GitHub] [helix] jiajunwang commented on a change in pull request #703: Created Helix distributed lock interface

Posted by GitBox <gi...@apache.org>.
jiajunwang commented on a change in pull request #703: Created Helix distributed lock interface
URL: https://github.com/apache/helix/pull/703#discussion_r370790821
 
 

 ##########
 File path: helix-lock/src/main/java/org/apache/helix/lock/HelixLock.java
 ##########
 @@ -0,0 +1,43 @@
+package org.apache.helix.lock;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/**
+ * Generic interface for Helix distributed lock
+ */
+public interface HelixLock {
+  /**
+   * Acquire a lock
 
 Review comment:
   Let's mention if this call is blocking or not in the comment.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org


[GitHub] [helix] pkuwm commented on a change in pull request #703: Created Helix distributed lock interface

Posted by GitBox <gi...@apache.org>.
pkuwm commented on a change in pull request #703: Created Helix distributed lock interface
URL: https://github.com/apache/helix/pull/703#discussion_r370413323
 
 

 ##########
 File path: helix-lock/src/main/java/org/apache/helix/lock/HelixLock.java
 ##########
 @@ -0,0 +1,43 @@
+package org.apache.helix.lock;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/**
+ * Generic interface for Helix distributed lock
+ */
+public interface HelixLock {
+  /**
+   * Acquire a lock
+   * @return true if the lock was acquired, false if could not be acquired
+   */
+  public boolean acquireLock();
 
 Review comment:
   - Do we want `public` modifier here because the interface is already `public`? I don't think it is necessary. All abstract, default, and static methods in an interface are implicitly public, so you can omit the public modifier.
   - Do we really need to return a boolean? I think it makes sense to return a boolean for a `tryAcquire`, but for `acquire`, I think void is enough and clean.
   - How about just naming `acquire`
   - Do we want a `tryAcquire` with timeout?

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org


[GitHub] [helix] mgao0 commented on a change in pull request #703: Created Helix distributed lock interface

Posted by GitBox <gi...@apache.org>.
mgao0 commented on a change in pull request #703: Created Helix distributed lock interface
URL: https://github.com/apache/helix/pull/703#discussion_r371500470
 
 

 ##########
 File path: helix-lock/src/main/java/org/apache/helix/lock/HelixLock.java
 ##########
 @@ -0,0 +1,43 @@
+package org.apache.helix.lock;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/**
+ * Generic interface for Helix distributed lock
+ */
+public interface HelixLock {
+  /**
+   * Acquire a lock
 
 Review comment:
   @dasahcc Not sure I fully understand the scenario you described. Can you provide a specific example? What we want to achieve is to hide the implementation from our customers. So they choose to use nonblocking or blocking lock by using different HelixLock constructors, then they use the lock by calling `acquire` or `release`, we handle the difference of nonblocking and blocking in implementations.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org


[GitHub] [helix] mgao0 commented on a change in pull request #703: Created Helix distributed lock interface

Posted by GitBox <gi...@apache.org>.
mgao0 commented on a change in pull request #703: Created Helix distributed lock interface
URL: https://github.com/apache/helix/pull/703#discussion_r370887773
 
 

 ##########
 File path: helix-lock/src/main/java/org/apache/helix/lock/HelixLock.java
 ##########
 @@ -0,0 +1,43 @@
+package org.apache.helix.lock;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/**
+ * Generic interface for Helix distributed lock
+ */
+public interface HelixLock {
+  /**
+   * Acquire a lock
 
 Review comment:
   The interface is for both blocking or nonblocking calls. I added into the comment.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org


[GitHub] [helix] mgao0 commented on issue #703: Created Helix distributed lock interface

Posted by GitBox <gi...@apache.org>.
mgao0 commented on issue #703: Created Helix distributed lock interface
URL: https://github.com/apache/helix/pull/703#issuecomment-580411088
 
 
   This PR is ready to be merged, approved by @jiajunwang 

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org


[GitHub] [helix] mgao0 commented on a change in pull request #703: Created Helix distributed lock interface

Posted by GitBox <gi...@apache.org>.
mgao0 commented on a change in pull request #703: Created Helix distributed lock interface
URL: https://github.com/apache/helix/pull/703#discussion_r371501875
 
 

 ##########
 File path: helix-lock/src/main/java/org/apache/helix/lock/HelixLock.java
 ##########
 @@ -0,0 +1,53 @@
+package org.apache.helix.lock;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/**
+ * Generic interface for Helix distributed lock for both nonblocking and blocking calls
+ */
+public interface HelixLock {
+  /**
+   * Acquire a lock
+   * @return true if the lock was successfully acquired,
+   * false if the lock could not be acquired
+   */
+  boolean acquireLock();
+
+  /**
+   * Release a lock
+   * @return true if the lock was successfully released,
+   * false if the locked is not locked or is not locked by the user,
+   * or the lock could not be released
+   */
+  boolean releaseLock();
 
 Review comment:
   We do not consider privilege override at this moment. But it may be considered later.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org


[GitHub] [helix] jiajunwang commented on a change in pull request #703: Created Helix distributed lock interface

Posted by GitBox <gi...@apache.org>.
jiajunwang commented on a change in pull request #703: Created Helix distributed lock interface
URL: https://github.com/apache/helix/pull/703#discussion_r373114340
 
 

 ##########
 File path: helix-lock/src/main/java/org/apache/helix/lock/LockInfo.java
 ##########
 @@ -0,0 +1,46 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.helix.lock;
+
+import java.util.Map;
+
+
+/**
+ * Generic interface for a map contains the Helix lock information
+ * @param <T> The type of the LockInfo value
+ */
+public interface LockInfo<T> {
 
 Review comment:
   Optional for this PR. But do we consider some mandatory fields to be here? For example, timeout, owner ID, etc.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org


[GitHub] [helix] mgao0 commented on a change in pull request #703: Created Helix distributed lock interface

Posted by GitBox <gi...@apache.org>.
mgao0 commented on a change in pull request #703: Created Helix distributed lock interface
URL: https://github.com/apache/helix/pull/703#discussion_r373129071
 
 

 ##########
 File path: helix-lock/src/main/java/org/apache/helix/lock/HelixLock.java
 ##########
 @@ -0,0 +1,43 @@
+package org.apache.helix.lock;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/**
+ * Generic interface for Helix distributed lock
+ */
+public interface HelixLock {
+  /**
+   * Acquire a lock
 
 Review comment:
   Currently this interface only includes blocking calls. Nonblocking calls can be added later if there is a need from our customers.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org


[GitHub] [helix] lei-xia commented on a change in pull request #703: Created Helix distributed lock interface (apache#702)

Posted by GitBox <gi...@apache.org>.
lei-xia commented on a change in pull request #703: Created Helix distributed lock interface (apache#702)
URL: https://github.com/apache/helix/pull/703#discussion_r370320137
 
 

 ##########
 File path: helix-lock/src/main/java/org/apache/helix/lock/HelixLock.java
 ##########
 @@ -0,0 +1,43 @@
+package org.apache.helix.lock;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/**
+ * Generic interface for Helix distributed lock
+ */
+public interface HelixLock {
+  /**
+   * Acquire a lock
+   * @return true if the lock was acquired, false if could not be acquired
+   */
+  public boolean acquireLock();
+
+  /**
+   * Release a lock
+   * @return true if the lock was released, false if it could not be released
+   */
+  public boolean releaseLock();
+
+  /**
+   * Retrieve the lock information, e.g. lock timeout, lock message, etc.
+   * @return lock information
+   */
+  public Object getLockInfo();
 
 Review comment:
   should this getLockInfo return a defined type (for example, LockInfo)?    Also, how does a piece of code check whether it owns the lock or not?

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org


[GitHub] [helix] dasahcc commented on a change in pull request #703: Created Helix distributed lock interface

Posted by GitBox <gi...@apache.org>.
dasahcc commented on a change in pull request #703: Created Helix distributed lock interface
URL: https://github.com/apache/helix/pull/703#discussion_r371265748
 
 

 ##########
 File path: helix-lock/src/main/java/org/apache/helix/lock/HelixLock.java
 ##########
 @@ -0,0 +1,43 @@
+package org.apache.helix.lock;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/**
+ * Generic interface for Helix distributed lock
+ */
+public interface HelixLock {
+  /**
+   * Acquire a lock
 
 Review comment:
   If we design the method for both types, it may end up with only one implementation at a time. What if user may have both use cases for same lock object?

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org


[GitHub] [helix] jiajunwang commented on issue #703: Created Helix distributed lock interface

Posted by GitBox <gi...@apache.org>.
jiajunwang commented on issue #703: Created Helix distributed lock interface
URL: https://github.com/apache/helix/pull/703#issuecomment-580387263
 
 
   Please ensure you addressed the other's comments as well.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org


[GitHub] [helix] mgao0 commented on a change in pull request #703: Created Helix distributed lock interface

Posted by GitBox <gi...@apache.org>.
mgao0 commented on a change in pull request #703: Created Helix distributed lock interface
URL: https://github.com/apache/helix/pull/703#discussion_r370887773
 
 

 ##########
 File path: helix-lock/src/main/java/org/apache/helix/lock/HelixLock.java
 ##########
 @@ -0,0 +1,43 @@
+package org.apache.helix.lock;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/**
+ * Generic interface for Helix distributed lock
+ */
+public interface HelixLock {
+  /**
+   * Acquire a lock
 
 Review comment:
   The interface is for both blocking or nonblocking calls. I added into the comment.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org


[GitHub] [helix] mgao0 commented on a change in pull request #703: Created Helix distributed lock interface

Posted by GitBox <gi...@apache.org>.
mgao0 commented on a change in pull request #703: Created Helix distributed lock interface
URL: https://github.com/apache/helix/pull/703#discussion_r370888049
 
 

 ##########
 File path: helix-lock/src/main/java/org/apache/helix/lock/HelixLock.java
 ##########
 @@ -0,0 +1,43 @@
+package org.apache.helix.lock;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/**
+ * Generic interface for Helix distributed lock
+ */
+public interface HelixLock {
+  /**
+   * Acquire a lock
+   * @return true if the lock was acquired, false if could not be acquired
+   */
+  public boolean acquireLock();
+
+  /**
+   * Release a lock
+   * @return true if the lock was released, false if it could not be released
 
 Review comment:
   False will be returned. I added it into the method comment.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org


[GitHub] [helix] zhangmeng916 commented on a change in pull request #703: Created Helix distributed lock interface

Posted by GitBox <gi...@apache.org>.
zhangmeng916 commented on a change in pull request #703: Created Helix distributed lock interface
URL: https://github.com/apache/helix/pull/703#discussion_r370767302
 
 

 ##########
 File path: helix-lock/src/main/java/org/apache/helix/lock/HelixLock.java
 ##########
 @@ -0,0 +1,43 @@
+package org.apache.helix.lock;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/**
+ * Generic interface for Helix distributed lock
+ */
+public interface HelixLock {
+  /**
+   * Acquire a lock
+   * @return true if the lock was acquired, false if could not be acquired
+   */
+  public boolean acquireLock();
+
+  /**
+   * Release a lock
+   * @return true if the lock was released, false if it could not be released
+   */
+  public boolean releaseLock();
+
+  /**
+   * Retrieve the lock information, e.g. lock timeout, lock message, etc.
+   * @return lock information
+   */
+  public Object getLockInfo();
 
 Review comment:
   Please try to make LockInfo very generic, as we will have various implementations, and we cannot force all of them to have certain fields. 

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org


[GitHub] [helix] jiajunwang commented on a change in pull request #703: Created Helix distributed lock interface

Posted by GitBox <gi...@apache.org>.
jiajunwang commented on a change in pull request #703: Created Helix distributed lock interface
URL: https://github.com/apache/helix/pull/703#discussion_r370791642
 
 

 ##########
 File path: helix-lock/src/main/java/org/apache/helix/lock/HelixLock.java
 ##########
 @@ -0,0 +1,43 @@
+package org.apache.helix.lock;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/**
+ * Generic interface for Helix distributed lock
+ */
+public interface HelixLock {
+  /**
+   * Acquire a lock
+   * @return true if the lock was acquired, false if could not be acquired
+   */
+  public boolean acquireLock();
+
+  /**
+   * Release a lock
+   * @return true if the lock was released, false if it could not be released
+   */
+  public boolean releaseLock();
+
+  /**
+   * Retrieve the lock information, e.g. lock timeout, lock message, etc.
+   * @return lock information
+   */
+  public Object getLockInfo();
 
 Review comment:
   This should be part of the API design. Please either create that class with full Java doc, or upload the design doc to github wiki.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org


[GitHub] [helix] mgao0 commented on a change in pull request #703: Created Helix distributed lock interface

Posted by GitBox <gi...@apache.org>.
mgao0 commented on a change in pull request #703: Created Helix distributed lock interface
URL: https://github.com/apache/helix/pull/703#discussion_r370398855
 
 

 ##########
 File path: helix-lock/src/main/java/org/apache/helix/lock/HelixLock.java
 ##########
 @@ -0,0 +1,43 @@
+package org.apache.helix.lock;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/**
+ * Generic interface for Helix distributed lock
+ */
+public interface HelixLock {
+  /**
+   * Acquire a lock
+   * @return true if the lock was acquired, false if could not be acquired
+   */
+  public boolean acquireLock();
+
+  /**
+   * Release a lock
+   * @return true if the lock was released, false if it could not be released
+   */
+  public boolean releaseLock();
+
+  /**
+   * Retrieve the lock information, e.g. lock timeout, lock message, etc.
+   * @return lock information
+   */
+  public Object getLockInfo();
 
 Review comment:
   Good point. I'll change the return type to LockInfo, which contains these fields for now: owner ID, expiring time, lock message.
   Since the lock info already have the owner ID, user can check if they are the lock owner or not by comparing their ID to the ID returned in LockInfo. If we want to make it easier for users, we can take in the user ID in the lock constructor and when they call isOwner() API, we compare the user ID with  the owner ID, and return true or false.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org


[GitHub] [helix] mgao0 commented on a change in pull request #703: Created Helix distributed lock interface

Posted by GitBox <gi...@apache.org>.
mgao0 commented on a change in pull request #703: Created Helix distributed lock interface
URL: https://github.com/apache/helix/pull/703#discussion_r373131051
 
 

 ##########
 File path: helix-lock/src/main/java/org/apache/helix/lock/LockInfo.java
 ##########
 @@ -0,0 +1,44 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.helix.lock;
+
+import java.util.Map;
+
+
+/**
+ * Generic interface for a map contains the Helix lock information
+ * @param <T> The type of the LockInfo value
+ */
+public interface LockInfo<T> {
+
+  /**
+   * Create a single filed of LockInfo, or update the value of the field if it already exists
+   * @param key the key of the LockInfo field
+   * @param value the value of the LockInfo field
+   */
+  void setValue(String key, T value);
 
 Review comment:
   I added a TODO to add specific setter and getter for fields that we identify that are mandatory as LockInfo, so the method names can make more sense to our customers.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org


[GitHub] [helix] mgao0 commented on a change in pull request #703: Created Helix distributed lock interface

Posted by GitBox <gi...@apache.org>.
mgao0 commented on a change in pull request #703: Created Helix distributed lock interface
URL: https://github.com/apache/helix/pull/703#discussion_r373131968
 
 

 ##########
 File path: helix-lock/src/main/java/org/apache/helix/lock/HelixLock.java
 ##########
 @@ -0,0 +1,43 @@
+package org.apache.helix.lock;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/**
+ * Generic interface for Helix distributed lock
+ */
+public interface HelixLock {
+  /**
+   * Acquire a lock
+   * @return true if the lock was acquired, false if could not be acquired
+   */
+  public boolean acquireLock();
+
+  /**
+   * Release a lock
+   * @return true if the lock was released, false if it could not be released
+   */
+  public boolean releaseLock();
+
+  /**
+   * Retrieve the lock information, e.g. lock timeout, lock message, etc.
+   * @return lock information
+   */
+  public Object getLockInfo();
 
 Review comment:
   Added LockInfo interface as the type contains lock metadata. And I also added a isOwner method for customers to check if they are the owner of 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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org


[GitHub] [helix] jiajunwang merged pull request #703: Created Helix distributed lock interface

Posted by GitBox <gi...@apache.org>.
jiajunwang merged pull request #703: Created Helix distributed lock interface
URL: https://github.com/apache/helix/pull/703
 
 
   

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org


[GitHub] [helix] mgao0 commented on a change in pull request #703: Created Helix distributed lock interface

Posted by GitBox <gi...@apache.org>.
mgao0 commented on a change in pull request #703: Created Helix distributed lock interface
URL: https://github.com/apache/helix/pull/703#discussion_r370780812
 
 

 ##########
 File path: helix-lock/src/main/java/org/apache/helix/lock/HelixLock.java
 ##########
 @@ -0,0 +1,43 @@
+package org.apache.helix.lock;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/**
+ * Generic interface for Helix distributed lock
+ */
+public interface HelixLock {
+  /**
+   * Acquire a lock
+   * @return true if the lock was acquired, false if could not be acquired
+   */
+  public boolean acquireLock();
 
 Review comment:
   - Good catch, will remove `public`.
   
   - This interface is a generic interface for both blocking and nonblocking blocks. In the design, if a false is return for `acquireLock`, it means the user failed to acquire the lock; a returned true means the lock is successfully acquired. A user cannot work on the resource without holding the lock.
   
   - I personally think `acquireLock` is more clear that the user is using a lock, even if the variable name does not show it's a lock
   
   - For blocking lock, we will not expose the `tryAcquire` to our users, `tryAcquire` and `retry` will be wrapped inside the `acquireLock` implementation.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org


[GitHub] [helix] dasahcc commented on a change in pull request #703: Created Helix distributed lock interface

Posted by GitBox <gi...@apache.org>.
dasahcc commented on a change in pull request #703: Created Helix distributed lock interface
URL: https://github.com/apache/helix/pull/703#discussion_r371266774
 
 

 ##########
 File path: helix-lock/src/main/java/org/apache/helix/lock/HelixLock.java
 ##########
 @@ -0,0 +1,53 @@
+package org.apache.helix.lock;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/**
+ * Generic interface for Helix distributed lock for both nonblocking and blocking calls
+ */
+public interface HelixLock {
+  /**
+   * Acquire a lock
+   * @return true if the lock was successfully acquired,
+   * false if the lock could not be acquired
+   */
+  boolean acquireLock();
+
+  /**
+   * Release a lock
+   * @return true if the lock was successfully released,
+   * false if the locked is not locked or is not locked by the user,
+   * or the lock could not be released
+   */
+  boolean releaseLock();
 
 Review comment:
   Do we consider privilege override or something else? What is the scope of locking

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org