[GitHub] [helix] mgao0 commented on a change in pull request #718: Implement Helix nonblocking lock
mgao0 commented on a change in pull request #718: Implement Helix nonblocking
lock
URL: https://github.com/apache/helix/pull/718#discussion_r381673956
##
File path:
helix-lock/src/main/java/org/apache/helix/lock/helix/ZKHelixNonblockingLock.java
##
@@ -0,0 +1,145 @@
+/*
+ * 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.helix;
+
+import java.util.Date;
+
+import org.I0Itec.zkclient.DataUpdater;
+import org.apache.helix.AccessOption;
+import org.apache.helix.BaseDataAccessor;
+import org.apache.helix.HelixException;
+import org.apache.helix.ZNRecord;
+import org.apache.helix.lock.HelixLock;
+import org.apache.helix.lock.LockInfo;
+import org.apache.helix.lock.LockScope;
+import org.apache.helix.manager.zk.ZkBaseDataAccessor;
+import org.apache.log4j.Logger;
+
+
+/**
+ * Helix nonblocking lock implementation based on Zookeeper
+ */
+public class ZKHelixNonblockingLock implements HelixLock {
+
+ private static final Logger LOG =
Logger.getLogger(ZKHelixNonblockingLock.class);
+
+ private final String _lockPath;
+ private final String _userId;
+ private final long _timeout;
+ private final String _lockMsg;
+ private final BaseDataAccessor _baseDataAccessor;
+
+ /**
+ * Initialize the lock with user provided information, e.g.,cluster, scope,
etc.
+ * @param scope the scope to lock
+ * @param zkAddress the zk address the cluster connects to
+ * @param timeout the timeout period of the lcok
+ * @param lockMsg the reason for having this lock
+ * @param userId a universal unique userId for lock owner identity
+ */
+ public ZKHelixNonblockingLock(LockScope scope, String zkAddress, Long
timeout, String lockMsg,
+ String userId) {
+this(scope.getPath(), zkAddress, timeout, lockMsg, userId);
+ }
+
+ /**
+ * Initialize the lock with user provided information, e.g., lock path under
zookeeper, etc.
+ * @param lockPath the path of the lock under Zookeeper
+ * @param zkAddress the zk address of the cluster
+ * @param timeout the timeout period of the lcok
+ * @param lockMsg the reason for having this lock
+ * @param userId a universal unique userId for lock owner identity
+ */
+ private ZKHelixNonblockingLock(String lockPath, String zkAddress, Long
timeout, String lockMsg,
+ String userId) {
+_lockPath = lockPath;
+if (timeout < 0) {
+ throw new IllegalArgumentException("The expiration time cannot be
negative.");
+}
+_timeout = timeout;
+_lockMsg = lockMsg;
+_userId = userId;
+_baseDataAccessor = new ZkBaseDataAccessor<>(zkAddress);
+ }
+
+ @Override
+ public boolean acquireLock() {
+
+// Set lock information fields
+long deadline;
+// Prevent value overflow
+if (_timeout > Long.MAX_VALUE - System.currentTimeMillis()) {
+ deadline = Long.MAX_VALUE;
+} else {
+ deadline = System.currentTimeMillis() + _timeout;
+}
+LockUpdater updater = new LockUpdater(new LockInfo(_userId, _lockMsg,
deadline));
+return _baseDataAccessor.update(_lockPath, updater,
AccessOption.PERSISTENT);
+ }
+
+ @Override
+ public boolean releaseLock() {
+// Initialize the lock updater with a default lock info represents the
state of a unlocked lock
+LockUpdater updater = new LockUpdater(LockInfo.defaultLockInfo);
+return _baseDataAccessor.update(_lockPath, updater,
AccessOption.PERSISTENT);
Review comment:
Keeping this way for now. Added TODO to change it in the future.
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:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: reviews-unsubscr...@helix.apache.org
For additional commands, e-mail: reviews-h...@helix.apache.org
[GitHub] [helix] mgao0 commented on a change in pull request #718: Implement Helix nonblocking lock
mgao0 commented on a change in pull request #718: Implement Helix nonblocking
lock
URL: https://github.com/apache/helix/pull/718#discussion_r380964332
##
File path: helix-lock/src/main/java/org/apache/helix/lock/LockInfo.java
##
@@ -19,28 +19,109 @@
package org.apache.helix.lock;
-import java.util.Map;
+import org.apache.helix.HelixProperty;
+import org.apache.helix.ZNRecord;
/**
- * Generic interface for a map contains the Helix lock information
- * @param The type of the LockInfo value
+ * Structure represents a lock node information, implemented using ZNRecord
*/
-public interface LockInfo {
+public class LockInfo extends HelixProperty {
Review comment:
In this case I can remove the "extends HelixProperty" to make it independent
from Helix. The LockInfo is barely a ZNRecord, and currently I'm not using any
properties and methods exclusively provided by HelixProperty.
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:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: reviews-unsubscr...@helix.apache.org
For additional commands, e-mail: reviews-h...@helix.apache.org
[GitHub] [helix] mgao0 commented on a change in pull request #718: Implement Helix nonblocking lock
mgao0 commented on a change in pull request #718: Implement Helix nonblocking
lock
URL: https://github.com/apache/helix/pull/718#discussion_r380962857
##
File path:
helix-lock/src/main/java/org/apache/helix/lock/helix/ZKHelixNonblockingLock.java
##
@@ -0,0 +1,145 @@
+/*
+ * 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.helix;
+
+import java.util.Date;
+
+import org.I0Itec.zkclient.DataUpdater;
+import org.apache.helix.AccessOption;
+import org.apache.helix.BaseDataAccessor;
+import org.apache.helix.HelixException;
+import org.apache.helix.ZNRecord;
+import org.apache.helix.lock.HelixLock;
+import org.apache.helix.lock.LockInfo;
+import org.apache.helix.lock.LockScope;
+import org.apache.helix.manager.zk.ZkBaseDataAccessor;
+import org.apache.log4j.Logger;
+
+
+/**
+ * Helix nonblocking lock implementation based on Zookeeper
+ */
+public class ZKHelixNonblockingLock implements HelixLock {
+
+ private static final Logger LOG =
Logger.getLogger(ZKHelixNonblockingLock.class);
+
+ private final String _lockPath;
+ private final String _userId;
+ private final long _timeout;
+ private final String _lockMsg;
+ private final BaseDataAccessor _baseDataAccessor;
+
+ /**
+ * Initialize the lock with user provided information, e.g.,cluster, scope,
etc.
+ * @param scope the scope to lock
+ * @param zkAddress the zk address the cluster connects to
+ * @param timeout the timeout period of the lcok
+ * @param lockMsg the reason for having this lock
+ * @param userId a universal unique userId for lock owner identity
+ */
+ public ZKHelixNonblockingLock(LockScope scope, String zkAddress, Long
timeout, String lockMsg,
+ String userId) {
+this(scope.getPath(), zkAddress, timeout, lockMsg, userId);
+ }
+
+ /**
+ * Initialize the lock with user provided information, e.g., lock path under
zookeeper, etc.
+ * @param lockPath the path of the lock under Zookeeper
+ * @param zkAddress the zk address of the cluster
+ * @param timeout the timeout period of the lcok
+ * @param lockMsg the reason for having this lock
+ * @param userId a universal unique userId for lock owner identity
+ */
+ private ZKHelixNonblockingLock(String lockPath, String zkAddress, Long
timeout, String lockMsg,
+ String userId) {
+_lockPath = lockPath;
+if (timeout < 0) {
+ throw new IllegalArgumentException("The expiration time cannot be
negative.");
+}
+_timeout = timeout;
+_lockMsg = lockMsg;
+_userId = userId;
+_baseDataAccessor = new ZkBaseDataAccessor<>(zkAddress);
+ }
+
+ @Override
+ public boolean acquireLock() {
+
+// Set lock information fields
+long deadline;
+// Prevent value overflow
+if (_timeout > Long.MAX_VALUE - System.currentTimeMillis()) {
+ deadline = Long.MAX_VALUE;
+} else {
+ deadline = System.currentTimeMillis() + _timeout;
+}
+LockUpdater updater = new LockUpdater(new LockInfo(_userId, _lockMsg,
deadline));
+return _baseDataAccessor.update(_lockPath, updater,
AccessOption.PERSISTENT);
+ }
+
+ @Override
+ public boolean releaseLock() {
+// Initialize the lock updater with a default lock info represents the
state of a unlocked lock
+LockUpdater updater = new LockUpdater(LockInfo.defaultLockInfo);
+return _baseDataAccessor.update(_lockPath, updater,
AccessOption.PERSISTENT);
Review comment:
Directly deleting it can cause race conditions. For example, when we call
delete to delete lock node A, but during the process someone else acquired the
lock or the current lock owner updated the lock, so currently it's lock node B,
as a result the delete call deletes lock node B instead of lock node A, and
both the caller and the lock owner wouldn't be aware of this mistake. The
update method checks the version of the znode, so it can prevent this situation.
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:
us...@infra.apache.org
[GitHub] [helix] mgao0 commented on a change in pull request #718: Implement Helix nonblocking lock
mgao0 commented on a change in pull request #718: Implement Helix nonblocking
lock
URL: https://github.com/apache/helix/pull/718#discussion_r380960097
##
File path:
helix-lock/src/main/java/org/apache/helix/lock/helix/ZKHelixNonblockingLock.java
##
@@ -0,0 +1,145 @@
+/*
+ * 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.helix;
+
+import java.util.Date;
+
+import org.I0Itec.zkclient.DataUpdater;
+import org.apache.helix.AccessOption;
+import org.apache.helix.BaseDataAccessor;
+import org.apache.helix.HelixException;
+import org.apache.helix.ZNRecord;
+import org.apache.helix.lock.HelixLock;
+import org.apache.helix.lock.LockInfo;
+import org.apache.helix.lock.LockScope;
+import org.apache.helix.manager.zk.ZkBaseDataAccessor;
+import org.apache.log4j.Logger;
+
+
+/**
+ * Helix nonblocking lock implementation based on Zookeeper
+ */
+public class ZKHelixNonblockingLock implements HelixLock {
+
+ private static final Logger LOG =
Logger.getLogger(ZKHelixNonblockingLock.class);
+
+ private final String _lockPath;
+ private final String _userId;
+ private final long _timeout;
+ private final String _lockMsg;
+ private final BaseDataAccessor _baseDataAccessor;
+
+ /**
+ * Initialize the lock with user provided information, e.g.,cluster, scope,
etc.
+ * @param scope the scope to lock
+ * @param zkAddress the zk address the cluster connects to
+ * @param timeout the timeout period of the lcok
+ * @param lockMsg the reason for having this lock
+ * @param userId a universal unique userId for lock owner identity
+ */
+ public ZKHelixNonblockingLock(LockScope scope, String zkAddress, Long
timeout, String lockMsg,
+ String userId) {
+this(scope.getPath(), zkAddress, timeout, lockMsg, userId);
+ }
+
+ /**
+ * Initialize the lock with user provided information, e.g., lock path under
zookeeper, etc.
+ * @param lockPath the path of the lock under Zookeeper
+ * @param zkAddress the zk address of the cluster
+ * @param timeout the timeout period of the lcok
+ * @param lockMsg the reason for having this lock
+ * @param userId a universal unique userId for lock owner identity
+ */
+ private ZKHelixNonblockingLock(String lockPath, String zkAddress, Long
timeout, String lockMsg,
+ String userId) {
+_lockPath = lockPath;
+if (timeout < 0) {
+ throw new IllegalArgumentException("The expiration time cannot be
negative.");
+}
+_timeout = timeout;
+_lockMsg = lockMsg;
+_userId = userId;
+_baseDataAccessor = new ZkBaseDataAccessor<>(zkAddress);
+ }
+
+ @Override
+ public boolean acquireLock() {
+
+// Set lock information fields
+long deadline;
+// Prevent value overflow
+if (_timeout > Long.MAX_VALUE - System.currentTimeMillis()) {
+ deadline = Long.MAX_VALUE;
+} else {
+ deadline = System.currentTimeMillis() + _timeout;
+}
+LockUpdater updater = new LockUpdater(new LockInfo(_userId, _lockMsg,
deadline));
+return _baseDataAccessor.update(_lockPath, updater,
AccessOption.PERSISTENT);
+ }
+
+ @Override
+ public boolean releaseLock() {
+// Initialize the lock updater with a default lock info represents the
state of a unlocked lock
+LockUpdater updater = new LockUpdater(LockInfo.defaultLockInfo);
+return _baseDataAccessor.update(_lockPath, updater,
AccessOption.PERSISTENT);
+ }
+
+ @Override
+ public LockInfo getCurrentLockInfo() {
+ZNRecord curLockInfo = _baseDataAccessor.get(_lockPath, null,
AccessOption.PERSISTENT);
+return new LockInfo(curLockInfo);
Review comment:
Data accessor returns a null, and when converting it to the LockInfo, a
LockInfo instance with all the fields filled with default values (which is also
the value we use to represent an unlocked lock node) will be returned.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [helix] mgao0 commented on a change in pull request #718: Implement Helix nonblocking lock
mgao0 commented on a change in pull request #718: Implement Helix nonblocking
lock
URL: https://github.com/apache/helix/pull/718#discussion_r379712887
##
File path:
helix-lock/src/main/java/org/apache/helix/lock/helix/HelixLockScope.java
##
@@ -0,0 +1,183 @@
+/*
+ * 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.helix;
+
+import java.util.List;
+
+import org.apache.helix.lock.LockScope;
+import org.apache.helix.util.StringTemplate;
+
+
+/**
+ * Defines the various scopes of Helix locks, and how they are represented on
Zookeeper
+ */
+public class HelixLockScope implements LockScope {
+
+ /**
+ * Define various properties of Helix lock, and associate them with the
number of arguments required for getting znode path
+ */
+ public enum LockScopeProperty {
+
+CLUSTER(1),
+
+PARTICIPANT(2),
+
+RESOURCE(3),
+
+PARTITION(4);
+
+//the number of arguments required to generate a full path for the
specific scope
+final int _pathArgNum;
+
+/**
+ * Initialize a LockScopeProperty
+ * @param pathArgNum the number of arguments required to generate a full
path for the specific scope
+\ */
+private LockScopeProperty(int pathArgNum) {
+ _pathArgNum = pathArgNum;
+}
+
+/**
+ * Get the number of template arguments required to generate a full path
+ * @return number of template arguments in the path
+ */
+public int getPathArgNum() {
+ return _pathArgNum;
+}
+
+/**
+ * Get the position of this argument from the input that used to generate
the scope
+ * @return the number of position of value for this property in the list
of keys input
+ */
+public int getArgPos() {
+ return _pathArgNum - 1;
+}
+ }
+
+ /**
+ * string templates to generate znode path
+ */
+ private static final StringTemplate template = new StringTemplate();
+
+ static {
+template.addEntry(LockScopeProperty.CLUSTER, 1, "/{clusterName}/LOCK");
Review comment:
Changed it to the same format as path for cluster in HelixConfigScope,
should not have conflicts now.
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:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: reviews-unsubscr...@helix.apache.org
For additional commands, e-mail: reviews-h...@helix.apache.org
[GitHub] [helix] mgao0 commented on a change in pull request #718: Implement Helix nonblocking lock
mgao0 commented on a change in pull request #718: Implement Helix nonblocking
lock
URL: https://github.com/apache/helix/pull/718#discussion_r379712825
##
File path:
helix-lock/src/main/java/org/apache/helix/lock/helix/ZKHelixNonblockingLock.java
##
@@ -0,0 +1,186 @@
+/*
+ * 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.helix;
+
+import java.util.Date;
+
+import org.I0Itec.zkclient.DataUpdater;
+import org.apache.helix.AccessOption;
+import org.apache.helix.BaseDataAccessor;
+import org.apache.helix.HelixException;
+import org.apache.helix.ZNRecord;
+import org.apache.helix.lock.HelixLock;
+import org.apache.helix.lock.LockInfo;
+import org.apache.helix.lock.LockScope;
+import org.apache.helix.manager.zk.ZkBaseDataAccessor;
+import org.apache.log4j.Logger;
+
+
+/**
+ * Helix nonblocking lock implementation based on Zookeeper
+ */
+public class ZKHelixNonblockingLock implements HelixLock {
+
+ private static final Logger LOG =
Logger.getLogger(ZKHelixNonblockingLock.class);
+
+ private final String _lockPath;
+ private final String _userId;
+ private final long _timeout;
+ private final String _lockMsg;
+ private final BaseDataAccessor _baseDataAccessor;
+
+ /**
+ * Initialize the lock with user provided information, e.g.,cluster, scope,
etc.
+ * @param scope the scope to lock
+ * @param zkAddress the zk address the cluster connects to
+ * @param timeout the timeout period of the lcok
+ * @param lockMsg the reason for having this lock
+ * @param userId a universal unique userId for lock owner identity
+ */
+ public ZKHelixNonblockingLock(LockScope scope, String zkAddress, Long
timeout, String lockMsg,
+ String userId) {
+this(scope.getPath(), zkAddress, timeout, lockMsg, userId);
+ }
+
+ /**
+ * Initialize the lock with user provided information, e.g., lock path under
zookeeper, etc.
+ * @param lockPath the path of the lock under Zookeeper
+ * @param zkAddress the zk address of the cluster
+ * @param timeout the timeout period of the lcok
+ * @param lockMsg the reason for having this lock
+ * @param userId a universal unique userId for lock owner identity
+ */
+ private ZKHelixNonblockingLock(String lockPath, String zkAddress, Long
timeout, String lockMsg,
+ String userId) {
+_lockPath = lockPath;
+_timeout = timeout;
+_lockMsg = lockMsg;
+_userId = userId;
+_baseDataAccessor = new ZkBaseDataAccessor<>(zkAddress);
+ }
+
+ @Override
+ public boolean acquireLock() {
+
+// Set lock information fields
+long deadline;
+// Prevent value overflow
+if (_timeout > Long.MAX_VALUE - System.currentTimeMillis()) {
+ deadline = Long.MAX_VALUE;
+} else {
+ deadline = System.currentTimeMillis() + _timeout;
+}
+LockInfo lockInfo = new LockInfo(_userId);
+lockInfo.setLockInfoFields(_userId, _lockMsg, deadline);
+
+LockUpdater updater = new LockUpdater(lockInfo);
+return _baseDataAccessor.update(_lockPath, updater,
AccessOption.PERSISTENT);
+ }
+
+ @Override
+ public boolean releaseLock() {
+// Initialize the lock updater with a default lock info represents the
state of a unlocked lock
+LockUpdater updater = new LockUpdater(LockInfo.defaultLockInfo);
+return _baseDataAccessor.update(_lockPath, updater,
AccessOption.PERSISTENT);
+ }
+
+ @Override
+ public LockInfo getLockInfo() {
Review comment:
To call getCurrentLockInfo would need a lockPath defined. So here I will not
change it to a static method.
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:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: reviews-unsubscr...@helix.apache.org
For additional commands, e-mail: reviews-h...@helix.apache.org
[GitHub] [helix] mgao0 commented on a change in pull request #718: Implement Helix nonblocking lock
mgao0 commented on a change in pull request #718: Implement Helix nonblocking
lock
URL: https://github.com/apache/helix/pull/718#discussion_r379709673
##
File path:
helix-lock/src/main/java/org/apache/helix/lock/helix/HelixLockScope.java
##
@@ -0,0 +1,183 @@
+/*
+ * 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.helix;
+
+import java.util.List;
+
+import org.apache.helix.lock.LockScope;
+import org.apache.helix.util.StringTemplate;
+
+
+/**
+ * Defines the various scopes of Helix locks, and how they are represented on
Zookeeper
+ */
+public class HelixLockScope implements LockScope {
+
+ /**
+ * Define various properties of Helix lock, and associate them with the
number of arguments required for getting znode path
+ */
+ public enum LockScopeProperty {
+
+CLUSTER(1),
+
+PARTICIPANT(2),
+
+RESOURCE(3),
+
+PARTITION(4);
+
+//the number of arguments required to generate a full path for the
specific scope
+final int _pathArgNum;
+
+/**
+ * Initialize a LockScopeProperty
+ * @param pathArgNum the number of arguments required to generate a full
path for the specific scope
+\ */
+private LockScopeProperty(int pathArgNum) {
+ _pathArgNum = pathArgNum;
+}
+
+/**
+ * Get the number of template arguments required to generate a full path
+ * @return number of template arguments in the path
+ */
+public int getPathArgNum() {
+ return _pathArgNum;
+}
+
+/**
+ * Get the position of this argument from the input that used to generate
the scope
+ * @return the number of position of value for this property in the list
of keys input
+ */
+public int getArgPos() {
+ return _pathArgNum - 1;
+}
+ }
+
+ /**
+ * string templates to generate znode path
+ */
+ private static final StringTemplate template = new StringTemplate();
+
+ static {
+template.addEntry(LockScopeProperty.CLUSTER, 1, "/{clusterName}/LOCK");
+template.addEntry(HelixLockScope.LockScopeProperty.PARTICIPANT, 2,
+"/{clusterName}/LOCK/{participantName}");
+template.addEntry(HelixLockScope.LockScopeProperty.RESOURCE, 3,
+"/{clusterName}/LOCK/{participantName}/{resourceName}");
Review comment:
Yes, as discussed, I'll make it flat path for now.
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:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: reviews-unsubscr...@helix.apache.org
For additional commands, e-mail: reviews-h...@helix.apache.org
[GitHub] [helix] mgao0 commented on a change in pull request #718: Implement Helix nonblocking lock
mgao0 commented on a change in pull request #718: Implement Helix nonblocking
lock
URL: https://github.com/apache/helix/pull/718#discussion_r379706750
##
File path:
helix-lock/src/main/java/org/apache/helix/lock/helix/ZKHelixNonblockingLock.java
##
@@ -0,0 +1,186 @@
+/*
+ * 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.helix;
+
+import java.util.Date;
+
+import org.I0Itec.zkclient.DataUpdater;
+import org.apache.helix.AccessOption;
+import org.apache.helix.BaseDataAccessor;
+import org.apache.helix.HelixException;
+import org.apache.helix.ZNRecord;
+import org.apache.helix.lock.HelixLock;
+import org.apache.helix.lock.LockInfo;
+import org.apache.helix.lock.LockScope;
+import org.apache.helix.manager.zk.ZkBaseDataAccessor;
+import org.apache.log4j.Logger;
+
+
+/**
+ * Helix nonblocking lock implementation based on Zookeeper
+ */
+public class ZKHelixNonblockingLock implements HelixLock {
+
+ private static final Logger LOG =
Logger.getLogger(ZKHelixNonblockingLock.class);
+
+ private final String _lockPath;
+ private final String _userId;
+ private final long _timeout;
+ private final String _lockMsg;
+ private final BaseDataAccessor _baseDataAccessor;
+
+ /**
+ * Initialize the lock with user provided information, e.g.,cluster, scope,
etc.
+ * @param scope the scope to lock
+ * @param zkAddress the zk address the cluster connects to
+ * @param timeout the timeout period of the lcok
+ * @param lockMsg the reason for having this lock
+ * @param userId a universal unique userId for lock owner identity
+ */
+ public ZKHelixNonblockingLock(LockScope scope, String zkAddress, Long
timeout, String lockMsg,
+ String userId) {
+this(scope.getPath(), zkAddress, timeout, lockMsg, userId);
+ }
+
+ /**
+ * Initialize the lock with user provided information, e.g., lock path under
zookeeper, etc.
+ * @param lockPath the path of the lock under Zookeeper
+ * @param zkAddress the zk address of the cluster
+ * @param timeout the timeout period of the lcok
+ * @param lockMsg the reason for having this lock
+ * @param userId a universal unique userId for lock owner identity
+ */
+ private ZKHelixNonblockingLock(String lockPath, String zkAddress, Long
timeout, String lockMsg,
+ String userId) {
+_lockPath = lockPath;
+_timeout = timeout;
+_lockMsg = lockMsg;
+_userId = userId;
+_baseDataAccessor = new ZkBaseDataAccessor<>(zkAddress);
+ }
+
+ @Override
+ public boolean acquireLock() {
+
+// Set lock information fields
+long deadline;
+// Prevent value overflow
+if (_timeout > Long.MAX_VALUE - System.currentTimeMillis()) {
+ deadline = Long.MAX_VALUE;
+} else {
+ deadline = System.currentTimeMillis() + _timeout;
+}
+LockInfo lockInfo = new LockInfo(_userId);
+lockInfo.setLockInfoFields(_userId, _lockMsg, deadline);
+
+LockUpdater updater = new LockUpdater(lockInfo);
+return _baseDataAccessor.update(_lockPath, updater,
AccessOption.PERSISTENT);
+ }
+
+ @Override
+ public boolean releaseLock() {
+// Initialize the lock updater with a default lock info represents the
state of a unlocked lock
+LockUpdater updater = new LockUpdater(LockInfo.defaultLockInfo);
+return _baseDataAccessor.update(_lockPath, updater,
AccessOption.PERSISTENT);
+ }
+
+ @Override
+ public LockInfo getLockInfo() {
+ZNRecord curLockInfo = _baseDataAccessor.get(_lockPath, null,
AccessOption.PERSISTENT);
+return new LockInfo(curLockInfo);
+ }
+
+ @Override
+ public boolean isOwner() {
+LockInfo lockInfo = getLockInfo();
+return userIdMatches(lockInfo) && !hasTimedOut(lockInfo);
+ }
+
+ /**
+ * Check if a lock has timed out
+ * @return return true if the lock has timed out, otherwise return false.
+ */
+ private boolean hasTimedOut(LockInfo lockInfo) {
+return System.currentTimeMillis() >= lockInfo.getTimeout();
+ }
+
+ /**
+ * Check if a lock has timed out with lock information stored in a ZNRecord
+ * @return return true if the lock has timed out, otherwise return false.
+ */
+ private boolean hasTimedOut(ZNRecord znRecord) {
+return
[GitHub] [helix] mgao0 commented on a change in pull request #718: Implement Helix nonblocking lock
mgao0 commented on a change in pull request #718: Implement Helix nonblocking
lock
URL: https://github.com/apache/helix/pull/718#discussion_r379705245
##
File path:
helix-lock/src/main/java/org/apache/helix/lock/helix/ZKHelixNonblockingLock.java
##
@@ -0,0 +1,186 @@
+/*
+ * 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.helix;
+
+import java.util.Date;
+
+import org.I0Itec.zkclient.DataUpdater;
+import org.apache.helix.AccessOption;
+import org.apache.helix.BaseDataAccessor;
+import org.apache.helix.HelixException;
+import org.apache.helix.ZNRecord;
+import org.apache.helix.lock.HelixLock;
+import org.apache.helix.lock.LockInfo;
+import org.apache.helix.lock.LockScope;
+import org.apache.helix.manager.zk.ZkBaseDataAccessor;
+import org.apache.log4j.Logger;
+
+
+/**
+ * Helix nonblocking lock implementation based on Zookeeper
+ */
+public class ZKHelixNonblockingLock implements HelixLock {
+
+ private static final Logger LOG =
Logger.getLogger(ZKHelixNonblockingLock.class);
+
+ private final String _lockPath;
+ private final String _userId;
+ private final long _timeout;
+ private final String _lockMsg;
+ private final BaseDataAccessor _baseDataAccessor;
+
+ /**
+ * Initialize the lock with user provided information, e.g.,cluster, scope,
etc.
+ * @param scope the scope to lock
+ * @param zkAddress the zk address the cluster connects to
+ * @param timeout the timeout period of the lcok
+ * @param lockMsg the reason for having this lock
+ * @param userId a universal unique userId for lock owner identity
+ */
+ public ZKHelixNonblockingLock(LockScope scope, String zkAddress, Long
timeout, String lockMsg,
+ String userId) {
+this(scope.getPath(), zkAddress, timeout, lockMsg, userId);
+ }
+
+ /**
+ * Initialize the lock with user provided information, e.g., lock path under
zookeeper, etc.
+ * @param lockPath the path of the lock under Zookeeper
+ * @param zkAddress the zk address of the cluster
+ * @param timeout the timeout period of the lcok
+ * @param lockMsg the reason for having this lock
+ * @param userId a universal unique userId for lock owner identity
+ */
+ private ZKHelixNonblockingLock(String lockPath, String zkAddress, Long
timeout, String lockMsg,
+ String userId) {
+_lockPath = lockPath;
+_timeout = timeout;
+_lockMsg = lockMsg;
+_userId = userId;
+_baseDataAccessor = new ZkBaseDataAccessor<>(zkAddress);
+ }
+
+ @Override
+ public boolean acquireLock() {
+
+// Set lock information fields
+long deadline;
+// Prevent value overflow
+if (_timeout > Long.MAX_VALUE - System.currentTimeMillis()) {
Review comment:
Added the check in the constructor.
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:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: reviews-unsubscr...@helix.apache.org
For additional commands, e-mail: reviews-h...@helix.apache.org
[GitHub] [helix] mgao0 commented on a change in pull request #718: Implement Helix nonblocking lock
mgao0 commented on a change in pull request #718: Implement Helix nonblocking
lock
URL: https://github.com/apache/helix/pull/718#discussion_r379704896
##
File path: helix-lock/src/main/java/org/apache/helix/lock/LockInfo.java
##
@@ -19,28 +19,162 @@
package org.apache.helix.lock;
-import java.util.Map;
+import org.apache.helix.HelixProperty;
+import org.apache.helix.ZNRecord;
/**
- * Generic interface for a map contains the Helix lock information
- * @param The type of the LockInfo value
+ * Structure represents a lock node information, implemented using ZNRecord
*/
-public interface LockInfo {
+public class LockInfo extends HelixProperty {
- //TODO: add specific setter and getter for any field that is determined to
be universal for all implementations of HelixLock
+ // Default values for each attribute if there are no current values set by
user
+ public static final String DEFAULT_OWNER_TEXT = "";
+ public static final String DEFAULT_MESSAGE_TEXT = "";
+ public static final long DEFAULT_TIMEOUT_LONG = -1L;
+
+ // default lock info represents the status of a unlocked lock
+ public static final LockInfo defaultLockInfo = new LockInfo("");
+
+ /**
+ * The keys to lock information
+ */
+ public enum LockInfoAttribute {
+OWNER, MESSAGE, TIMEOUT
+ }
+
+ /**
+ * Initialize a LockInfo with a ZNRecord id, set all info fields to default
data
+ */
+ public LockInfo(String id) {
+super(id);
Review comment:
I has changed the ZNRecord id to "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:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: reviews-unsubscr...@helix.apache.org
For additional commands, e-mail: reviews-h...@helix.apache.org
[GitHub] [helix] mgao0 commented on a change in pull request #718: Implement Helix nonblocking lock
mgao0 commented on a change in pull request #718: Implement Helix nonblocking
lock
URL: https://github.com/apache/helix/pull/718#discussion_r377872027
##
File path: helix-lock/src/main/java/org/apache/helix/lock/HelixLockScope.java
##
@@ -0,0 +1,179 @@
+/*
+ * 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.List;
+
+import org.apache.helix.util.StringTemplate;
+
+
+/**
+ * Defines the various scopes of Helix locks, and how they are represented on
Zookeeper
+ */
+public class HelixLockScope {
Review comment:
Discussed offline. Will make this LockScope more generic so user can define
the properties themselves.
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:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: reviews-unsubscr...@helix.apache.org
For additional commands, e-mail: reviews-h...@helix.apache.org
[GitHub] [helix] mgao0 commented on a change in pull request #718: Implement Helix nonblocking lock
mgao0 commented on a change in pull request #718: Implement Helix nonblocking
lock
URL: https://github.com/apache/helix/pull/718#discussion_r377833465
##
File path:
helix-lock/src/main/java/org/apache/helix/lock/ZKHelixNonblockingLockInfo.java
##
@@ -0,0 +1,92 @@
+/*
+ * 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.HashMap;
+import java.util.Map;
+
+import org.apache.helix.ZNRecord;
+
+
+public class ZKHelixNonblockingLockInfo implements LockInfo
{
Review comment:
Made it extend HelixPropery. I also handled it to ensure there are no null
values in the LockInfo, so there are some methods or logics to set the null
values in ZNRecord to the default values I defined for the LockInfo.
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:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: reviews-unsubscr...@helix.apache.org
For additional commands, e-mail: reviews-h...@helix.apache.org
[GitHub] [helix] mgao0 commented on a change in pull request #718: Implement Helix nonblocking lock
mgao0 commented on a change in pull request #718: Implement Helix nonblocking
lock
URL: https://github.com/apache/helix/pull/718#discussion_r377832567
##
File path:
helix-lock/src/main/java/org/apache/helix/lock/ZKHelixNonblockingLockInfo.java
##
@@ -0,0 +1,92 @@
+/*
+ * 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.HashMap;
+import java.util.Map;
+
+import org.apache.helix.ZNRecord;
+
+
+public class ZKHelixNonblockingLockInfo implements LockInfo
{
+
+ public static final String DEFAULT_OWNER_TEXT = "";
+ public static final String DEFAULT_MESSAGE_TEXT = "";
+ public static final long DEFAULT_TIMEOUT_LONG = -1L;
+ public static final String DEFAULT_TIMEOUT_TEXT =
String.valueOf(DEFAULT_TIMEOUT_LONG);
+ private Map lockInfo;
+
+ public enum InfoKey {
+OWNER, MESSAGE, TIMEOUT
+ }
+
+ /**
+ * Constructor of ZKHelixNonblockingLockInfo that set each field to default
data
+ */
+ public ZKHelixNonblockingLockInfo() {
+lockInfo = new HashMap<>();
+lockInfo.put(InfoKey.OWNER, DEFAULT_OWNER_TEXT);
+lockInfo.put(InfoKey.MESSAGE, DEFAULT_MESSAGE_TEXT);
+lockInfo.put(InfoKey.TIMEOUT, DEFAULT_TIMEOUT_TEXT);
+ }
+
+ /**
+ * Construct a ZKHelixNonblockingLockInfo using a ZNRecord format of data
+ * @param znRecord A ZNRecord that contains lock information in its simple
fields
+ */
+ public ZKHelixNonblockingLockInfo(ZNRecord znRecord) {
Review comment:
I added the strongly typed parameter constructor. But currently I'm not
calling this method using the other 2 constructors since I made the LockInfo
class extend HelixProperty, and I used the super(). With super(), a ZNRecord is
already created so we only need to update the values in the ZNRecord.
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:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: reviews-unsubscr...@helix.apache.org
For additional commands, e-mail: reviews-h...@helix.apache.org
[GitHub] [helix] mgao0 commented on a change in pull request #718: Implement Helix nonblocking lock
mgao0 commented on a change in pull request #718: Implement Helix nonblocking
lock
URL: https://github.com/apache/helix/pull/718#discussion_r377829738
##
File path:
helix-lock/src/main/java/org/apache/helix/lock/ZKHelixNonblockingLock.java
##
@@ -0,0 +1,198 @@
+/*
+ * 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.Date;
+
+import org.I0Itec.zkclient.DataUpdater;
+import org.apache.helix.AccessOption;
+import org.apache.helix.BaseDataAccessor;
+import org.apache.helix.HelixException;
+import org.apache.helix.ZNRecord;
+import org.apache.helix.manager.zk.ZkBaseDataAccessor;
+import org.apache.log4j.Logger;
+
+import static
org.apache.helix.lock.ZKHelixNonblockingLockInfo.DEFAULT_OWNER_TEXT;
+import static
org.apache.helix.lock.ZKHelixNonblockingLockInfo.DEFAULT_TIMEOUT_LONG;
+
+
+/**
+ * Helix nonblocking lock implementation based on Zookeeper
+ */
+public class ZKHelixNonblockingLock implements HelixLock {
+
+ private static final Logger LOG =
Logger.getLogger(ZKHelixNonblockingLock.class);
+
+ private static final String LOCK_ROOT = "/LOCK";
+ private final String _lockPath;
+ private final String _userId;
+ private final long _timeout;
+ private final String _lockMsg;
+ private final BaseDataAccessor _baseDataAccessor;
+
+ /**
+ * Initialize the lock with user provided information, e.g.,cluster, scope,
etc.
+ * @param clusterName the cluster under which the lock will live
+ * @param scope the scope to lock
+ * @param zkAddress the zk address the cluster connects to
+ * @param timeout the timeout period of the lcok
+ * @param lockMsg the reason for having this lock
+ * @param userId a universal unique userId for lock owner identity
+ */
+ public ZKHelixNonblockingLock(String clusterName, HelixLockScope scope,
String zkAddress,
+ Long timeout, String lockMsg, String userId) {
+this("/" + clusterName.toUpperCase() + LOCK_ROOT + scope.getZkPath(),
zkAddress, timeout, lockMsg, userId);
+ }
+
+ /**
+ * Initialize the lock with user provided information, e.g., lock path under
zookeeper, etc.
+ * @param lockPath the path of the lock under Zookeeper
+ * @param zkAddress the zk address of the cluster
+ * @param timeout the timeout period of the lcok
+ * @param lockMsg the reason for having this lock
+ * @param userId a universal unique userId for lock owner identity
+ */
+ private ZKHelixNonblockingLock(String lockPath, String zkAddress, Long
timeout, String lockMsg,
+ String userId) {
+_lockPath = lockPath;
+_timeout = timeout;
+_lockMsg = lockMsg;
+_userId = userId;
+_baseDataAccessor = new ZkBaseDataAccessor<>(zkAddress);
+ }
+
+ @Override
+ public boolean acquireLock() {
+
+// Set lock information fields
Review comment:
Very good suggestion. I moved all the set lock info logic to LockInfo class.
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:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: reviews-unsubscr...@helix.apache.org
For additional commands, e-mail: reviews-h...@helix.apache.org
[GitHub] [helix] mgao0 commented on a change in pull request #718: Implement Helix nonblocking lock
mgao0 commented on a change in pull request #718: Implement Helix nonblocking
lock
URL: https://github.com/apache/helix/pull/718#discussion_r377829043
##
File path: helix-lock/src/main/java/org/apache/helix/lock/HelixLockScope.java
##
@@ -0,0 +1,179 @@
+/*
+ * 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.List;
+
+import org.apache.helix.util.StringTemplate;
+
+
+/**
+ * Defines the various scopes of Helix locks, and how they are represented on
Zookeeper
+ */
+public class HelixLockScope {
Review comment:
You are right, it would be cleaner if we have an abstract class. But I
decided not to do it at this time because
1. HelixLockScope and HelixConfigScope don't share the same logic for most
part, for example: HelixConfigScope takes a mapKey as a variable in the
constructor while HelixLockScope does not, and these two classes don't define
same zkPath argument number for each level so the getZkPath logic is different
too.
2. We want to decouple Helix Lock module from Helix core module, so in the
future if needed we can freely extend the lock scope for other usages of the
lock module.
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:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: reviews-unsubscr...@helix.apache.org
For additional commands, e-mail: reviews-h...@helix.apache.org
[GitHub] [helix] mgao0 commented on a change in pull request #718: Implement Helix nonblocking lock
mgao0 commented on a change in pull request #718: Implement Helix nonblocking
lock
URL: https://github.com/apache/helix/pull/718#discussion_r375610360
##
File path:
helix-lock/src/main/java/org/apache/helix/lock/ZKHelixNonblockingLock.java
##
@@ -0,0 +1,200 @@
+/*
+ * 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.UUID;
+
+import com.google.common.annotations.VisibleForTesting;
+import org.I0Itec.zkclient.DataUpdater;
+import org.apache.helix.AccessOption;
+import org.apache.helix.BaseDataAccessor;
+import org.apache.helix.HelixException;
+import org.apache.helix.ZNRecord;
+import org.apache.helix.manager.zk.ZkBaseDataAccessor;
+import org.apache.helix.manager.zk.ZkClient;
+import org.apache.helix.manager.zk.client.HelixZkClient;
+import org.apache.helix.model.HelixConfigScope;
+import org.apache.log4j.Logger;
+import org.apache.zookeeper.data.Stat;
+
+
+/**
+ * Helix nonblocking lock implementation based on Zookeeper
+ */
+public class ZKHelixNonblockingLock implements HelixLock {
+
+ private static final Logger LOG =
Logger.getLogger(ZKHelixNonblockingLock.class);
+
+ private static final String LOCK_ROOT = "LOCKS";
+ private static final String PATH_DELIMITER = "/";
+ private static final String UUID_FORMAT_REGEX =
+
"[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}";
+ private final String _lockPath;
+ private final String _userId;
+ private final long _timeout;
+ private final String _lockMsg;
+ private final BaseDataAccessor _baseDataAccessor;
+
+ /**
+ * Initialize the lock with user provided information, e.g.,cluster, scope,
etc.
+ * @param clusterName the cluster under which the lock will live
+ * @param scope the scope to lock
+ * @param zkAddress the zk address the cluster connects to
+ * @param timeout the timeout period of the lcok
+ * @param lockMsg the reason for having this lock
+ * @param userId a universal unique userId for lock owner identity
+ */
+ public ZKHelixNonblockingLock(String clusterName, HelixConfigScope scope,
String zkAddress,
+ Long timeout, String lockMsg, String userId) {
+this(PATH_DELIMITER + clusterName + PATH_DELIMITER + LOCK_ROOT +
PATH_DELIMITER + scope,
+zkAddress, timeout, lockMsg, userId);
+ }
+
+ /**
+ * Initialize the lock with user provided information, e.g., lock path under
zookeeper, etc.
+ * @param lockPath the path of the lock under Zookeeper
+ * @param zkAddress the zk address of the cluster
+ * @param timeout the timeout period of the lcok
+ * @param lockMsg the reason for having this lock
+ * @param userId a universal unique userId for lock owner identity
+ */
+ private ZKHelixNonblockingLock(String lockPath, String zkAddress, Long
timeout, String lockMsg,
+ String userId) {
+HelixZkClient zkClient = new ZkClient(zkAddress);
+_lockPath = lockPath;
+_timeout = timeout;
+_lockMsg = lockMsg;
+if (!userId.matches(UUID_FORMAT_REGEX)) {
+ throw new IllegalArgumentException("The input user id is not a valid
UUID.");
+}
+_userId = userId;
+_baseDataAccessor = new
ZkBaseDataAccessor(zkClient.getServers());
+ }
+
+ @Override
+ public boolean acquireLock() {
+
+// Set lock information fields
+ZNRecord lockInfo = new ZNRecord(_userId);
+lockInfo.setSimpleField(ZKHelixNonblockingLockInfo.InfoKey.OWNER.name(),
_userId);
+lockInfo.setSimpleField(ZKHelixNonblockingLockInfo.InfoKey.MESSAGE.name(),
_lockMsg);
+long timeout;
+
+// If the input timeout value is the max value, set the expire time to max
value
+if (_timeout == Long.MAX_VALUE) {
+ timeout = _timeout;
+} else {
+ timeout = System.currentTimeMillis() + _timeout;
+}
+lockInfo
+.setSimpleField(ZKHelixNonblockingLockInfo.InfoKey.TIMEOUT.name(),
String.valueOf(timeout));
+
+// Try to create the lock node
+boolean success = _baseDataAccessor.create(_lockPath, lockInfo,
AccessOption.PERSISTENT);
+
+// If fail to create the lock node (acquire the lock), compare the timeout
timestamp of current lock node with current time, if already passes the
timeout, release
[GitHub] [helix] mgao0 commented on a change in pull request #718: Implement Helix nonblocking lock
mgao0 commented on a change in pull request #718: Implement Helix nonblocking
lock
URL: https://github.com/apache/helix/pull/718#discussion_r375597685
##
File path:
helix-lock/src/main/java/org/apache/helix/lock/ZKHelixNonblockingLock.java
##
@@ -0,0 +1,200 @@
+/*
+ * 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.UUID;
+
+import com.google.common.annotations.VisibleForTesting;
+import org.I0Itec.zkclient.DataUpdater;
+import org.apache.helix.AccessOption;
+import org.apache.helix.BaseDataAccessor;
+import org.apache.helix.HelixException;
+import org.apache.helix.ZNRecord;
+import org.apache.helix.manager.zk.ZkBaseDataAccessor;
+import org.apache.helix.manager.zk.ZkClient;
+import org.apache.helix.manager.zk.client.HelixZkClient;
+import org.apache.helix.model.HelixConfigScope;
+import org.apache.log4j.Logger;
+import org.apache.zookeeper.data.Stat;
+
+
+/**
+ * Helix nonblocking lock implementation based on Zookeeper
+ */
+public class ZKHelixNonblockingLock implements HelixLock {
+
+ private static final Logger LOG =
Logger.getLogger(ZKHelixNonblockingLock.class);
+
+ private static final String LOCK_ROOT = "LOCKS";
+ private static final String PATH_DELIMITER = "/";
+ private static final String UUID_FORMAT_REGEX =
+
"[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}";
+ private final String _lockPath;
+ private final String _userId;
+ private final long _timeout;
+ private final String _lockMsg;
+ private final BaseDataAccessor _baseDataAccessor;
+
+ /**
+ * Initialize the lock with user provided information, e.g.,cluster, scope,
etc.
+ * @param clusterName the cluster under which the lock will live
+ * @param scope the scope to lock
+ * @param zkAddress the zk address the cluster connects to
+ * @param timeout the timeout period of the lcok
+ * @param lockMsg the reason for having this lock
+ * @param userId a universal unique userId for lock owner identity
+ */
+ public ZKHelixNonblockingLock(String clusterName, HelixConfigScope scope,
String zkAddress,
+ Long timeout, String lockMsg, String userId) {
+this(PATH_DELIMITER + clusterName + PATH_DELIMITER + LOCK_ROOT +
PATH_DELIMITER + scope,
+zkAddress, timeout, lockMsg, userId);
+ }
+
+ /**
+ * Initialize the lock with user provided information, e.g., lock path under
zookeeper, etc.
+ * @param lockPath the path of the lock under Zookeeper
+ * @param zkAddress the zk address of the cluster
+ * @param timeout the timeout period of the lcok
+ * @param lockMsg the reason for having this lock
+ * @param userId a universal unique userId for lock owner identity
+ */
+ private ZKHelixNonblockingLock(String lockPath, String zkAddress, Long
timeout, String lockMsg,
+ String userId) {
+HelixZkClient zkClient = new ZkClient(zkAddress);
+_lockPath = lockPath;
+_timeout = timeout;
+_lockMsg = lockMsg;
+if (!userId.matches(UUID_FORMAT_REGEX)) {
+ throw new IllegalArgumentException("The input user id is not a valid
UUID.");
+}
+_userId = userId;
+_baseDataAccessor = new
ZkBaseDataAccessor(zkClient.getServers());
+ }
+
+ @Override
+ public boolean acquireLock() {
+
+// Set lock information fields
+ZNRecord lockInfo = new ZNRecord(_userId);
+lockInfo.setSimpleField(ZKHelixNonblockingLockInfo.InfoKey.OWNER.name(),
_userId);
+lockInfo.setSimpleField(ZKHelixNonblockingLockInfo.InfoKey.MESSAGE.name(),
_lockMsg);
+long timeout;
+
+// If the input timeout value is the max value, set the expire time to max
value
+if (_timeout == Long.MAX_VALUE) {
+ timeout = _timeout;
+} else {
+ timeout = System.currentTimeMillis() + _timeout;
+}
+lockInfo
+.setSimpleField(ZKHelixNonblockingLockInfo.InfoKey.TIMEOUT.name(),
String.valueOf(timeout));
+
+// Try to create the lock node
+boolean success = _baseDataAccessor.create(_lockPath, lockInfo,
AccessOption.PERSISTENT);
+
+// If fail to create the lock node (acquire the lock), compare the timeout
timestamp of current lock node with current time, if already passes the
timeout, release
[GitHub] [helix] mgao0 commented on a change in pull request #718: Implement Helix nonblocking lock
mgao0 commented on a change in pull request #718: Implement Helix nonblocking
lock
URL: https://github.com/apache/helix/pull/718#discussion_r375591453
##
File path:
helix-lock/src/main/java/org/apache/helix/lock/ZKHelixNonblockingLock.java
##
@@ -0,0 +1,200 @@
+/*
+ * 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.UUID;
+
+import com.google.common.annotations.VisibleForTesting;
+import org.I0Itec.zkclient.DataUpdater;
+import org.apache.helix.AccessOption;
+import org.apache.helix.BaseDataAccessor;
+import org.apache.helix.HelixException;
+import org.apache.helix.ZNRecord;
+import org.apache.helix.manager.zk.ZkBaseDataAccessor;
+import org.apache.helix.manager.zk.ZkClient;
+import org.apache.helix.manager.zk.client.HelixZkClient;
+import org.apache.helix.model.HelixConfigScope;
+import org.apache.log4j.Logger;
+import org.apache.zookeeper.data.Stat;
+
+
+/**
+ * Helix nonblocking lock implementation based on Zookeeper
+ */
+public class ZKHelixNonblockingLock implements HelixLock {
+
+ private static final Logger LOG =
Logger.getLogger(ZKHelixNonblockingLock.class);
+
+ private static final String LOCK_ROOT = "LOCKS";
+ private static final String PATH_DELIMITER = "/";
+ private static final String UUID_FORMAT_REGEX =
+
"[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}";
+ private final String _lockPath;
+ private final String _userId;
+ private final long _timeout;
+ private final String _lockMsg;
+ private final BaseDataAccessor _baseDataAccessor;
+
+ /**
+ * Initialize the lock with user provided information, e.g.,cluster, scope,
etc.
+ * @param clusterName the cluster under which the lock will live
+ * @param scope the scope to lock
+ * @param zkAddress the zk address the cluster connects to
+ * @param timeout the timeout period of the lcok
+ * @param lockMsg the reason for having this lock
+ * @param userId a universal unique userId for lock owner identity
+ */
+ public ZKHelixNonblockingLock(String clusterName, HelixConfigScope scope,
String zkAddress,
+ Long timeout, String lockMsg, String userId) {
+this(PATH_DELIMITER + clusterName + PATH_DELIMITER + LOCK_ROOT +
PATH_DELIMITER + scope,
+zkAddress, timeout, lockMsg, userId);
+ }
+
+ /**
+ * Initialize the lock with user provided information, e.g., lock path under
zookeeper, etc.
+ * @param lockPath the path of the lock under Zookeeper
+ * @param zkAddress the zk address of the cluster
+ * @param timeout the timeout period of the lcok
+ * @param lockMsg the reason for having this lock
+ * @param userId a universal unique userId for lock owner identity
+ */
+ private ZKHelixNonblockingLock(String lockPath, String zkAddress, Long
timeout, String lockMsg,
+ String userId) {
+HelixZkClient zkClient = new ZkClient(zkAddress);
+_lockPath = lockPath;
+_timeout = timeout;
+_lockMsg = lockMsg;
+if (!userId.matches(UUID_FORMAT_REGEX)) {
+ throw new IllegalArgumentException("The input user id is not a valid
UUID.");
+}
+_userId = userId;
+_baseDataAccessor = new
ZkBaseDataAccessor(zkClient.getServers());
+ }
+
+ @Override
+ public boolean acquireLock() {
+
+// Set lock information fields
+ZNRecord lockInfo = new ZNRecord(_userId);
+lockInfo.setSimpleField(ZKHelixNonblockingLockInfo.InfoKey.OWNER.name(),
_userId);
+lockInfo.setSimpleField(ZKHelixNonblockingLockInfo.InfoKey.MESSAGE.name(),
_lockMsg);
+long timeout;
+
+// If the input timeout value is the max value, set the expire time to max
value
+if (_timeout == Long.MAX_VALUE) {
+ timeout = _timeout;
+} else {
+ timeout = System.currentTimeMillis() + _timeout;
+}
+lockInfo
+.setSimpleField(ZKHelixNonblockingLockInfo.InfoKey.TIMEOUT.name(),
String.valueOf(timeout));
+
+// Try to create the lock node
+boolean success = _baseDataAccessor.create(_lockPath, lockInfo,
AccessOption.PERSISTENT);
+
+// If fail to create the lock node (acquire the lock), compare the timeout
timestamp of current lock node with current time, if already passes the
timeout, release
[GitHub] [helix] mgao0 commented on a change in pull request #718: Implement Helix nonblocking lock
mgao0 commented on a change in pull request #718: Implement Helix nonblocking
lock
URL: https://github.com/apache/helix/pull/718#discussion_r375590005
##
File path:
helix-lock/src/main/java/org/apache/helix/lock/ZKHelixNonblockingLock.java
##
@@ -0,0 +1,200 @@
+/*
+ * 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.UUID;
+
+import com.google.common.annotations.VisibleForTesting;
+import org.I0Itec.zkclient.DataUpdater;
+import org.apache.helix.AccessOption;
+import org.apache.helix.BaseDataAccessor;
+import org.apache.helix.HelixException;
+import org.apache.helix.ZNRecord;
+import org.apache.helix.manager.zk.ZkBaseDataAccessor;
+import org.apache.helix.manager.zk.ZkClient;
+import org.apache.helix.manager.zk.client.HelixZkClient;
+import org.apache.helix.model.HelixConfigScope;
+import org.apache.log4j.Logger;
+import org.apache.zookeeper.data.Stat;
+
+
+/**
+ * Helix nonblocking lock implementation based on Zookeeper
+ */
+public class ZKHelixNonblockingLock implements HelixLock {
+
+ private static final Logger LOG =
Logger.getLogger(ZKHelixNonblockingLock.class);
+
+ private static final String LOCK_ROOT = "LOCKS";
+ private static final String PATH_DELIMITER = "/";
+ private static final String UUID_FORMAT_REGEX =
+
"[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}";
Review comment:
You are right, as long as we ask caller to provide user id themselves, we
don't have control over it. But it works for our current use cases. So I'll
remove the user id validation for now, since we don't want to make it harder
for users to use.
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:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: reviews-unsubscr...@helix.apache.org
For additional commands, e-mail: reviews-h...@helix.apache.org
[GitHub] [helix] mgao0 commented on a change in pull request #718: Implement Helix nonblocking lock
mgao0 commented on a change in pull request #718: Implement Helix nonblocking
lock
URL: https://github.com/apache/helix/pull/718#discussion_r375022574
##
File path:
helix-lock/src/main/java/org/apache/helix/lock/ZKHelixNonblockingLock.java
##
@@ -0,0 +1,159 @@
+/*
+ * 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.HashMap;
+import java.util.Map;
+import java.util.UUID;
+
+import org.apache.helix.AccessOption;
+import org.apache.helix.BaseDataAccessor;
+import org.apache.helix.ZNRecord;
+import org.apache.helix.ZNRecordUpdater;
+import org.apache.helix.manager.zk.ZNRecordSerializer;
+import org.apache.helix.manager.zk.ZkBaseDataAccessor;
+import org.apache.helix.manager.zk.ZkClient;
+import org.apache.helix.manager.zk.client.HelixZkClient;
+import org.apache.helix.model.HelixConfigScope;
+import org.apache.helix.util.ZNRecordUtil;
+import org.apache.log4j.Logger;
+import org.apache.zookeeper.data.Stat;
+
+
+/**
+ * Helix nonblocking lock implementation based on Zookeeper
+ */
+public class ZKHelixNonblockingLock implements HelixLock {
+
+ private static final Logger LOG =
Logger.getLogger(ZKHelixNonblockingLock.class);
+
+ private static final String LOCK_ROOT = "LOCKS";
+ private final String _lockPath;
+ private final String _userID;
+ private final long _timeout;
+ private final String _lockMsg;
+ private final BaseDataAccessor _baseDataAccessor;
+
+ /**
+ * Initialize the lock with user provided information, e.g.,cluster, scope,
etc.
+ * @param clusterName the cluster under which the lock will live
+ * @param scope the scope to lock
+ * @param zkAddress the zk address the cluster connects to
+ * @param timeout the timeout period of the lcok
+ * @param lockMsg the reason for having this lock
+ */
+ public ZKHelixNonblockingLock(String clusterName, HelixConfigScope scope,
String zkAddress,
+ Long timeout, String lockMsg, String userID) {
+this("/" + clusterName + '/' + LOCK_ROOT + '/' + scope, zkAddress,
timeout, lockMsg, userID);
+ }
+
+ /**
+ * Initialize the lock with user provided information, e.g., lock path under
zookeeper, etc.
+ * @param lockPath the path of the lock under Zookeeper
+ * @param zkAddress the zk address of the cluster
+ * @param timeout the timeout period of the lcok
+ * @param lockMsg the reason for having this lock
+ */
+ public ZKHelixNonblockingLock(String lockPath, String zkAddress, Long
timeout, String lockMsg,
+ String userID) {
+HelixZkClient zkClient = new ZkClient(zkAddress);
+_lockPath = lockPath;
+_timeout = timeout;
+_lockMsg = lockMsg;
+_userID = userID;
+_baseDataAccessor = new
ZkBaseDataAccessor(zkClient.getServers());
+ }
+
+ @Override
+ /**
+ * Blocking call to acquire a lock
+ * @return true if the lock was successfully acquired,
+ * false if the lock could not be acquired
+ */ public boolean acquireLock() {
+
+// Set lock information fields
+ZNRecord lockInfo = new ZNRecord(_userID);
+lockInfo.setSimpleField(ZKHelixNonblockingLockInfo.InfoKey.OWNER.name(),
_userID);
+lockInfo.setSimpleField(ZKHelixNonblockingLockInfo.InfoKey.MESSAGE.name(),
_lockMsg);
+long timeout = System.currentTimeMillis() + _timeout;
+lockInfo
+.setSimpleField(ZKHelixNonblockingLockInfo.InfoKey.TIMEOUT.name(),
String.valueOf(timeout));
+
+// Try to create the lock node
+boolean success = _baseDataAccessor.create(_lockPath, lockInfo,
AccessOption.PERSISTENT);
+
+// If fail to create the lock node (acquire the lock), compare the timeout
timestamp of current lock node with current time, if already passes the
timeout, release current lock and try to acquire the lock again
+if (!success) {
+ Stat stat = new Stat();
+ ZNRecord curLock = _baseDataAccessor.get(_lockPath, stat,
AccessOption.PERSISTENT);
+ long curTimeout =
+
Long.parseLong(curLock.getSimpleField(ZKHelixNonblockingLockInfo.InfoKey.TIMEOUT.name()));
+ if (System.currentTimeMillis() >= curTimeout) {
+success =
+_baseDataAccessor.set(_lockPath, lockInfo, stat.getVersion(),
AccessOption.PERSISTENT);
+
[GitHub] [helix] mgao0 commented on a change in pull request #718: Implement Helix nonblocking lock
mgao0 commented on a change in pull request #718: Implement Helix nonblocking
lock
URL: https://github.com/apache/helix/pull/718#discussion_r375021645
##
File path:
helix-lock/src/main/java/org/apache/helix/lock/ZKHelixNonblockingLock.java
##
@@ -0,0 +1,151 @@
+/*
+ * 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.HashMap;
+import java.util.Map;
+import java.util.UUID;
+
+import org.apache.helix.AccessOption;
+import org.apache.helix.BaseDataAccessor;
+import org.apache.helix.ZNRecord;
+import org.apache.helix.ZNRecordUpdater;
+import org.apache.helix.manager.zk.ZNRecordSerializer;
+import org.apache.helix.manager.zk.ZkBaseDataAccessor;
+import org.apache.helix.manager.zk.ZkClient;
+import org.apache.helix.manager.zk.client.HelixZkClient;
+import org.apache.helix.model.HelixConfigScope;
+import org.apache.helix.util.ZNRecordUtil;
+import org.apache.log4j.Logger;
+import org.apache.zookeeper.data.Stat;
+
+
+/**
+ * Helix nonblocking lock implementation based on Zookeeper
+ */
+public class ZKHelixNonblockingLock implements HelixLock {
+
+ private static final Logger LOG =
Logger.getLogger(ZKHelixNonblockingLock.class);
+
+ private static final String LOCK_ROOT = "LOCKS";
+ private final String _lockPath;
+ private final String _userId;
+ private final long _timeout;
+ private final String _lockMsg;
+ private final BaseDataAccessor _baseDataAccessor;
+
+ /**
+ * Initialize the lock with user provided information, e.g.,cluster, scope,
etc.
+ * @param clusterName the cluster under which the lock will live
+ * @param scope the scope to lock
+ * @param zkAddress the zk address the cluster connects to
+ * @param timeout the timeout period of the lcok
+ * @param lockMsg the reason for having this lock
+ * @param userId a universal unique userId for lock owner identity
+ */
+ public ZKHelixNonblockingLock(String clusterName, HelixConfigScope scope,
String zkAddress,
+ Long timeout, String lockMsg, String userId) {
+this("/" + clusterName + '/' + LOCK_ROOT + '/' + scope, zkAddress,
timeout, lockMsg, userId);
+ }
+
+ /**
+ * Initialize the lock with user provided information, e.g., lock path under
zookeeper, etc.
+ * @param lockPath the path of the lock under Zookeeper
+ * @param zkAddress the zk address of the cluster
+ * @param timeout the timeout period of the lcok
+ * @param lockMsg the reason for having this lock
+ * @param userId a universal unique userId for lock owner identity
+ */
+ public ZKHelixNonblockingLock(String lockPath, String zkAddress, Long
timeout, String lockMsg,
+ String userId) {
+HelixZkClient zkClient = new ZkClient(zkAddress);
+_lockPath = lockPath;
+_timeout = timeout;
+_lockMsg = lockMsg;
+if
(userId.matches("[a-f0-9]{8}-[a-f0-9]{4}-4[0-9]{3}-[89ab][a-f0-9]{3}-[0-9a-f]{12}"))
{
+ _userId = userId;
+} else {
+ throw new IllegalArgumentException();
+}
+_baseDataAccessor = new
ZkBaseDataAccessor(zkClient.getServers());
+ }
+
+ @Override
+ public boolean acquireLock() {
+
+// Set lock information fields
+ZNRecord lockInfo = new ZNRecord(_userId);
+lockInfo.setSimpleField(ZKHelixNonblockingLockInfo.InfoKey.OWNER.name(),
_userId);
+lockInfo.setSimpleField(ZKHelixNonblockingLockInfo.InfoKey.MESSAGE.name(),
_lockMsg);
+long timeout = System.currentTimeMillis() + _timeout;
+lockInfo
+.setSimpleField(ZKHelixNonblockingLockInfo.InfoKey.TIMEOUT.name(),
String.valueOf(timeout));
+
+// Try to create the lock node
+boolean success = _baseDataAccessor.create(_lockPath, lockInfo,
AccessOption.PERSISTENT);
+
+// If fail to create the lock node (acquire the lock), compare the timeout
timestamp of current lock node with current time, if already passes the
timeout, release current lock and try to acquire the lock again
+if (!success) {
+ Stat stat = new Stat();
+ ZNRecord curLock = _baseDataAccessor.get(_lockPath, stat,
AccessOption.PERSISTENT);
Review comment:
I use this Stat data to get the version number for the set operation later.
It is one time use so i didn't set it in the ZNRecord.
[GitHub] [helix] mgao0 commented on a change in pull request #718: Implement Helix nonblocking lock
mgao0 commented on a change in pull request #718: Implement Helix nonblocking
lock
URL: https://github.com/apache/helix/pull/718#discussion_r375021075
##
File path:
helix-lock/src/main/java/org/apache/helix/lock/ZKHelixNonblockingLock.java
##
@@ -0,0 +1,151 @@
+/*
+ * 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.HashMap;
+import java.util.Map;
+import java.util.UUID;
+
+import org.apache.helix.AccessOption;
+import org.apache.helix.BaseDataAccessor;
+import org.apache.helix.ZNRecord;
+import org.apache.helix.ZNRecordUpdater;
+import org.apache.helix.manager.zk.ZNRecordSerializer;
+import org.apache.helix.manager.zk.ZkBaseDataAccessor;
+import org.apache.helix.manager.zk.ZkClient;
+import org.apache.helix.manager.zk.client.HelixZkClient;
+import org.apache.helix.model.HelixConfigScope;
+import org.apache.helix.util.ZNRecordUtil;
+import org.apache.log4j.Logger;
+import org.apache.zookeeper.data.Stat;
+
+
+/**
+ * Helix nonblocking lock implementation based on Zookeeper
+ */
+public class ZKHelixNonblockingLock implements HelixLock {
+
+ private static final Logger LOG =
Logger.getLogger(ZKHelixNonblockingLock.class);
+
+ private static final String LOCK_ROOT = "LOCKS";
+ private final String _lockPath;
+ private final String _userId;
+ private final long _timeout;
+ private final String _lockMsg;
+ private final BaseDataAccessor _baseDataAccessor;
+
+ /**
+ * Initialize the lock with user provided information, e.g.,cluster, scope,
etc.
+ * @param clusterName the cluster under which the lock will live
+ * @param scope the scope to lock
+ * @param zkAddress the zk address the cluster connects to
+ * @param timeout the timeout period of the lcok
+ * @param lockMsg the reason for having this lock
+ * @param userId a universal unique userId for lock owner identity
+ */
+ public ZKHelixNonblockingLock(String clusterName, HelixConfigScope scope,
String zkAddress,
+ Long timeout, String lockMsg, String userId) {
+this("/" + clusterName + '/' + LOCK_ROOT + '/' + scope, zkAddress,
timeout, lockMsg, userId);
Review comment:
I created a constant for path delimiter. Please see updated version.
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:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: reviews-unsubscr...@helix.apache.org
For additional commands, e-mail: reviews-h...@helix.apache.org
[GitHub] [helix] mgao0 commented on a change in pull request #718: Implement Helix nonblocking lock
mgao0 commented on a change in pull request #718: Implement Helix nonblocking
lock
URL: https://github.com/apache/helix/pull/718#discussion_r375021107
##
File path:
helix-lock/src/main/java/org/apache/helix/lock/ZKHelixNonblockingLock.java
##
@@ -0,0 +1,151 @@
+/*
+ * 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.HashMap;
+import java.util.Map;
+import java.util.UUID;
+
+import org.apache.helix.AccessOption;
+import org.apache.helix.BaseDataAccessor;
+import org.apache.helix.ZNRecord;
+import org.apache.helix.ZNRecordUpdater;
+import org.apache.helix.manager.zk.ZNRecordSerializer;
+import org.apache.helix.manager.zk.ZkBaseDataAccessor;
+import org.apache.helix.manager.zk.ZkClient;
+import org.apache.helix.manager.zk.client.HelixZkClient;
+import org.apache.helix.model.HelixConfigScope;
+import org.apache.helix.util.ZNRecordUtil;
+import org.apache.log4j.Logger;
+import org.apache.zookeeper.data.Stat;
+
+
+/**
+ * Helix nonblocking lock implementation based on Zookeeper
+ */
+public class ZKHelixNonblockingLock implements HelixLock {
+
+ private static final Logger LOG =
Logger.getLogger(ZKHelixNonblockingLock.class);
+
+ private static final String LOCK_ROOT = "LOCKS";
+ private final String _lockPath;
+ private final String _userId;
+ private final long _timeout;
+ private final String _lockMsg;
+ private final BaseDataAccessor _baseDataAccessor;
+
+ /**
+ * Initialize the lock with user provided information, e.g.,cluster, scope,
etc.
+ * @param clusterName the cluster under which the lock will live
+ * @param scope the scope to lock
+ * @param zkAddress the zk address the cluster connects to
+ * @param timeout the timeout period of the lcok
+ * @param lockMsg the reason for having this lock
+ * @param userId a universal unique userId for lock owner identity
+ */
+ public ZKHelixNonblockingLock(String clusterName, HelixConfigScope scope,
String zkAddress,
+ Long timeout, String lockMsg, String userId) {
+this("/" + clusterName + '/' + LOCK_ROOT + '/' + scope, zkAddress,
timeout, lockMsg, userId);
+ }
+
+ /**
+ * Initialize the lock with user provided information, e.g., lock path under
zookeeper, etc.
+ * @param lockPath the path of the lock under Zookeeper
+ * @param zkAddress the zk address of the cluster
+ * @param timeout the timeout period of the lcok
+ * @param lockMsg the reason for having this lock
+ * @param userId a universal unique userId for lock owner identity
+ */
+ public ZKHelixNonblockingLock(String lockPath, String zkAddress, Long
timeout, String lockMsg,
+ String userId) {
+HelixZkClient zkClient = new ZkClient(zkAddress);
+_lockPath = lockPath;
+_timeout = timeout;
+_lockMsg = lockMsg;
+if
(userId.matches("[a-f0-9]{8}-[a-f0-9]{4}-4[0-9]{3}-[89ab][a-f0-9]{3}-[0-9a-f]{12}"))
{
Review comment:
Done.
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:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: reviews-unsubscr...@helix.apache.org
For additional commands, e-mail: reviews-h...@helix.apache.org
[GitHub] [helix] mgao0 commented on a change in pull request #718: Implement Helix nonblocking lock
mgao0 commented on a change in pull request #718: Implement Helix nonblocking
lock
URL: https://github.com/apache/helix/pull/718#discussion_r375020850
##
File path:
helix-lock/src/main/java/org/apache/helix/lock/ZKHelixNonblockingLockInfo.java
##
@@ -0,0 +1,69 @@
+/*
+ * 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.HashMap;
+import java.util.Map;
+
+import org.apache.helix.ZNRecord;
+
+
+public class ZKHelixNonblockingLockInfo implements
LockInfo {
+
+ private Map lockInfo;
+
+ enum InfoKey {
+OWNER, MESSAGE, TIMEOUT
+ }
+
+ public ZKHelixNonblockingLockInfo() {
+lockInfo = new HashMap<>();
+ }
+
+ @Override
+ /**
+ * Create a single filed of LockInfo, or update the value of the field if it
already exists
+ * @param infoKey the key of the LockInfo field
+ * @param infoValue the value of the LockInfo field
+ */ public void setInfoValue(String infoKey, String infoValue) {
+lockInfo.put(infoKey, infoValue);
+ }
+
+ @Override
+ /**
+ * Get the value of a field in LockInfo
+ * @param infoKey the key of the LockInfo field
+ * @return the value of the field or null if this key does not exist
+ */ public T getInfoValue(String infoKey) {
+return (T) lockInfo.get(infoKey);
+ }
+
+ /**
+ * Update the lock info with information in a ZNRecord
+ * @param record Information about the lock that stored as ZNRecord format
+ */
+ public void setLockInfoFields(ZNRecord record) {
Review comment:
I deleted this method, and created a copy constructor in
ZKHelixNonblockingLockInfo to copy fields from ZNRecord.
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:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: reviews-unsubscr...@helix.apache.org
For additional commands, e-mail: reviews-h...@helix.apache.org
[GitHub] [helix] mgao0 commented on a change in pull request #718: Implement Helix nonblocking lock
mgao0 commented on a change in pull request #718: Implement Helix nonblocking
lock
URL: https://github.com/apache/helix/pull/718#discussion_r375020573
##
File path:
helix-lock/src/main/java/org/apache/helix/lock/ZKHelixNonblockingLock.java
##
@@ -0,0 +1,159 @@
+/*
+ * 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.HashMap;
+import java.util.Map;
+import java.util.UUID;
+
+import org.apache.helix.AccessOption;
+import org.apache.helix.BaseDataAccessor;
+import org.apache.helix.ZNRecord;
+import org.apache.helix.ZNRecordUpdater;
+import org.apache.helix.manager.zk.ZNRecordSerializer;
+import org.apache.helix.manager.zk.ZkBaseDataAccessor;
+import org.apache.helix.manager.zk.ZkClient;
+import org.apache.helix.manager.zk.client.HelixZkClient;
+import org.apache.helix.model.HelixConfigScope;
+import org.apache.helix.util.ZNRecordUtil;
+import org.apache.log4j.Logger;
+import org.apache.zookeeper.data.Stat;
+
+
+/**
+ * Helix nonblocking lock implementation based on Zookeeper
+ */
+public class ZKHelixNonblockingLock implements HelixLock {
+
+ private static final Logger LOG =
Logger.getLogger(ZKHelixNonblockingLock.class);
+
+ private static final String LOCK_ROOT = "LOCKS";
+ private final String _lockPath;
+ private final String _userID;
Review comment:
The purpose of this is to let user save a user Id locally, so even they lost
session with Zk, after reconnect they will have the same user Id, so they can
still check the lock owner and know if the lock belongs to them, as well as
maintain authorizations to release their own 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:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: reviews-unsubscr...@helix.apache.org
For additional commands, e-mail: reviews-h...@helix.apache.org
[GitHub] [helix] mgao0 commented on a change in pull request #718: Implement Helix nonblocking lock
mgao0 commented on a change in pull request #718: Implement Helix nonblocking
lock
URL: https://github.com/apache/helix/pull/718#discussion_r375020033
##
File path:
helix-lock/src/main/java/org/apache/helix/lock/ZKHelixNonblockingLock.java
##
@@ -0,0 +1,159 @@
+/*
+ * 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.HashMap;
+import java.util.Map;
+import java.util.UUID;
+
+import org.apache.helix.AccessOption;
+import org.apache.helix.BaseDataAccessor;
+import org.apache.helix.ZNRecord;
+import org.apache.helix.ZNRecordUpdater;
+import org.apache.helix.manager.zk.ZNRecordSerializer;
+import org.apache.helix.manager.zk.ZkBaseDataAccessor;
+import org.apache.helix.manager.zk.ZkClient;
+import org.apache.helix.manager.zk.client.HelixZkClient;
+import org.apache.helix.model.HelixConfigScope;
+import org.apache.helix.util.ZNRecordUtil;
+import org.apache.log4j.Logger;
+import org.apache.zookeeper.data.Stat;
+
+
+/**
+ * Helix nonblocking lock implementation based on Zookeeper
+ */
+public class ZKHelixNonblockingLock implements HelixLock {
+
+ private static final Logger LOG =
Logger.getLogger(ZKHelixNonblockingLock.class);
+
+ private static final String LOCK_ROOT = "LOCKS";
+ private final String _lockPath;
+ private final String _userID;
+ private final long _timeout;
+ private final String _lockMsg;
+ private final BaseDataAccessor _baseDataAccessor;
+
+ /**
+ * Initialize the lock with user provided information, e.g.,cluster, scope,
etc.
+ * @param clusterName the cluster under which the lock will live
+ * @param scope the scope to lock
+ * @param zkAddress the zk address the cluster connects to
+ * @param timeout the timeout period of the lcok
+ * @param lockMsg the reason for having this lock
+ */
+ public ZKHelixNonblockingLock(String clusterName, HelixConfigScope scope,
String zkAddress,
+ Long timeout, String lockMsg, String userID) {
+this("/" + clusterName + '/' + LOCK_ROOT + '/' + scope, zkAddress,
timeout, lockMsg, userID);
+ }
+
+ /**
+ * Initialize the lock with user provided information, e.g., lock path under
zookeeper, etc.
+ * @param lockPath the path of the lock under Zookeeper
+ * @param zkAddress the zk address of the cluster
+ * @param timeout the timeout period of the lcok
+ * @param lockMsg the reason for having this lock
+ */
+ public ZKHelixNonblockingLock(String lockPath, String zkAddress, Long
timeout, String lockMsg,
+ String userID) {
+HelixZkClient zkClient = new ZkClient(zkAddress);
+_lockPath = lockPath;
+_timeout = timeout;
+_lockMsg = lockMsg;
+_userID = userID;
+_baseDataAccessor = new
ZkBaseDataAccessor(zkClient.getServers());
+ }
+
+ @Override
+ /**
+ * Blocking call to acquire a lock
+ * @return true if the lock was successfully acquired,
+ * false if the lock could not be acquired
+ */ public boolean acquireLock() {
+
+// Set lock information fields
+ZNRecord lockInfo = new ZNRecord(_userID);
+lockInfo.setSimpleField(ZKHelixNonblockingLockInfo.InfoKey.OWNER.name(),
_userID);
+lockInfo.setSimpleField(ZKHelixNonblockingLockInfo.InfoKey.MESSAGE.name(),
_lockMsg);
+long timeout = System.currentTimeMillis() + _timeout;
+lockInfo
+.setSimpleField(ZKHelixNonblockingLockInfo.InfoKey.TIMEOUT.name(),
String.valueOf(timeout));
+
+// Try to create the lock node
+boolean success = _baseDataAccessor.create(_lockPath, lockInfo,
AccessOption.PERSISTENT);
+
+// If fail to create the lock node (acquire the lock), compare the timeout
timestamp of current lock node with current time, if already passes the
timeout, release current lock and try to acquire the lock again
+if (!success) {
+ Stat stat = new Stat();
+ ZNRecord curLock = _baseDataAccessor.get(_lockPath, stat,
AccessOption.PERSISTENT);
+ long curTimeout =
+
Long.parseLong(curLock.getSimpleField(ZKHelixNonblockingLockInfo.InfoKey.TIMEOUT.name()));
+ if (System.currentTimeMillis() >= curTimeout) {
+success =
+_baseDataAccessor.set(_lockPath, lockInfo, stat.getVersion(),
AccessOption.PERSISTENT);
+
[GitHub] [helix] mgao0 commented on a change in pull request #718: Implement Helix nonblocking lock
mgao0 commented on a change in pull request #718: Implement Helix nonblocking
lock
URL: https://github.com/apache/helix/pull/718#discussion_r375019525
##
File path:
helix-lock/src/main/java/org/apache/helix/lock/ZKHelixNonblockingLock.java
##
@@ -0,0 +1,159 @@
+/*
+ * 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.HashMap;
+import java.util.Map;
+import java.util.UUID;
+
+import org.apache.helix.AccessOption;
+import org.apache.helix.BaseDataAccessor;
+import org.apache.helix.ZNRecord;
+import org.apache.helix.ZNRecordUpdater;
+import org.apache.helix.manager.zk.ZNRecordSerializer;
+import org.apache.helix.manager.zk.ZkBaseDataAccessor;
+import org.apache.helix.manager.zk.ZkClient;
+import org.apache.helix.manager.zk.client.HelixZkClient;
+import org.apache.helix.model.HelixConfigScope;
+import org.apache.helix.util.ZNRecordUtil;
+import org.apache.log4j.Logger;
+import org.apache.zookeeper.data.Stat;
+
+
+/**
+ * Helix nonblocking lock implementation based on Zookeeper
+ */
+public class ZKHelixNonblockingLock implements HelixLock {
+
+ private static final Logger LOG =
Logger.getLogger(ZKHelixNonblockingLock.class);
+
+ private static final String LOCK_ROOT = "LOCKS";
+ private final String _lockPath;
+ private final String _userID;
+ private final long _timeout;
+ private final String _lockMsg;
+ private final BaseDataAccessor _baseDataAccessor;
+
+ /**
+ * Initialize the lock with user provided information, e.g.,cluster, scope,
etc.
+ * @param clusterName the cluster under which the lock will live
+ * @param scope the scope to lock
+ * @param zkAddress the zk address the cluster connects to
+ * @param timeout the timeout period of the lcok
+ * @param lockMsg the reason for having this lock
+ */
+ public ZKHelixNonblockingLock(String clusterName, HelixConfigScope scope,
String zkAddress,
+ Long timeout, String lockMsg, String userID) {
+this("/" + clusterName + '/' + LOCK_ROOT + '/' + scope, zkAddress,
timeout, lockMsg, userID);
+ }
+
+ /**
+ * Initialize the lock with user provided information, e.g., lock path under
zookeeper, etc.
+ * @param lockPath the path of the lock under Zookeeper
+ * @param zkAddress the zk address of the cluster
+ * @param timeout the timeout period of the lcok
+ * @param lockMsg the reason for having this lock
+ */
+ public ZKHelixNonblockingLock(String lockPath, String zkAddress, Long
timeout, String lockMsg,
+ String userID) {
+HelixZkClient zkClient = new ZkClient(zkAddress);
+_lockPath = lockPath;
+_timeout = timeout;
+_lockMsg = lockMsg;
+_userID = userID;
+_baseDataAccessor = new
ZkBaseDataAccessor(zkClient.getServers());
+ }
+
+ @Override
+ /**
+ * Blocking call to acquire a lock
+ * @return true if the lock was successfully acquired,
+ * false if the lock could not be acquired
+ */ public boolean acquireLock() {
+
+// Set lock information fields
+ZNRecord lockInfo = new ZNRecord(_userID);
+lockInfo.setSimpleField(ZKHelixNonblockingLockInfo.InfoKey.OWNER.name(),
_userID);
+lockInfo.setSimpleField(ZKHelixNonblockingLockInfo.InfoKey.MESSAGE.name(),
_lockMsg);
+long timeout = System.currentTimeMillis() + _timeout;
+lockInfo
+.setSimpleField(ZKHelixNonblockingLockInfo.InfoKey.TIMEOUT.name(),
String.valueOf(timeout));
+
+// Try to create the lock node
+boolean success = _baseDataAccessor.create(_lockPath, lockInfo,
AccessOption.PERSISTENT);
+
+// If fail to create the lock node (acquire the lock), compare the timeout
timestamp of current lock node with current time, if already passes the
timeout, release current lock and try to acquire the lock again
+if (!success) {
+ Stat stat = new Stat();
+ ZNRecord curLock = _baseDataAccessor.get(_lockPath, stat,
AccessOption.PERSISTENT);
+ long curTimeout =
+
Long.parseLong(curLock.getSimpleField(ZKHelixNonblockingLockInfo.InfoKey.TIMEOUT.name()));
+ if (System.currentTimeMillis() >= curTimeout) {
+success =
+_baseDataAccessor.set(_lockPath, lockInfo, stat.getVersion(),
AccessOption.PERSISTENT);
+
[GitHub] [helix] mgao0 commented on a change in pull request #718: Implement Helix nonblocking lock
mgao0 commented on a change in pull request #718: Implement Helix nonblocking lock URL: https://github.com/apache/helix/pull/718#discussion_r375018986 ## File path: helix-lock/src/test/java/org/apache/helix/lock/TestZKHelixNonblockingLock.java ## @@ -0,0 +1,176 @@ +/* Review comment: Test added. Please see testSimultaneousAcquire(). 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: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: reviews-unsubscr...@helix.apache.org For additional commands, e-mail: reviews-h...@helix.apache.org
[GitHub] [helix] mgao0 commented on a change in pull request #718: Implement Helix nonblocking lock
mgao0 commented on a change in pull request #718: Implement Helix nonblocking
lock
URL: https://github.com/apache/helix/pull/718#discussion_r375001296
##
File path:
helix-lock/src/main/java/org/apache/helix/lock/ZKHelixNonblockingLock.java
##
@@ -0,0 +1,159 @@
+/*
+ * 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.HashMap;
+import java.util.Map;
+import java.util.UUID;
+
+import org.apache.helix.AccessOption;
+import org.apache.helix.BaseDataAccessor;
+import org.apache.helix.ZNRecord;
+import org.apache.helix.ZNRecordUpdater;
+import org.apache.helix.manager.zk.ZNRecordSerializer;
+import org.apache.helix.manager.zk.ZkBaseDataAccessor;
+import org.apache.helix.manager.zk.ZkClient;
+import org.apache.helix.manager.zk.client.HelixZkClient;
+import org.apache.helix.model.HelixConfigScope;
+import org.apache.helix.util.ZNRecordUtil;
+import org.apache.log4j.Logger;
+import org.apache.zookeeper.data.Stat;
+
+
+/**
+ * Helix nonblocking lock implementation based on Zookeeper
+ */
+public class ZKHelixNonblockingLock implements HelixLock {
+
+ private static final Logger LOG =
Logger.getLogger(ZKHelixNonblockingLock.class);
+
+ private static final String LOCK_ROOT = "LOCKS";
+ private final String _lockPath;
+ private final String _userID;
+ private final long _timeout;
+ private final String _lockMsg;
+ private final BaseDataAccessor _baseDataAccessor;
+
+ /**
+ * Initialize the lock with user provided information, e.g.,cluster, scope,
etc.
+ * @param clusterName the cluster under which the lock will live
+ * @param scope the scope to lock
+ * @param zkAddress the zk address the cluster connects to
+ * @param timeout the timeout period of the lcok
+ * @param lockMsg the reason for having this lock
+ */
+ public ZKHelixNonblockingLock(String clusterName, HelixConfigScope scope,
String zkAddress,
+ Long timeout, String lockMsg, String userID) {
+this("/" + clusterName + '/' + LOCK_ROOT + '/' + scope, zkAddress,
timeout, lockMsg, userID);
+ }
+
+ /**
+ * Initialize the lock with user provided information, e.g., lock path under
zookeeper, etc.
+ * @param lockPath the path of the lock under Zookeeper
+ * @param zkAddress the zk address of the cluster
+ * @param timeout the timeout period of the lcok
+ * @param lockMsg the reason for having this lock
+ */
+ public ZKHelixNonblockingLock(String lockPath, String zkAddress, Long
timeout, String lockMsg,
+ String userID) {
+HelixZkClient zkClient = new ZkClient(zkAddress);
+_lockPath = lockPath;
+_timeout = timeout;
+_lockMsg = lockMsg;
+_userID = userID;
+_baseDataAccessor = new
ZkBaseDataAccessor(zkClient.getServers());
+ }
+
+ @Override
+ /**
+ * Blocking call to acquire a lock
+ * @return true if the lock was successfully acquired,
+ * false if the lock could not be acquired
+ */ public boolean acquireLock() {
Review comment:
This is dangling comment, I deleted it since there is already a same comment
in the interface.
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:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: reviews-unsubscr...@helix.apache.org
For additional commands, e-mail: reviews-h...@helix.apache.org
[GitHub] [helix] mgao0 commented on a change in pull request #718: Implement Helix nonblocking lock
mgao0 commented on a change in pull request #718: Implement Helix nonblocking
lock
URL: https://github.com/apache/helix/pull/718#discussion_r374999880
##
File path:
helix-lock/src/main/java/org/apache/helix/lock/ZKHelixNonblockingLock.java
##
@@ -0,0 +1,151 @@
+/*
+ * 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.HashMap;
+import java.util.Map;
+import java.util.UUID;
+
+import org.apache.helix.AccessOption;
+import org.apache.helix.BaseDataAccessor;
+import org.apache.helix.ZNRecord;
+import org.apache.helix.ZNRecordUpdater;
+import org.apache.helix.manager.zk.ZNRecordSerializer;
+import org.apache.helix.manager.zk.ZkBaseDataAccessor;
+import org.apache.helix.manager.zk.ZkClient;
+import org.apache.helix.manager.zk.client.HelixZkClient;
+import org.apache.helix.model.HelixConfigScope;
+import org.apache.helix.util.ZNRecordUtil;
+import org.apache.log4j.Logger;
+import org.apache.zookeeper.data.Stat;
+
+
+/**
+ * Helix nonblocking lock implementation based on Zookeeper
+ */
+public class ZKHelixNonblockingLock implements HelixLock {
+
+ private static final Logger LOG =
Logger.getLogger(ZKHelixNonblockingLock.class);
+
+ private static final String LOCK_ROOT = "LOCKS";
+ private final String _lockPath;
+ private final String _userId;
+ private final long _timeout;
+ private final String _lockMsg;
+ private final BaseDataAccessor _baseDataAccessor;
+
+ /**
+ * Initialize the lock with user provided information, e.g.,cluster, scope,
etc.
+ * @param clusterName the cluster under which the lock will live
+ * @param scope the scope to lock
+ * @param zkAddress the zk address the cluster connects to
+ * @param timeout the timeout period of the lcok
+ * @param lockMsg the reason for having this lock
+ * @param userId a universal unique userId for lock owner identity
+ */
+ public ZKHelixNonblockingLock(String clusterName, HelixConfigScope scope,
String zkAddress,
+ Long timeout, String lockMsg, String userId) {
+this("/" + clusterName + '/' + LOCK_ROOT + '/' + scope, zkAddress,
timeout, lockMsg, userId);
+ }
+
+ /**
+ * Initialize the lock with user provided information, e.g., lock path under
zookeeper, etc.
+ * @param lockPath the path of the lock under Zookeeper
+ * @param zkAddress the zk address of the cluster
+ * @param timeout the timeout period of the lcok
+ * @param lockMsg the reason for having this lock
+ * @param userId a universal unique userId for lock owner identity
+ */
+ public ZKHelixNonblockingLock(String lockPath, String zkAddress, Long
timeout, String lockMsg,
+ String userId) {
+HelixZkClient zkClient = new ZkClient(zkAddress);
+_lockPath = lockPath;
+_timeout = timeout;
+_lockMsg = lockMsg;
+if
(userId.matches("[a-f0-9]{8}-[a-f0-9]{4}-4[0-9]{3}-[89ab][a-f0-9]{3}-[0-9a-f]{12}"))
{
+ _userId = userId;
+} else {
+ throw new IllegalArgumentException();
+}
+_baseDataAccessor = new
ZkBaseDataAccessor(zkClient.getServers());
+ }
+
+ @Override
+ public boolean acquireLock() {
+
+// Set lock information fields
+ZNRecord lockInfo = new ZNRecord(_userId);
+lockInfo.setSimpleField(ZKHelixNonblockingLockInfo.InfoKey.OWNER.name(),
_userId);
+lockInfo.setSimpleField(ZKHelixNonblockingLockInfo.InfoKey.MESSAGE.name(),
_lockMsg);
+long timeout = System.currentTimeMillis() + _timeout;
+lockInfo
+.setSimpleField(ZKHelixNonblockingLockInfo.InfoKey.TIMEOUT.name(),
String.valueOf(timeout));
+
+// Try to create the lock node
+boolean success = _baseDataAccessor.create(_lockPath, lockInfo,
AccessOption.PERSISTENT);
+
+// If fail to create the lock node (acquire the lock), compare the timeout
timestamp of current lock node with current time, if already passes the
timeout, release current lock and try to acquire the lock again
+if (!success) {
+ Stat stat = new Stat();
+ ZNRecord curLock = _baseDataAccessor.get(_lockPath, stat,
AccessOption.PERSISTENT);
+ long curTimeout =
+
Long.parseLong(curLock.getSimpleField(ZKHelixNonblockingLockInfo.InfoKey.TIMEOUT.name()));
Review comment:
It is
