[GitHub] [helix] dasahcc commented on a change in pull request #718: Implement Helix nonblocking lock
dasahcc commented on a change in pull request #718: Implement Helix nonblocking
lock
URL: https://github.com/apache/helix/pull/718#discussion_r381032011
##
File path: helix-lock/src/main/java/org/apache/helix/lock/LockInfo.java
##
@@ -19,28 +19,106 @@
package org.apache.helix.lock;
-import java.util.Map;
+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 {
- //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(DEFAULT_OWNER_TEXT, DEFAULT_MESSAGE_TEXT,
DEFAULT_TIMEOUT_LONG);
+
+ private ZNRecord record;
+
+ /**
+ * The keys to lock information
+ */
+ public enum LockInfoAttribute {
+OWNER, MESSAGE, TIMEOUT
+ }
+
+ /**
+ * Initialize a default LockInfo instance
+ */
+ private LockInfo() {
+record = new ZNRecord("LOCK");
Review comment:
Make "LOCK" to be a constant instead of hard coding here.
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] dasahcc commented on a change in pull request #718: Implement Helix nonblocking lock
dasahcc commented on a change in pull request #718: Implement Helix nonblocking
lock
URL: https://github.com/apache/helix/pull/718#discussion_r381032659
##
File path: helix-lock/src/main/java/org/apache/helix/lock/LockInfo.java
##
@@ -19,28 +19,106 @@
package org.apache.helix.lock;
-import java.util.Map;
+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 {
- //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(DEFAULT_OWNER_TEXT, DEFAULT_MESSAGE_TEXT,
DEFAULT_TIMEOUT_LONG);
+
+ private ZNRecord record;
Review comment:
naming convention: _record;
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] dasahcc commented on a change in pull request #718: Implement Helix nonblocking lock
dasahcc commented on a change in pull request #718: Implement Helix nonblocking
lock
URL: https://github.com/apache/helix/pull/718#discussion_r381031816
##
File path: helix-lock/src/main/java/org/apache/helix/lock/HelixLock.java
##
@@ -22,7 +22,7 @@
/**
* Generic interface for Helix distributed lock
*/
-public interface HelixLock {
+public interface HelixLock {
Review comment:
Let's have the interface as DistributedLock or something. Name it HelixLock
still seemed to be sticking to Helix.
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] dasahcc commented on a change in pull request #718: Implement Helix nonblocking lock
dasahcc commented on a change in pull request #718: Implement Helix nonblocking
lock
URL: https://github.com/apache/helix/pull/718#discussion_r381031871
##
File path: helix-lock/src/main/java/org/apache/helix/lock/LockInfo.java
##
@@ -19,28 +19,106 @@
package org.apache.helix.lock;
-import java.util.Map;
+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 {
- //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(DEFAULT_OWNER_TEXT, DEFAULT_MESSAGE_TEXT,
DEFAULT_TIMEOUT_LONG);
+
+ private ZNRecord record;
+
+ /**
+ * The keys to lock information
+ */
+ public enum LockInfoAttribute {
+OWNER, MESSAGE, TIMEOUT
Review comment:
Make one element per line.
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] dasahcc commented on a change in pull request #718: Implement Helix nonblocking lock
dasahcc commented on a change in pull request #718: Implement Helix nonblocking
lock
URL: https://github.com/apache/helix/pull/718#discussion_r381031248
##
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:
Sounds good.
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] dasahcc commented on a change in pull request #718: Implement Helix nonblocking lock
dasahcc commented on a change in pull request #718: Implement Helix nonblocking
lock
URL: https://github.com/apache/helix/pull/718#discussion_r380915772
##
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:
Put a TODO here. This Helix Property will be replaced since distributed lock
will be a module independent from Helix.
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] dasahcc commented on a change in pull request #718: Implement Helix nonblocking lock
dasahcc commented on a change in pull request #718: Implement Helix nonblocking
lock
URL: https://github.com/apache/helix/pull/718#discussion_r380916833
##
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:
What if the path does not exists?
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] dasahcc commented on a change in pull request #718: Implement Helix nonblocking lock
dasahcc commented on a change in pull request #718: Implement Helix nonblocking
lock
URL: https://github.com/apache/helix/pull/718#discussion_r380916271
##
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:
Why we dont delete the node instead of updating it?
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] dasahcc commented on a change in pull request #718: Implement Helix nonblocking lock
dasahcc commented on a change in pull request #718: Implement Helix nonblocking
lock
URL: https://github.com/apache/helix/pull/718#discussion_r374901334
##
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:
Is
[GitHub] [helix] dasahcc commented on a change in pull request #718: Implement Helix nonblocking lock
dasahcc commented on a change in pull request #718: Implement Helix nonblocking
lock
URL: https://github.com/apache/helix/pull/718#discussion_r374900908
##
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();
+}
Review comment:
you can refactor code like:
if (!userId.matches(...)) {
throw new IllegalArgumentException();
}
_userId = userId;
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] dasahcc commented on a change in pull request #718: Implement Helix nonblocking lock
dasahcc commented on a change in pull request #718: Implement Helix nonblocking
lock
URL: https://github.com/apache/helix/pull/718#discussion_r374900480
##
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:
Do not hard code it. Make it as a constant.
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] dasahcc commented on a change in pull request #718: Implement Helix nonblocking lock
dasahcc commented on a change in pull request #718: Implement Helix nonblocking
lock
URL: https://github.com/apache/helix/pull/718#discussion_r374902436
##
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:
If you dont need to return Stat data, then you can pass null as argument. If
you need it, set it in the ZNRecord.
