You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@helix.apache.org by jx...@apache.org on 2020/04/14 02:30:13 UTC

[helix] branch distributed-lock updated (d6ba1f3 -> c11ac95)

This is an automated email from the ASF dual-hosted git repository.

jxue pushed a change to branch distributed-lock
in repository https://gitbox.apache.org/repos/asf/helix.git.


    from d6ba1f3  Use ZnRecordSerializer in ConfigAccessor (#666)
     new ec6bd29  Add Helix Distributed lock module (#673)
     new 5bea978  Created Helix distributed lock interface (#703)
     new f0a3bb2  Created Helix distributed lock design (apache#702)
     new df5b89c  Added LockInfo interface
     new e01c8d5  Added details in comments
     new 86c4b30  created Helix nonblocking lock based on zk
     new d58fbe6  Added unit tests for Helix nonblocking lock
     new 32f3b86  Fixed logic of release and isOwner
     new ba3bec9  Added test to acquire lock simultaneously
     new 6868eb6  A few fixes on syntax
     new 33bceca  Changed method doc for releaseLock in HelixLock interface
     new 1038473  Fixed lock path generation
     new f1d7b31  simplified acquireLock logic
     new 2a7d5ee  Added cluster level to HelixLockScope and convert lock path to uppercase
     new ed3b444  refactor LockInfo and some updates on the HelixLockScope
     new 62a0c4b  Created LockScope interface
     new 5984267  Clean up code
     new 1dc7e59  Remove dependency of LockInfo on HelixProperty
     new c11ac95  Rename interface HelixLock to DistributedLock

The 19 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 bump-up.command                                    |  17 +++
 {helix-rest => helix-lock}/LICENSE                 |   0
 {helix-rest => helix-lock}/NOTICE                  |   0
 .../helix-lock-0.9.2-SNAPSHOT.ivy                  |   9 +-
 {helix-agent => helix-lock}/pom.xml                |  67 ++++-----
 .../src/assemble/assembly.xml                      |   6 +-
 .../src/main/config/log4j.properties               |   4 +-
 .../org/apache/helix/lock/DistributedLock.java     |  52 +++++++
 .../main/java/org/apache/helix/lock/LockInfo.java  | 131 +++++++++++++++++
 .../main/java/org/apache/helix/lock/LockScope.java |  16 +-
 .../apache/helix/lock/helix/HelixLockScope.java    | 100 +++++++++++++
 .../lock/helix/ZKDistributedNonblockingLock.java   | 147 +++++++++++++++++++
 .../src/test/conf/testng.xml                       |   2 +-
 .../lock/helix/TestZKHelixNonblockingLock.java     | 163 +++++++++++++++++++++
 .../src/test/resources/log4j.properties            |   0
 pom.xml                                            |   1 +
 16 files changed, 664 insertions(+), 51 deletions(-)
 copy {helix-rest => helix-lock}/LICENSE (100%)
 copy {helix-rest => helix-lock}/NOTICE (100%)
 copy helix-rest/helix-rest-0.9.2-SNAPSHOT.ivy => helix-lock/helix-lock-0.9.2-SNAPSHOT.ivy (85%)
 copy {helix-agent => helix-lock}/pom.xml (69%)
 copy {helix-rest => helix-lock}/src/assemble/assembly.xml (91%)
 copy {helix-rest => helix-lock}/src/main/config/log4j.properties (92%)
 create mode 100644 helix-lock/src/main/java/org/apache/helix/lock/DistributedLock.java
 create mode 100644 helix-lock/src/main/java/org/apache/helix/lock/LockInfo.java
 copy helix-core/src/main/java/org/apache/helix/controller/stages/IntermediateStateOutput.java => helix-lock/src/main/java/org/apache/helix/lock/LockScope.java (74%)
 create mode 100644 helix-lock/src/main/java/org/apache/helix/lock/helix/HelixLockScope.java
 create mode 100644 helix-lock/src/main/java/org/apache/helix/lock/helix/ZKDistributedNonblockingLock.java
 copy {helix-rest => helix-lock}/src/test/conf/testng.xml (95%)
 create mode 100644 helix-lock/src/test/java/org/apache/helix/lock/helix/TestZKHelixNonblockingLock.java
 copy {helix-rest => helix-lock}/src/test/resources/log4j.properties (100%)


[helix] 01/19: Add Helix Distributed lock module (#673)

Posted by jx...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

jxue pushed a commit to branch distributed-lock
in repository https://gitbox.apache.org/repos/asf/helix.git

commit ec6bd2982124a2f9456df34cd4940ac161294b30
Author: zhangmeng916 <56...@users.noreply.github.com>
AuthorDate: Tue Jan 14 17:11:25 2020 -0800

    Add Helix Distributed lock module (#673)
    
    Add Helix Lock module.
---
 bump-up.command                                |  17 ++
 helix-lock/LICENSE                             | 273 +++++++++++++++++++++++++
 helix-lock/NOTICE                              |  37 ++++
 helix-lock/helix-lock-0.9.2-SNAPSHOT.ivy       |  51 +++++
 helix-lock/pom.xml                             | 112 ++++++++++
 helix-lock/src/assemble/assembly.xml           |  60 ++++++
 helix-lock/src/main/config/log4j.properties    |  32 +++
 helix-lock/src/test/conf/testng.xml            |  27 +++
 helix-lock/src/test/resources/log4j.properties |  41 ++++
 pom.xml                                        |   1 +
 10 files changed, 651 insertions(+)

diff --git a/bump-up.command b/bump-up.command
index d67616c..cb2a4b5 100755
--- a/bump-up.command
+++ b/bump-up.command
@@ -92,6 +92,23 @@ else
   echo "helix-rest/$ivy_file not exist"
 fi
 
+echo "bump up helix-lock/pom.xml"
+sed -i "s/${version}/${new_version}/g" helix-lock/pom.xml
+grep -C 1 "$new_version" helix-lock/pom.xml
+# git diff helix-lock/pom.xml
+
+ivy_file="helix-lock-"$version".ivy"
+new_ivy_file="helix-lock-"$new_version".ivy"
+# echo "$ivy_file"
+if [ -f helix-lock/$ivy_file ]; then
+  echo "bump up helix-lock/$ivy_file"
+  git mv "helix-lock/$ivy_file" "helix-lock/$new_ivy_file"
+  sed -i "s/${version}/${new_version}/g" "helix-lock/$new_ivy_file"
+  grep -C 1 "$new_version" "helix-lock/$new_ivy_file"
+else
+  echo "helix-lock/$ivy_file not exist"
+fi
+
 echo "bump up helix-agent/pom.xml"
 sed -i "s/${version}/${new_version}/g" helix-agent/pom.xml
 grep -C 1 "$new_version" helix-agent/pom.xml
diff --git a/helix-lock/LICENSE b/helix-lock/LICENSE
new file mode 100644
index 0000000..413913f
--- /dev/null
+++ b/helix-lock/LICENSE
@@ -0,0 +1,273 @@
+
+                                 Apache License
+                           Version 2.0, January 2004
+                        http://www.apache.org/licenses/
+
+   TERMS AND CONDITIONS FOR USE, REPRODUCTION, AND DISTRIBUTION
+
+   1. Definitions.
+
+      "License" shall mean the terms and conditions for use, reproduction,
+      and distribution as defined by Sections 1 through 9 of this document.
+
+      "Licensor" shall mean the copyright owner or entity authorized by
+      the copyright owner that is granting the License.
+
+      "Legal Entity" shall mean the union of the acting entity and all
+      other entities that control, are controlled by, or are under common
+      control with that entity. For the purposes of this definition,
+      "control" means (i) the power, direct or indirect, to cause the
+      direction or management of such entity, whether by contract or
+      otherwise, or (ii) ownership of fifty percent (50%) or more of the
+      outstanding shares, or (iii) beneficial ownership of such entity.
+
+      "You" (or "Your") shall mean an individual or Legal Entity
+      exercising permissions granted by this License.
+
+      "Source" form shall mean the preferred form for making modifications,
+      including but not limited to software source code, documentation
+      source, and configuration files.
+
+      "Object" form shall mean any form resulting from mechanical
+      transformation or translation of a Source form, including but
+      not limited to compiled object code, generated documentation,
+      and conversions to other media types.
+
+      "Work" shall mean the work of authorship, whether in Source or
+      Object form, made available under the License, as indicated by a
+      copyright notice that is included in or attached to the work
+      (an example is provided in the Appendix below).
+
+      "Derivative Works" shall mean any work, whether in Source or Object
+      form, that is based on (or derived from) the Work and for which the
+      editorial revisions, annotations, elaborations, or other modifications
+      represent, as a whole, an original work of authorship. For the purposes
+      of this License, Derivative Works shall not include works that remain
+      separable from, or merely link (or bind by name) to the interfaces of,
+      the Work and Derivative Works thereof.
+
+      "Contribution" shall mean any work of authorship, including
+      the original version of the Work and any modifications or additions
+      to that Work or Derivative Works thereof, that is intentionally
+      submitted to Licensor for inclusion in the Work by the copyright owner
+      or by an individual or Legal Entity authorized to submit on behalf of
+      the copyright owner. For the purposes of this definition, "submitted"
+      means any form of electronic, verbal, or written communication sent
+      to the Licensor or its representatives, including but not limited to
+      communication on electronic mailing lists, source code control systems,
+      and issue tracking systems that are managed by, or on behalf of, the
+      Licensor for the purpose of discussing and improving the Work, but
+      excluding communication that is conspicuously marked or otherwise
+      designated in writing by the copyright owner as "Not a Contribution."
+
+      "Contributor" shall mean Licensor and any individual or Legal Entity
+      on behalf of whom a Contribution has been received by Licensor and
+      subsequently incorporated within the Work.
+
+   2. Grant of Copyright License. Subject to the terms and conditions of
+      this License, each Contributor hereby grants to You a perpetual,
+      worldwide, non-exclusive, no-charge, royalty-free, irrevocable
+      copyright license to reproduce, prepare Derivative Works of,
+      publicly display, publicly perform, sublicense, and distribute the
+      Work and such Derivative Works in Source or Object form.
+
+   3. Grant of Patent License. Subject to the terms and conditions of
+      this License, each Contributor hereby grants to You a perpetual,
+      worldwide, non-exclusive, no-charge, royalty-free, irrevocable
+      (except as stated in this section) patent license to make, have made,
+      use, offer to sell, sell, import, and otherwise transfer the Work,
+      where such license applies only to those patent claims licensable
+      by such Contributor that are necessarily infringed by their
+      Contribution(s) alone or by combination of their Contribution(s)
+      with the Work to which such Contribution(s) was submitted. If You
+      institute patent litigation against any entity (including a
+      cross-claim or counterclaim in a lawsuit) alleging that the Work
+      or a Contribution incorporated within the Work constitutes direct
+      or contributory patent infringement, then any patent licenses
+      granted to You under this License for that Work shall terminate
+      as of the date such litigation is filed.
+
+   4. Redistribution. You may reproduce and distribute copies of the
+      Work or Derivative Works thereof in any medium, with or without
+      modifications, and in Source or Object form, provided that You
+      meet the following conditions:
+
+      (a) You must give any other recipients of the Work or
+          Derivative Works a copy of this License; and
+
+      (b) You must cause any modified files to carry prominent notices
+          stating that You changed the files; and
+
+      (c) You must retain, in the Source form of any Derivative Works
+          that You distribute, all copyright, patent, trademark, and
+          attribution notices from the Source form of the Work,
+          excluding those notices that do not pertain to any part of
+          the Derivative Works; and
+
+      (d) If the Work includes a "NOTICE" text file as part of its
+          distribution, then any Derivative Works that You distribute must
+          include a readable copy of the attribution notices contained
+          within such NOTICE file, excluding those notices that do not
+          pertain to any part of the Derivative Works, in at least one
+          of the following places: within a NOTICE text file distributed
+          as part of the Derivative Works; within the Source form or
+          documentation, if provided along with the Derivative Works; or,
+          within a display generated by the Derivative Works, if and
+          wherever such third-party notices normally appear. The contents
+          of the NOTICE file are for informational purposes only and
+          do not modify the License. You may add Your own attribution
+          notices within Derivative Works that You distribute, alongside
+          or as an addendum to the NOTICE text from the Work, provided
+          that such additional attribution notices cannot be construed
+          as modifying the License.
+
+      You may add Your own copyright statement to Your modifications and
+      may provide additional or different license terms and conditions
+      for use, reproduction, or distribution of Your modifications, or
+      for any such Derivative Works as a whole, provided Your use,
+      reproduction, and distribution of the Work otherwise complies with
+      the conditions stated in this License.
+
+   5. Submission of Contributions. Unless You explicitly state otherwise,
+      any Contribution intentionally submitted for inclusion in the Work
+      by You to the Licensor shall be under the terms and conditions of
+      this License, without any additional terms or conditions.
+      Notwithstanding the above, nothing herein shall supersede or modify
+      the terms of any separate license agreement you may have executed
+      with Licensor regarding such Contributions.
+
+   6. Trademarks. This License does not grant permission to use the trade
+      names, trademarks, service marks, or product names of the Licensor,
+      except as required for reasonable and customary use in describing the
+      origin of the Work and reproducing the content of the NOTICE file.
+
+   7. Disclaimer of Warranty. Unless required by applicable law or
+      agreed to in writing, Licensor provides the Work (and each
+      Contributor provides its Contributions) on an "AS IS" BASIS,
+      WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
+      implied, including, without limitation, any warranties or conditions
+      of TITLE, NON-INFRINGEMENT, MERCHANTABILITY, or FITNESS FOR A
+      PARTICULAR PURPOSE. You are solely responsible for determining the
+      appropriateness of using or redistributing the Work and assume any
+      risks associated with Your exercise of permissions under this License.
+
+   8. Limitation of Liability. In no event and under no legal theory,
+      whether in tort (including negligence), contract, or otherwise,
+      unless required by applicable law (such as deliberate and grossly
+      negligent acts) or agreed to in writing, shall any Contributor be
+      liable to You for damages, including any direct, indirect, special,
+      incidental, or consequential damages of any character arising as a
+      result of this License or out of the use or inability to use the
+      Work (including but not limited to damages for loss of goodwill,
+      work stoppage, computer failure or malfunction, or any and all
+      other commercial damages or losses), even if such Contributor
+      has been advised of the possibility of such damages.
+
+   9. Accepting Warranty or Additional Liability. While redistributing
+      the Work or Derivative Works thereof, You may choose to offer,
+      and charge a fee for, acceptance of support, warranty, indemnity,
+      or other liability obligations and/or rights consistent with this
+      License. However, in accepting such obligations, You may act only
+      on Your own behalf and on Your sole responsibility, not on behalf
+      of any other Contributor, and only if You agree to indemnify,
+      defend, and hold each Contributor harmless for any liability
+      incurred by, or claims asserted against, such Contributor by reason
+      of your accepting any such warranty or additional liability.
+
+   END OF TERMS AND CONDITIONS
+
+   APPENDIX: How to apply the Apache License to your work.
+
+      To apply the Apache License to your work, attach the following
+      boilerplate notice, with the fields enclosed by brackets "[]"
+      replaced with your own identifying information. (Don't include
+      the brackets!)  The text should be enclosed in the appropriate
+      comment syntax for the file format. We also recommend that a
+      file or class name and description of purpose be included on the
+      same "printed page" as the copyright notice for easier
+      identification within third-party archives.
+
+   Copyright [yyyy] [name of copyright owner]
+
+   Licensed 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.
+
+
+
+For xstream:
+
+Copyright (c) 2003-2006, Joe Walnes
+Copyright (c) 2006-2009, 2011 XStream Committers
+All rights reserved.
+
+Redistribution and use in source and binary forms, with or without
+modification, are permitted provided that the following conditions are met:
+
+1. Redistributions of source code must retain the above copyright notice, this list of
+conditions and the following disclaimer.
+
+2. Redistributions in binary form must reproduce the above copyright notice, this list of
+conditions and the following disclaimer in the documentation and/or other materials provided
+with the distribution.
+
+3. Neither the name of XStream nor the names of its contributors may be used to endorse
+or promote products derived from this software without specific prior written
+permission.
+
+THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND ANY
+EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT
+SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
+INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED
+TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
+BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY
+WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH
+DAMAGE.
+
+for jline:
+
+Copyright (c) 2002-2006, Marc Prud'hommeaux <mw...@cornell.edu>
+All rights reserved.
+
+Redistribution and use in source and binary forms, with or
+without modification, are permitted provided that the following
+conditions are met:
+
+Redistributions of source code must retain the above copyright
+notice, this list of conditions and the following disclaimer.
+
+Redistributions in binary form must reproduce the above copyright
+notice, this list of conditions and the following disclaimer
+in the documentation and/or other materials provided with
+the distribution.
+
+Neither the name of JLine nor the names of its contributors
+may be used to endorse or promote products derived from this
+software without specific prior written permission.
+
+THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+"AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING,
+BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY
+AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO
+EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE
+FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY,
+OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
+AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING
+IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED
+OF THE POSSIBILITY OF SUCH DAMAGE.
+
+
+
diff --git a/helix-lock/NOTICE b/helix-lock/NOTICE
new file mode 100644
index 0000000..9dc340b
--- /dev/null
+++ b/helix-lock/NOTICE
@@ -0,0 +1,37 @@
+Apache Helix
+Copyright 2014 The Apache Software Foundation
+
+
+I. Included Software
+
+This product includes software developed at
+The Apache Software Foundation (http://www.apache.org/).
+Licensed under the Apache License 2.0.
+
+This product includes software developed at
+Codehaus (http://www.codehaus.org/).
+Licensed under the BSD License.
+
+This product includes software developed at
+jline (http://jline.sourceforge.net/).
+Licensed under the BSD License.
+
+This product includes software developed at
+restlet (http://www.restlet.org/about/legal).
+Licensed under the Apache License 2.0.
+
+This product includes software developed at
+Google (http://www.google.com/).
+Licensed under the Apache License 2.0.
+
+This product includes software developed at
+snakeyaml (http://www.snakeyaml.org/).
+Licensed under the Apache License 2.0.
+
+This product includes software developed at
+zkclient (https://github.com/sgroschupf/zkclient).
+Licensed under the Apache License 2.0.
+
+II. License Summary
+- Apache License 2.0
+- BSD License
diff --git a/helix-lock/helix-lock-0.9.2-SNAPSHOT.ivy b/helix-lock/helix-lock-0.9.2-SNAPSHOT.ivy
new file mode 100644
index 0000000..1ec1822
--- /dev/null
+++ b/helix-lock/helix-lock-0.9.2-SNAPSHOT.ivy
@@ -0,0 +1,51 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+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.
+-->
+<ivy-module version="1.0">
+	<info organisation="org.apache.helix"
+		module="helix-lock"
+		revision="0.9.2-SNAPSHOT"
+		status="integration"
+		publication="20200114115923"
+	/>
+	<configurations>
+		<conf name="default" visibility="public" description="runtime dependencies and master artifact can be used with this conf" extends="runtime,master"/>
+		<conf name="master" visibility="public" description="contains only the artifact published by this module itself, with no transitive dependencies"/>
+		<conf name="compile" visibility="public" description="this is the default scope, used if none is specified. Compile dependencies are available in all classpaths."/>
+		<conf name="provided" visibility="public" description="this is much like compile, but indicates you expect the JDK or a container to provide it. It is only available on the compilation classpath, and is not transitive."/>
+		<conf name="runtime" visibility="public" description="this scope indicates that the dependency is not required for compilation, but is for execution. It is in the runtime and test classpaths, but not the compile classpath." extends="compile"/>
+		<conf name="test" visibility="private" description="this scope indicates that the dependency is not required for normal use of the application, and is only available for the test compilation and execution phases."/>
+		<conf name="system" visibility="public" description="this scope is similar to provided except that you have to provide the JAR which contains it explicitly. The artifact is always available and is not looked up in a repository."/>
+	</configurations>
+	<publications>
+		<artifact name="helix-lock" type="jar" ext="jar" conf="master"/>
+	</publications>
+	<dependencies>
+	  <dependency org="org.slf4j" name="slf4j-api" rev="1.7.25" force="true" conf="compile->compile(*),master(*);runtime->runtime(*)">
+        <artifact name="slf4j-api" ext="jar"/>
+    </dependency>
+    <dependency org="org.slf4j" name="slf4j-log4j12" rev="1.7.14" force="true" conf="compile->compile(*),master(*);runtime->runtime(*)">
+        <artifact name="slf4j-log4j12" ext="jar"/>
+    </dependency>
+    <dependency org="org.yaml" name="snakeyaml" rev="1.17">
+        <artifact name="snakeyaml" m:classifier="sources" ext="jar"/>
+    </dependency>
+		<dependency org="org.apache.helix" name="helix-core" rev="0.9.2-SNAPSHOT" force="true" conf="compile->compile(*),master(*);runtime->runtime(*)"/>
+	</dependencies>
+</ivy-module>
diff --git a/helix-lock/pom.xml b/helix-lock/pom.xml
new file mode 100644
index 0000000..e36970e
--- /dev/null
+++ b/helix-lock/pom.xml
@@ -0,0 +1,112 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+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.
+-->
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+  <parent>
+    <groupId>org.apache.helix</groupId>
+    <artifactId>helix</artifactId>
+    <version>0.9.2-SNAPSHOT</version>
+  </parent>
+  <modelVersion>4.0.0</modelVersion>
+
+  <artifactId>helix-lock</artifactId>
+  <packaging>bundle</packaging>
+  <name>Apache Helix :: Distributed Lock</name>
+
+  <properties>
+    <osgi.import>
+      org.apache.helix*,
+      org.slf4j*;version="[1.6,2)",
+      *
+    </osgi.import>
+    <osgi.export>org.apache.helix.lock*;version="${project.version};-noimport:=true</osgi.export>
+  </properties>
+
+  <dependencies>
+    <dependency>
+      <groupId>org.yaml</groupId>
+      <artifactId>snakeyaml</artifactId>
+      <version>1.17</version>
+    </dependency>
+    <dependency>
+      <groupId>org.apache.helix</groupId>
+      <artifactId>helix-core</artifactId>
+    </dependency>
+    <dependency>
+      <groupId>org.apache.commons</groupId>
+      <artifactId>commons-lang3</artifactId>
+      <version>3.8.1</version>
+    </dependency>
+    <dependency>
+      <groupId>org.apache.httpcomponents</groupId>
+      <artifactId>httpclient</artifactId>
+      <version>4.5.8</version>
+    </dependency>
+    <dependency>
+      <groupId>com.thoughtworks.xstream</groupId>
+      <artifactId>xstream</artifactId>
+      <version>1.3.1</version>
+    </dependency>
+    <dependency>
+      <groupId>org.testng</groupId>
+      <artifactId>testng</artifactId>
+      <scope>test</scope>
+    </dependency>
+    <dependency>
+      <groupId>org.apache.helix</groupId>
+      <artifactId>helix-core</artifactId>
+      <type>test-jar</type>
+      <scope>test</scope>
+    </dependency>
+    <dependency>
+      <groupId>org.mockito</groupId>
+      <artifactId>mockito-all</artifactId>
+      <scope>test</scope>
+    </dependency>
+  </dependencies>
+  <build>
+    <resources>
+      <resource>
+        <directory>${basedir}</directory>
+        <includes>
+          <include>DISCLAIMER</include>
+        </includes>
+      </resource>
+    </resources>
+    <plugins>
+      <plugin>
+        <groupId>org.apache.maven.plugins</groupId>
+        <artifactId>maven-assembly-plugin</artifactId>
+        <configuration>
+          <descriptors>
+            <descriptor>src/assemble/assembly.xml</descriptor>
+          </descriptors>
+        </configuration>
+        <executions>
+          <execution>
+            <phase>package</phase>
+            <goals>
+              <goal>single</goal>
+            </goals>
+          </execution>
+        </executions>
+      </plugin>
+    </plugins>
+  </build>
+</project>
diff --git a/helix-lock/src/assemble/assembly.xml b/helix-lock/src/assemble/assembly.xml
new file mode 100644
index 0000000..a6bec12
--- /dev/null
+++ b/helix-lock/src/assemble/assembly.xml
@@ -0,0 +1,60 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+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.
+-->
+<assembly>
+  <id>pkg</id>
+  <formats>
+    <format>tar</format>
+  </formats>
+  <fileSets>
+    <fileSet>
+      <directory>target/helix-lock-pkg/bin</directory>
+      <outputDirectory>bin</outputDirectory>
+      <lineEnding>unix</lineEnding>
+      <fileMode>0755</fileMode>
+      <directoryMode>0755</directoryMode>
+    </fileSet>
+    <fileSet>
+      <directory>target/helix-lock-pkg/repo/</directory>
+      <outputDirectory>repo</outputDirectory>
+      <fileMode>0755</fileMode>
+      <directoryMode>0755</directoryMode>
+      <excludes>
+        <exclude>**/*.xml</exclude>
+      </excludes>
+    </fileSet>
+   <fileSet>
+      <directory>target/helix-lock-pkg/conf</directory>
+      <outputDirectory>conf</outputDirectory>
+      <lineEnding>unix</lineEnding>
+      <fileMode>0755</fileMode>
+      <directoryMode>0755</directoryMode>
+    </fileSet>
+    <fileSet>
+      <directory>${project.basedir}</directory>
+      <outputDirectory>/</outputDirectory>
+      <includes>
+        <include>LICENSE</include>
+        <include>NOTICE</include>
+        <include>DISCLAIMER</include>
+      </includes>
+      <fileMode>0755</fileMode>
+    </fileSet>
+  </fileSets>
+</assembly>
diff --git a/helix-lock/src/main/config/log4j.properties b/helix-lock/src/main/config/log4j.properties
new file mode 100644
index 0000000..f52dab8
--- /dev/null
+++ b/helix-lock/src/main/config/log4j.properties
@@ -0,0 +1,32 @@
+#
+# 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.
+#
+
+# Set root logger level to INFO and its only appender to A1.
+log4j.rootLogger=INFO,A1
+
+# A1 is set to be a ConsoleAppender.
+log4j.appender.A1=org.apache.log4j.ConsoleAppender
+
+# A1 uses PatternLayout.
+log4j.appender.A1.layout=org.apache.log4j.PatternLayout
+log4j.appender.A1.layout.ConversionPattern=%-4r [%t] %-5p %c %x - %m%n
+
+log4j.logger.org.I0Itec=ERROR
+log4j.logger.org.apache.zookeeper=ERROR
+log4j.logger.org.apache.helix=INFO
diff --git a/helix-lock/src/test/conf/testng.xml b/helix-lock/src/test/conf/testng.xml
new file mode 100644
index 0000000..ec3c8dc
--- /dev/null
+++ b/helix-lock/src/test/conf/testng.xml
@@ -0,0 +1,27 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+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.
+-->
+<!DOCTYPE suite SYSTEM "http://testng.org/testng-1.0.dtd">
+<suite name="Suite" parallel="false">
+  <test name="Test" preserve-order="true">
+    <packages>
+      <package name="org.apache.helix.lock.*"/>
+    </packages>
+  </test>
+</suite>
diff --git a/helix-lock/src/test/resources/log4j.properties b/helix-lock/src/test/resources/log4j.properties
new file mode 100644
index 0000000..24b6d10
--- /dev/null
+++ b/helix-lock/src/test/resources/log4j.properties
@@ -0,0 +1,41 @@
+#
+# 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.
+#
+# Set root logger level to DEBUG and its only appender to R.
+log4j.rootLogger=ERROR, C
+
+# A1 is set to be a ConsoleAppender.
+log4j.appender.C=org.apache.log4j.ConsoleAppender
+log4j.appender.C.layout=org.apache.log4j.PatternLayout
+log4j.appender.C.layout.ConversionPattern=%-4r [%t] %-5p %c %x - %m%n
+
+log4j.appender.R=org.apache.log4j.RollingFileAppender
+log4j.appender.R.layout=org.apache.log4j.PatternLayout
+log4j.appender.R.layout.ConversionPattern=%5p [%C:%M] (%F:%L) - %m%n
+log4j.appender.R.File=target/ClusterManagerLogs/log.txt
+
+log4j.appender.STATUSDUMP=org.apache.log4j.RollingFileAppender
+log4j.appender.STATUSDUMP.layout=org.apache.log4j.SimpleLayout
+log4j.appender.STATUSDUMP.File=target/ClusterManagerLogs/statusUpdates.log
+
+log4j.logger.org.I0Itec=ERROR
+log4j.logger.org.apache=ERROR
+log4j.logger.com.noelios=ERROR
+log4j.logger.org.restlet=ERROR
+
+log4j.logger.org.apache.helix.monitoring.ZKPathDataDumpTask=ERROR,STATUSDUMP
diff --git a/pom.xml b/pom.xml
index 6c78f06..ae79dce 100644
--- a/pom.xml
+++ b/pom.xml
@@ -243,6 +243,7 @@ under the License.
     <module>helix-core</module>
     <module>helix-admin-webapp</module>
     <module>helix-rest</module>
+    <module>helix-lock</module>
     <module>helix-agent</module>
     <!--<module>helix-front</module>-->
     <module>recipes</module>


[helix] 13/19: simplified acquireLock logic

Posted by jx...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

jxue pushed a commit to branch distributed-lock
in repository https://gitbox.apache.org/repos/asf/helix.git

commit f1d7b319379477974f9831c2a66fcfac5204c6d4
Author: Molly Gao <mg...@mgao-mn1.linkedin.biz>
AuthorDate: Thu Feb 6 20:40:50 2020 -0800

    simplified acquireLock logic
---
 .../org/apache/helix/lock/ZKHelixNonblockingLock.java    | 16 ++--------------
 1 file changed, 2 insertions(+), 14 deletions(-)

diff --git a/helix-lock/src/main/java/org/apache/helix/lock/ZKHelixNonblockingLock.java b/helix-lock/src/main/java/org/apache/helix/lock/ZKHelixNonblockingLock.java
index 0573228..03e712f 100644
--- a/helix-lock/src/main/java/org/apache/helix/lock/ZKHelixNonblockingLock.java
+++ b/helix-lock/src/main/java/org/apache/helix/lock/ZKHelixNonblockingLock.java
@@ -105,20 +105,8 @@ public class ZKHelixNonblockingLock implements HelixLock {
     }
     lockInfo.setLongField(ZKHelixNonblockingLockInfo.InfoKey.TIMEOUT.name(), deadline);
 
-    // 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);
-      if (hasTimedOut(curLock)) {
-        // set may fail when the znode version changes in between the get and set, meaning there are some changes in the lock
-        success =
-            _baseDataAccessor.set(_lockPath, lockInfo, stat.getVersion(), AccessOption.PERSISTENT);
-      }
-    }
-    return success;
+    LockUpdater updater = new LockUpdater(lockInfo);
+    return _baseDataAccessor.update(_lockPath, updater, AccessOption.PERSISTENT);
   }
 
   @Override


[helix] 04/19: Added LockInfo interface

Posted by jx...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

jxue pushed a commit to branch distributed-lock
in repository https://gitbox.apache.org/repos/asf/helix.git

commit df5b89c00f8fa2d91df49741fa505af9f465a914
Author: Molly Gao <mg...@mgao-mn1.linkedin.biz>
AuthorDate: Fri Jan 24 15:33:04 2020 -0800

    Added LockInfo interface
---
 helix-lock/src/main/java/org/apache/helix/lock/HelixLock.java | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/helix-lock/src/main/java/org/apache/helix/lock/HelixLock.java b/helix-lock/src/main/java/org/apache/helix/lock/HelixLock.java
index a04ac01..d163718 100644
--- a/helix-lock/src/main/java/org/apache/helix/lock/HelixLock.java
+++ b/helix-lock/src/main/java/org/apache/helix/lock/HelixLock.java
@@ -20,9 +20,8 @@ package org.apache.helix.lock;
  */
 
 /**
- * Generic interface for Helix distributed lock
+ * Generic interface for Helix distributed lock for both nonblocking and blocking calls
  */
-<<<<<<< HEAD
 public interface HelixLock<T> {
   /**
    * Blocking call to acquire a lock


[helix] 14/19: Added cluster level to HelixLockScope and convert lock path to uppercase

Posted by jx...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

jxue pushed a commit to branch distributed-lock
in repository https://gitbox.apache.org/repos/asf/helix.git

commit 2a7d5eee56f55099dee9bc08ae133017a08a2cd3
Author: Molly Gao <mg...@mgao-mn1.linkedin.biz>
AuthorDate: Fri Feb 7 14:53:03 2020 -0800

    Added cluster level to HelixLockScope and convert lock path to uppercase
---
 .../java/org/apache/helix/lock/HelixLockScope.java | 39 ++++++++++++++++------
 .../apache/helix/lock/ZKHelixNonblockingLock.java  | 12 +------
 .../helix/lock/TestZKHelixNonblockingLock.java     |  6 ++--
 3 files changed, 32 insertions(+), 25 deletions(-)

diff --git a/helix-lock/src/main/java/org/apache/helix/lock/HelixLockScope.java b/helix-lock/src/main/java/org/apache/helix/lock/HelixLockScope.java
index 9fc12fc..e31991a 100644
--- a/helix-lock/src/main/java/org/apache/helix/lock/HelixLockScope.java
+++ b/helix-lock/src/main/java/org/apache/helix/lock/HelixLockScope.java
@@ -34,11 +34,13 @@ public class HelixLockScope {
    */
   public enum LockScopeProperty {
 
-    PARTICIPANT(1, 0),
+    CLUSTER(1, 0),
 
-    RESOURCE(2, 1),
+    PARTICIPANT(2, 1),
 
-    PARTITION(3, 2);
+    RESOURCE(3, 2),
+
+    PARTITION(4, 3);
 
     final int _zkPathArgNum;
     final int _argumentPos;
@@ -71,14 +73,17 @@ public class HelixLockScope {
   private static final StringTemplate template = new StringTemplate();
 
   static {
-    template.addEntry(HelixLockScope.LockScopeProperty.PARTICIPANT, 1, "/{participantName}");
-    template.addEntry(HelixLockScope.LockScopeProperty.RESOURCE, 2,
-        "/{participantName}/{resourceName}");
-    template.addEntry(HelixLockScope.LockScopeProperty.PARTITION, 3,
-        "/{participantName}/{resourceName}/{partitionName}");
+    template.addEntry(LockScopeProperty.CLUSTER, 1, "/{clusterName}");
+    template.addEntry(HelixLockScope.LockScopeProperty.PARTICIPANT, 2,
+        "/{clusterName}/{participantName}");
+    template.addEntry(HelixLockScope.LockScopeProperty.RESOURCE, 3,
+        "/{clusterName}/{participantName}/{resourceName}");
+    template.addEntry(HelixLockScope.LockScopeProperty.PARTITION, 4,
+        "/{clusterName}/{participantName}/{resourceName}/{partitionName}");
   }
 
   private final HelixLockScope.LockScopeProperty _type;
+  private final String _clusterName;
   private final String _participantName;
   private final String _resourceName;
   private final String _partitionName;
@@ -101,7 +106,13 @@ public class HelixLockScope {
     _type = type;
 
     //Initialize the name fields for various scope
-    _participantName = zkPathKeys.get(LockScopeProperty.PARTICIPANT.getArgumentPos());
+    _clusterName = zkPathKeys.get(LockScopeProperty.CLUSTER.getArgumentPos());
+
+    if (type.getZkPathArgNum() >= LockScopeProperty.PARTICIPANT.getZkPathArgNum()) {
+      _participantName = zkPathKeys.get(LockScopeProperty.PARTICIPANT.getArgumentPos());
+    } else {
+      _participantName = null;
+    }
 
     if (type.getZkPathArgNum() >= LockScopeProperty.RESOURCE.getZkPathArgNum()) {
       _resourceName = zkPathKeys.get(LockScopeProperty.RESOURCE.getArgumentPos());
@@ -115,7 +126,7 @@ public class HelixLockScope {
       _partitionName = null;
     }
 
-    _zkPath = template.instantiate(type, zkPathKeys.toArray(new String[0]));
+    _zkPath = template.instantiate(type, zkPathKeys.toArray(new String[0])).toUpperCase();
   }
 
   /**
@@ -127,6 +138,14 @@ public class HelixLockScope {
   }
 
   /**
+   * Get the cluster name if it exists
+   * @return the cluster name
+   */
+  public String getClusterName() {
+    return _clusterName;
+  }
+
+  /**
    * Get the participant name if it exists
    * @return the participant name
    */
diff --git a/helix-lock/src/main/java/org/apache/helix/lock/ZKHelixNonblockingLock.java b/helix-lock/src/main/java/org/apache/helix/lock/ZKHelixNonblockingLock.java
index 03e712f..18cac1a 100644
--- a/helix-lock/src/main/java/org/apache/helix/lock/ZKHelixNonblockingLock.java
+++ b/helix-lock/src/main/java/org/apache/helix/lock/ZKHelixNonblockingLock.java
@@ -20,24 +20,14 @@
 package org.apache.helix.lock;
 
 import java.util.Date;
-import java.util.UUID;
 
-import com.google.common.annotations.VisibleForTesting;
 import org.I0Itec.zkclient.DataUpdater;
-import org.I0Itec.zkclient.exception.ZkNoNodeException;
 import org.apache.helix.AccessOption;
 import org.apache.helix.BaseDataAccessor;
 import org.apache.helix.HelixException;
-import org.apache.helix.PropertyPathBuilder;
-import org.apache.helix.PropertyType;
 import org.apache.helix.ZNRecord;
-import org.apache.helix.api.exceptions.HelixMetaDataAccessException;
 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;
 
 import static org.apache.helix.lock.ZKHelixNonblockingLockInfo.DEFAULT_OWNER_TEXT;
 import static org.apache.helix.lock.ZKHelixNonblockingLockInfo.DEFAULT_TIMEOUT_LONG;
@@ -68,7 +58,7 @@ public class ZKHelixNonblockingLock implements HelixLock {
    */
   public ZKHelixNonblockingLock(String clusterName, HelixLockScope scope, String zkAddress,
       Long timeout, String lockMsg, String userId) {
-    this("/" + clusterName + LOCK_ROOT + scope.getZkPath(), zkAddress, timeout, lockMsg, userId);
+    this("/" + clusterName.toUpperCase() + LOCK_ROOT + scope.getZkPath(), zkAddress, timeout, lockMsg, userId);
   }
 
   /**
diff --git a/helix-lock/src/test/java/org/apache/helix/lock/TestZKHelixNonblockingLock.java b/helix-lock/src/test/java/org/apache/helix/lock/TestZKHelixNonblockingLock.java
index 96a15b3..090a43d 100644
--- a/helix-lock/src/test/java/org/apache/helix/lock/TestZKHelixNonblockingLock.java
+++ b/helix-lock/src/test/java/org/apache/helix/lock/TestZKHelixNonblockingLock.java
@@ -26,11 +26,8 @@ import java.util.Map;
 import java.util.UUID;
 import java.util.concurrent.Callable;
 
-import org.apache.helix.PropertyPathBuilder;
-import org.apache.helix.PropertyType;
 import org.apache.helix.TestHelper;
 import org.apache.helix.ZNRecord;
-import org.apache.helix.ZkUnitTestBase;
 import org.apache.helix.common.ZkTestBase;
 import org.apache.zookeeper.CreateMode;
 import org.testng.Assert;
@@ -59,12 +56,13 @@ public class TestZKHelixNonblockingLock extends ZkTestBase {
     _userId = UUID.randomUUID().toString();
 
     List<String> pathKeys = new ArrayList<>();
+    pathKeys.add(_clusterName);
     pathKeys.add("participant_name");
     pathKeys.add("resource_name");
     pathKeys.add("partition_name");
 
     _participantScope = new HelixLockScope(HelixLockScope.LockScopeProperty.PARTITION, pathKeys);
-    _lockPath = "/" + _clusterName + _lockRoot + _participantScope.getZkPath();
+    _lockPath = "/" + _clusterName.toUpperCase() + _lockRoot + _participantScope.getZkPath();
     _lock = new ZKHelixNonblockingLock(_clusterName, _participantScope, ZK_ADDR, Long.MAX_VALUE,
         _lockMessage, _userId);
   }


[helix] 08/19: Fixed logic of release and isOwner

Posted by jx...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

jxue pushed a commit to branch distributed-lock
in repository https://gitbox.apache.org/repos/asf/helix.git

commit 32f3b86a4da664b637bd19b13e624789c5dd3a25
Author: Molly Gao <mg...@mgao-mn1.linkedin.biz>
AuthorDate: Tue Feb 4 10:41:13 2020 -0800

    Fixed logic of release and isOwner
---
 .../main/java/org/apache/helix/lock/HelixLock.java |   2 +-
 .../main/java/org/apache/helix/lock/LockInfo.java  |   9 +-
 .../apache/helix/lock/ZKHelixNonblockingLock.java  | 136 ++++++++++++++-------
 .../helix/lock/ZKHelixNonblockingLockInfo.java     |  45 +++----
 .../helix/lock/TestZKHelixNonblockingLock.java     | 106 ++++++----------
 5 files changed, 150 insertions(+), 148 deletions(-)

diff --git a/helix-lock/src/main/java/org/apache/helix/lock/HelixLock.java b/helix-lock/src/main/java/org/apache/helix/lock/HelixLock.java
index 71b1ca6..2d7e318 100644
--- a/helix-lock/src/main/java/org/apache/helix/lock/HelixLock.java
+++ b/helix-lock/src/main/java/org/apache/helix/lock/HelixLock.java
@@ -42,7 +42,7 @@ public interface HelixLock {
    * Retrieve the lock information, e.g. lock timeout, lock message, etc.
    * @return lock metadata information
    */
-  <T> LockInfo<T> getLockInfo();
+  <K, V> LockInfo<K, V> getLockInfo();
 
   /**
    * If the user is current lock owner
diff --git a/helix-lock/src/main/java/org/apache/helix/lock/LockInfo.java b/helix-lock/src/main/java/org/apache/helix/lock/LockInfo.java
index 30322bb..afd4c00 100644
--- a/helix-lock/src/main/java/org/apache/helix/lock/LockInfo.java
+++ b/helix-lock/src/main/java/org/apache/helix/lock/LockInfo.java
@@ -24,9 +24,10 @@ import java.util.Map;
 
 /**
  * Generic interface for a map contains the Helix lock information
- * @param <T> The type of the LockInfo value
+ * @param <K> The type of the LockInfo key
+ * @param <V> the type of the LockInfo value
  */
-public interface LockInfo<T> {
+public interface LockInfo<K, V> {
 
   //TODO: add specific setter and getter for any field that is determined to be universal for all implementations of HelixLock
 
@@ -35,12 +36,12 @@ public interface LockInfo<T> {
    * @param infoKey the key of the LockInfo field
    * @param infoValue the value of the LockInfo field
    */
-  void setInfoValue(String infoKey, T infoValue);
+  void setInfoValue(K infoKey, V infoValue);
 
   /**
    * 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
    */
-  T getInfoValue(String infoKey);
+  V getInfoValue(K infoKey);
 }
diff --git a/helix-lock/src/main/java/org/apache/helix/lock/ZKHelixNonblockingLock.java b/helix-lock/src/main/java/org/apache/helix/lock/ZKHelixNonblockingLock.java
index 5be84bb..44b5aa9 100644
--- a/helix-lock/src/main/java/org/apache/helix/lock/ZKHelixNonblockingLock.java
+++ b/helix-lock/src/main/java/org/apache/helix/lock/ZKHelixNonblockingLock.java
@@ -19,20 +19,18 @@
 
 package org.apache.helix.lock;
 
-import java.util.HashMap;
-import java.util.Map;
 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.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;
 
@@ -45,8 +43,11 @@ 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 String _userId;
   private final long _timeout;
   private final String _lockMsg;
   private final BaseDataAccessor<ZNRecord> _baseDataAccessor;
@@ -58,10 +59,12 @@ public class ZKHelixNonblockingLock implements HelixLock {
    * @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);
+      Long timeout, String lockMsg, String userId) {
+    this(PATH_DELIMITER + clusterName + PATH_DELIMITER + LOCK_ROOT + PATH_DELIMITER + scope,
+        zkAddress, timeout, lockMsg, userId);
   }
 
   /**
@@ -70,29 +73,36 @@ public class ZKHelixNonblockingLock implements HelixLock {
    * @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) {
+  private ZKHelixNonblockingLock(String lockPath, String zkAddress, Long timeout, String lockMsg,
+      String userId) {
     HelixZkClient zkClient = new ZkClient(zkAddress);
     _lockPath = lockPath;
     _timeout = timeout;
     _lockMsg = lockMsg;
-    _userID = userID;
+    if (!userId.matches(UUID_FORMAT_REGEX)) {
+      throw new IllegalArgumentException("The input user id is not a valid UUID.");
+    }
+    _userId = userId;
     _baseDataAccessor = new ZkBaseDataAccessor<ZNRecord>(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() {
+  public boolean acquireLock() {
 
     // Set lock information fields
-    ZNRecord lockInfo = new ZNRecord(_userID);
-    lockInfo.setSimpleField(ZKHelixNonblockingLockInfo.InfoKey.OWNER.name(), _userID);
+    ZNRecord lockInfo = new ZNRecord(_userId);
+    lockInfo.setSimpleField(ZKHelixNonblockingLockInfo.InfoKey.OWNER.name(), _userId);
     lockInfo.setSimpleField(ZKHelixNonblockingLockInfo.InfoKey.MESSAGE.name(), _lockMsg);
-    long timeout = System.currentTimeMillis() + _timeout;
+    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));
 
@@ -106,6 +116,7 @@ public class ZKHelixNonblockingLock implements HelixLock {
       long curTimeout =
           Long.parseLong(curLock.getSimpleField(ZKHelixNonblockingLockInfo.InfoKey.TIMEOUT.name()));
       if (System.currentTimeMillis() >= curTimeout) {
+        // set may fail when the znode version changes in between the get and set, meaning there are some changes in the lock
         success =
             _baseDataAccessor.set(_lockPath, lockInfo, stat.getVersion(), AccessOption.PERSISTENT);
       }
@@ -114,46 +125,79 @@ public class ZKHelixNonblockingLock implements HelixLock {
   }
 
   @Override
-  /**
-   * Blocking call to release a lock
-   * @return true if the lock was successfully released,
-   * false if the locked is not locked or is not locked by the user,
-   * or the lock could not be released
-   */ public boolean releaseLock() {
-    if (isOwner()) {
-      return _baseDataAccessor.remove(_lockPath, AccessOption.PERSISTENT);
-    }
-    return false;
+  public boolean releaseLock() {
+    ZNRecord newLockInfo = new ZNRecord(_userId);
+    newLockInfo.setSimpleField(ZKHelixNonblockingLockInfo.InfoKey.OWNER.name(),
+        ZKHelixNonblockingLockInfo.DEFAULT_OWNER_TEXT);
+    newLockInfo.setSimpleField(ZKHelixNonblockingLockInfo.InfoKey.MESSAGE.name(),
+        ZKHelixNonblockingLockInfo.DEFAULT_MESSAGE_TEXT);
+    newLockInfo.setSimpleField(ZKHelixNonblockingLockInfo.InfoKey.TIMEOUT.name(),
+        ZKHelixNonblockingLockInfo.DEFAULT_TIMEOUT_TEXT);
+    LockUpdater updater = new LockUpdater(newLockInfo);
+    return _baseDataAccessor.update(_lockPath, updater, AccessOption.PERSISTENT);
   }
 
   @Override
-  /**
-   * Retrieve the lock information, e.g. lock timeout, lock message, etc.
-   * @return lock metadata information, return null if there is no lock node for the path provided
-   */ public LockInfo<String> getLockInfo() {
+  public ZKHelixNonblockingLockInfo getLockInfo() {
     if (!_baseDataAccessor.exists(_lockPath, AccessOption.PERSISTENT)) {
-      return null;
+      return new ZKHelixNonblockingLockInfo();
     }
-    ZKHelixNonblockingLockInfo<String> lockInfo = new ZKHelixNonblockingLockInfo<>();
     ZNRecord curLockInfo = _baseDataAccessor.get(_lockPath, null, AccessOption.PERSISTENT);
-    lockInfo.setLockInfoFields(curLockInfo);
-    return lockInfo;
+    return new ZKHelixNonblockingLockInfo(curLockInfo);
   }
 
   @Override
-  /**
-   * Check if the user is current lock owner
-   * @return true if the user is the lock owner,
-   * false if the user is not the lock owner or the lock doesn't have a owner
-   */ public boolean isOwner() {
+  public boolean isOwner() {
     ZNRecord curLockInfo = _baseDataAccessor.get(_lockPath, null, AccessOption.PERSISTENT);
     if (curLockInfo == null) {
       return false;
     }
-    String ownerID = curLockInfo.getSimpleField(ZKHelixNonblockingLockInfo.InfoKey.OWNER.name());
-    if (ownerID == null) {
-      return false;
+    if (userIdMatches(curLockInfo)) {
+      return !hasTimedOut(curLockInfo);
+    }
+    return false;
+  }
+
+  /**
+   * Check if a lock has timed out
+   * @param record the current lock information in ZNRecord format
+   * @return return true if the lock has timed out, otherwise return false.
+   */
+  private boolean hasTimedOut(ZNRecord record) {
+    String timeoutStr = record.getSimpleField(ZKHelixNonblockingLockInfo.InfoKey.TIMEOUT.name());
+    return System.currentTimeMillis() >= Long.parseLong(timeoutStr);
+  }
+
+  /**
+   * Check if the current user Id matches with the owner Id in a lock info
+   * @param record the lock information in ZNRecord format
+   * @return return true if the two ids match, otherwise return false.
+   */
+  private boolean userIdMatches(ZNRecord record) {
+    String ownerId = record.getSimpleField(ZKHelixNonblockingLockInfo.InfoKey.OWNER.name());
+    return ownerId.equals(_userId);
+  }
+
+  /**
+   * Class that specifies how a lock node should be updated with another lock node for a lock owner only
+   */
+  private class LockUpdater implements DataUpdater<ZNRecord> {
+    final ZNRecord _record;
+
+    /**
+     * Initialize a structure for lock owner to update a lock node value
+     * @param record the lock node value will be updated in ZNRecord format
+     */
+    public LockUpdater(ZNRecord record) {
+      _record = record;
+    }
+
+    @Override
+    public ZNRecord update(ZNRecord current) {
+      if (current != null && userIdMatches(current) && !hasTimedOut(current)) {
+        return _record;
+      }
+      throw new HelixException("User is not authorized to perform this operation.");
     }
-    return ownerID.equals(_userID);
   }
 }
diff --git a/helix-lock/src/main/java/org/apache/helix/lock/ZKHelixNonblockingLockInfo.java b/helix-lock/src/main/java/org/apache/helix/lock/ZKHelixNonblockingLockInfo.java
index fb86187..50daa03 100644
--- a/helix-lock/src/main/java/org/apache/helix/lock/ZKHelixNonblockingLockInfo.java
+++ b/helix-lock/src/main/java/org/apache/helix/lock/ZKHelixNonblockingLockInfo.java
@@ -25,45 +25,38 @@ import java.util.Map;
 import org.apache.helix.ZNRecord;
 
 
-public class ZKHelixNonblockingLockInfo<T extends String> implements LockInfo<T> {
+public class ZKHelixNonblockingLockInfo<K extends ZKHelixNonblockingLockInfo.InfoKey, V extends String> implements LockInfo<K, V> {
 
-  private Map<String, String> lockInfo;
+  public static final String DEFAULT_OWNER_TEXT = "";
+  public static final String DEFAULT_MESSAGE_TEXT = "";
+  public static final String DEFAULT_TIMEOUT_TEXT = String.valueOf(-1);
+  private Map<InfoKey, String> lockInfo;
 
-  enum InfoKey {
+  public enum InfoKey {
     OWNER, MESSAGE, TIMEOUT
   }
 
   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);
   }
 
-  @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);
+  public ZKHelixNonblockingLockInfo(ZNRecord znRecord) {
+    this();
+    lockInfo.put(InfoKey.OWNER, znRecord.getSimpleField(InfoKey.OWNER.name()));
+    lockInfo.put(InfoKey.MESSAGE, znRecord.getSimpleField(InfoKey.MESSAGE.name()));
+    lockInfo.put(InfoKey.TIMEOUT, znRecord.getSimpleField(InfoKey.TIMEOUT.name()));
   }
 
   @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);
+  public void setInfoValue(InfoKey infoKey, String infoValue) {
+    lockInfo.put(infoKey, infoValue);
   }
 
-  /**
-   * 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) {
-    if (record == null) {
-      return;
-    }
-    Map<String, String> recordSimpleFields = record.getSimpleFields();
-    lockInfo.putAll(recordSimpleFields);
+  @Override
+  public String getInfoValue(InfoKey infoKey) {
+    return lockInfo.get(infoKey);
   }
 }
diff --git a/helix-lock/src/test/java/org/apache/helix/lock/TestZKHelixNonblockingLock.java b/helix-lock/src/test/java/org/apache/helix/lock/TestZKHelixNonblockingLock.java
index 95c41ac..37daf7b 100644
--- a/helix-lock/src/test/java/org/apache/helix/lock/TestZKHelixNonblockingLock.java
+++ b/helix-lock/src/test/java/org/apache/helix/lock/TestZKHelixNonblockingLock.java
@@ -31,16 +31,19 @@ import org.apache.helix.model.builder.HelixConfigScopeBuilder;
 import org.apache.zookeeper.CreateMode;
 import org.testng.Assert;
 import org.testng.annotations.BeforeClass;
+import org.testng.annotations.BeforeMethod;
 import org.testng.annotations.Test;
 
 
 public class TestZKHelixNonblockingLock extends ZkUnitTestBase {
 
-  private final String _lockPath = "/testLockPath";
   private final String _className = TestHelper.getTestClassName();
   private final String _methodName = TestHelper.getTestMethodName();
   private final String _clusterName = _className + "_" + _methodName;
   private final String _lockMessage = "Test";
+  private String _lockPath;
+  private ZKHelixNonblockingLock _lock;
+  private String _userId;
 
   @BeforeClass
   public void beforeClass() throws Exception {
@@ -49,79 +52,47 @@ public class TestZKHelixNonblockingLock extends ZkUnitTestBase {
 
     TestHelper.setupCluster(_clusterName, ZK_ADDR, 12918, "localhost", "TestDB", 1, 10, 5, 3,
         "MasterSlave", true);
-  }
-
-  @Test
-  public void testAcquireLockWithParticipantScope() {
-
-    // Initialize lock with participant config scope
+    _userId = UUID.randomUUID().toString();
     HelixConfigScope participantScope =
         new HelixConfigScopeBuilder(ConfigScopeProperty.PARTICIPANT).forCluster(_clusterName)
             .forParticipant("localhost_12918").build();
 
-    String userID = UUID.randomUUID().toString();
-
-    ZKHelixNonblockingLock lock =
-        new ZKHelixNonblockingLock(_clusterName, participantScope, ZK_ADDR, Long.MAX_VALUE,
-            _lockMessage, userID);
+    _lockPath = "/" + _clusterName + '/' + "LOCKS" + '/' + participantScope;
+    _lock = new ZKHelixNonblockingLock(_clusterName, participantScope, ZK_ADDR, Long.MAX_VALUE,
+        _lockMessage, _userId);
 
-    // Acquire lock
-    lock.acquireLock();
-    String lockPath = "/" + _clusterName + '/' + "LOCKS" + '/' + participantScope;
-    Assert.assertTrue(_gZkClient.exists(lockPath));
-
-    // Get lock information
-    LockInfo<String> record = lock.getLockInfo();
-    Assert
-        .assertEquals(record.getInfoValue(ZKHelixNonblockingLockInfo.InfoKey.OWNER.name()), userID);
-    Assert.assertEquals(record.getInfoValue(ZKHelixNonblockingLockInfo.InfoKey.MESSAGE.name()),
-        _lockMessage);
-
-    // Check if the user is lock owner
-    Assert.assertTrue(lock.isOwner());
+  }
 
-    // Release lock
-    lock.releaseLock();
-    Assert.assertFalse(_gZkClient.exists(lockPath));
+  @BeforeMethod
+  public void beforeMethod() {
+    _gZkClient.delete(_lockPath);
+    Assert.assertFalse(_gZkClient.exists(_lockPath));
   }
 
   @Test
-  public void testAcquireLockWithUserProvidedPath() {
-
-    // Initialize lock with user provided path
-    String userID = UUID.randomUUID().toString();
-
-    ZKHelixNonblockingLock lock =
-        new ZKHelixNonblockingLock(_lockPath, ZK_ADDR, Long.MAX_VALUE, _lockMessage, userID);
+  public void testAcquireLock() {
 
-    //Acquire lock
-    lock.acquireLock();
+    // Acquire lock
+    _lock.acquireLock();
     Assert.assertTrue(_gZkClient.exists(_lockPath));
 
     // Get lock information
-    LockInfo<String> record = lock.getLockInfo();
-    Assert
-        .assertEquals(record.getInfoValue(ZKHelixNonblockingLockInfo.InfoKey.OWNER.name()), userID);
-    Assert.assertEquals(record.getInfoValue(ZKHelixNonblockingLockInfo.InfoKey.MESSAGE.name()),
+    LockInfo<ZKHelixNonblockingLockInfo.InfoKey, String> record = _lock.getLockInfo();
+    Assert.assertEquals(record.getInfoValue(ZKHelixNonblockingLockInfo.InfoKey.OWNER), _userId);
+    Assert.assertEquals(record.getInfoValue(ZKHelixNonblockingLockInfo.InfoKey.MESSAGE),
         _lockMessage);
 
     // Check if the user is lock owner
-    Assert.assertTrue(lock.isOwner());
+    Assert.assertTrue(_lock.isOwner());
 
     // Release lock
-    lock.releaseLock();
-    Assert.assertFalse(_gZkClient.exists(_lockPath));
+    _lock.releaseLock();
+    Assert.assertFalse(_lock.isOwner());
   }
 
   @Test
   public void testAcquireLockWhenExistingLockNotExpired() {
 
-    // Initialize lock with user provided path
-    String ownerID = UUID.randomUUID().toString();
-
-    ZKHelixNonblockingLock lock =
-        new ZKHelixNonblockingLock(_lockPath, ZK_ADDR, 0L, _lockMessage, ownerID);
-
     // Fake condition when the lock owner is not current user
     String fakeUserID = UUID.randomUUID().toString();
     ZNRecord fakeRecord = new ZNRecord(fakeUserID);
@@ -131,28 +102,19 @@ public class TestZKHelixNonblockingLock extends ZkUnitTestBase {
     _gZkClient.create(_lockPath, fakeRecord, CreateMode.PERSISTENT);
 
     // Check if the user is lock owner
-    Assert.assertFalse(lock.isOwner());
+    Assert.assertFalse(_lock.isOwner());
 
     // Acquire lock
-    Assert.assertFalse(lock.acquireLock());
-    Assert.assertFalse(lock.isOwner());
+    Assert.assertFalse(_lock.acquireLock());
+    Assert.assertFalse(_lock.isOwner());
 
     // Release lock
-    Assert.assertFalse(lock.releaseLock());
-    Assert.assertTrue(_gZkClient.exists(_lockPath));
-
-    _gZkClient.delete(_lockPath);
+    Assert.assertFalse(_lock.releaseLock());
   }
 
   @Test
   public void testAcquireLockWhenExistingLockExpired() {
 
-    // Initialize lock with user provided path
-    String ownerID = UUID.randomUUID().toString();
-
-    ZKHelixNonblockingLock lock =
-        new ZKHelixNonblockingLock(_lockPath, ZK_ADDR, Long.MAX_VALUE, _lockMessage, ownerID);
-
     // Fake condition when the current lock already expired
     String fakeUserID = UUID.randomUUID().toString();
     ZNRecord fakeRecord = new ZNRecord(fakeUserID);
@@ -162,15 +124,17 @@ public class TestZKHelixNonblockingLock extends ZkUnitTestBase {
     _gZkClient.create(_lockPath, fakeRecord, CreateMode.PERSISTENT);
 
     // Acquire lock
-    Assert.assertTrue(lock.acquireLock());
-    Assert.assertTrue(_gZkClient.exists(_lockPath));
-
-    // Check if the current user is the lock owner
-    Assert.assertTrue(lock.isOwner());
+    Assert.assertTrue(_lock.acquireLock());
+    Assert.assertTrue(_lock.isOwner());
 
     // Release lock
-    Assert.assertTrue(lock.releaseLock());
-    Assert.assertFalse(_gZkClient.exists(_lockPath));
+    Assert.assertTrue(_lock.releaseLock());
+    Assert.assertFalse(_lock.isOwner());
+  }
+
+  @Test
+  public void testSimultaneousAcquire() {
+
   }
 }
 


[helix] 19/19: Rename interface HelixLock to DistributedLock

Posted by jx...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

jxue pushed a commit to branch distributed-lock
in repository https://gitbox.apache.org/repos/asf/helix.git

commit c11ac9516ea48b06c95eabc322646094b05a6b96
Author: Molly Gao <mg...@mgao-mn1.linkedin.biz>
AuthorDate: Tue Feb 18 17:58:19 2020 -0800

    Rename interface HelixLock to DistributedLock
---
 .../lock/{HelixLock.java => DistributedLock.java}  |  2 +-
 .../main/java/org/apache/helix/lock/LockInfo.java  | 27 ++++++++++++++--------
 ...Lock.java => ZKDistributedNonblockingLock.java} | 11 +++++----
 .../lock/helix/TestZKHelixNonblockingLock.java     | 12 +++++-----
 4 files changed, 30 insertions(+), 22 deletions(-)

diff --git a/helix-lock/src/main/java/org/apache/helix/lock/HelixLock.java b/helix-lock/src/main/java/org/apache/helix/lock/DistributedLock.java
similarity index 98%
rename from helix-lock/src/main/java/org/apache/helix/lock/HelixLock.java
rename to helix-lock/src/main/java/org/apache/helix/lock/DistributedLock.java
index 97e2c35..594ccf4 100644
--- a/helix-lock/src/main/java/org/apache/helix/lock/HelixLock.java
+++ b/helix-lock/src/main/java/org/apache/helix/lock/DistributedLock.java
@@ -22,7 +22,7 @@ package org.apache.helix.lock;
 /**
  * Generic interface for Helix distributed lock
  */
-public interface HelixLock {
+public interface DistributedLock {
   /**
    * Blocking call to acquire a lock
    * @return true if the lock was successfully acquired,
diff --git a/helix-lock/src/main/java/org/apache/helix/lock/LockInfo.java b/helix-lock/src/main/java/org/apache/helix/lock/LockInfo.java
index 2c9a24f..71f7258 100644
--- a/helix-lock/src/main/java/org/apache/helix/lock/LockInfo.java
+++ b/helix-lock/src/main/java/org/apache/helix/lock/LockInfo.java
@@ -36,20 +36,23 @@ public class LockInfo {
   public static final LockInfo defaultLockInfo =
       new LockInfo(DEFAULT_OWNER_TEXT, DEFAULT_MESSAGE_TEXT, DEFAULT_TIMEOUT_LONG);
 
-  private ZNRecord record;
+  private static final String ZNODE_ID = "LOCK";
+  private ZNRecord _record;
 
   /**
    * The keys to lock information
    */
   public enum LockInfoAttribute {
-    OWNER, MESSAGE, TIMEOUT
+    OWNER,
+    MESSAGE,
+    TIMEOUT
   }
 
   /**
    * Initialize a default LockInfo instance
    */
   private LockInfo() {
-    record = new ZNRecord("LOCK");
+    _record = new ZNRecord(ZNODE_ID);
     setLockInfoFields(DEFAULT_OWNER_TEXT, DEFAULT_MESSAGE_TEXT, DEFAULT_TIMEOUT_LONG);
   }
 
@@ -85,11 +88,11 @@ public class LockInfo {
    * @param timeout value of TIMEOUT attribute
    */
   private void setLockInfoFields(String ownerId, String message, long timeout) {
-    record.setSimpleField(LockInfoAttribute.OWNER.name(),
+    _record.setSimpleField(LockInfoAttribute.OWNER.name(),
         ownerId == null ? DEFAULT_OWNER_TEXT : ownerId);
-    record.setSimpleField(LockInfoAttribute.MESSAGE.name(),
+    _record.setSimpleField(LockInfoAttribute.MESSAGE.name(),
         message == null ? DEFAULT_MESSAGE_TEXT : message);
-    record.setLongField(LockInfoAttribute.TIMEOUT.name(), timeout);
+    _record.setLongField(LockInfoAttribute.TIMEOUT.name(), timeout);
   }
 
   /**
@@ -97,7 +100,7 @@ public class LockInfo {
    * @return the owner id of the lock, empty string if there is no owner id set
    */
   public String getOwner() {
-    String owner = record.getSimpleField(LockInfoAttribute.OWNER.name());
+    String owner = _record.getSimpleField(LockInfoAttribute.OWNER.name());
     return owner == null ? DEFAULT_OWNER_TEXT : owner;
   }
 
@@ -106,7 +109,7 @@ public class LockInfo {
    * @return the message of the lock, empty string if there is no message set
    */
   public String getMessage() {
-    String message = record.getSimpleField(LockInfoAttribute.MESSAGE.name());
+    String message = _record.getSimpleField(LockInfoAttribute.MESSAGE.name());
     return message == null ? DEFAULT_MESSAGE_TEXT : message;
   }
 
@@ -115,10 +118,14 @@ public class LockInfo {
    * @return the expiring time of the lock, -1 if there is no timeout set
    */
   public Long getTimeout() {
-    return record.getLongField(LockInfoAttribute.TIMEOUT.name(), DEFAULT_TIMEOUT_LONG);
+    return _record.getLongField(LockInfoAttribute.TIMEOUT.name(), DEFAULT_TIMEOUT_LONG);
   }
 
+  /**
+   * Get the underlying ZNRecord in a LockInfo
+   * @return lock information contained in a ZNRecord
+   */
   public ZNRecord getRecord() {
-    return record;
+    return _record;
   }
 }
diff --git a/helix-lock/src/main/java/org/apache/helix/lock/helix/ZKHelixNonblockingLock.java b/helix-lock/src/main/java/org/apache/helix/lock/helix/ZKDistributedNonblockingLock.java
similarity index 90%
rename from helix-lock/src/main/java/org/apache/helix/lock/helix/ZKHelixNonblockingLock.java
rename to helix-lock/src/main/java/org/apache/helix/lock/helix/ZKDistributedNonblockingLock.java
index cbfe9da..8fe8c8c 100644
--- a/helix-lock/src/main/java/org/apache/helix/lock/helix/ZKHelixNonblockingLock.java
+++ b/helix-lock/src/main/java/org/apache/helix/lock/helix/ZKDistributedNonblockingLock.java
@@ -26,7 +26,7 @@ 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.DistributedLock;
 import org.apache.helix.lock.LockInfo;
 import org.apache.helix.lock.LockScope;
 import org.apache.helix.manager.zk.ZkBaseDataAccessor;
@@ -36,9 +36,9 @@ import org.apache.log4j.Logger;
 /**
  * Helix nonblocking lock implementation based on Zookeeper
  */
-public class ZKHelixNonblockingLock implements HelixLock {
+public class ZKDistributedNonblockingLock implements DistributedLock {
 
-  private static final Logger LOG = Logger.getLogger(ZKHelixNonblockingLock.class);
+  private static final Logger LOG = Logger.getLogger(ZKDistributedNonblockingLock.class);
 
   private final String _lockPath;
   private final String _userId;
@@ -54,7 +54,7 @@ public class ZKHelixNonblockingLock implements HelixLock {
    * @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,
+  public ZKDistributedNonblockingLock(LockScope scope, String zkAddress, Long timeout, String lockMsg,
       String userId) {
     this(scope.getPath(), zkAddress, timeout, lockMsg, userId);
   }
@@ -67,7 +67,7 @@ public class ZKHelixNonblockingLock implements HelixLock {
    * @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,
+  private ZKDistributedNonblockingLock(String lockPath, String zkAddress, Long timeout, String lockMsg,
       String userId) {
     _lockPath = lockPath;
     if (timeout < 0) {
@@ -94,6 +94,7 @@ public class ZKHelixNonblockingLock implements HelixLock {
     return _baseDataAccessor.update(_lockPath, updater, AccessOption.PERSISTENT);
   }
 
+  //TODO: update release lock logic so it would not leave empty znodes after the lock is released
   @Override
   public boolean releaseLock() {
     // Initialize the lock updater with a default lock info represents the state of a unlocked lock
diff --git a/helix-lock/src/test/java/org/apache/helix/lock/helix/TestZKHelixNonblockingLock.java b/helix-lock/src/test/java/org/apache/helix/lock/helix/TestZKHelixNonblockingLock.java
index 39205af..08cf33b 100644
--- a/helix-lock/src/test/java/org/apache/helix/lock/helix/TestZKHelixNonblockingLock.java
+++ b/helix-lock/src/test/java/org/apache/helix/lock/helix/TestZKHelixNonblockingLock.java
@@ -42,7 +42,7 @@ public class TestZKHelixNonblockingLock extends ZkTestBase {
   private final String _clusterName = TestHelper.getTestClassName();
   private final String _lockMessage = "Test";
   private String _lockPath;
-  private ZKHelixNonblockingLock _lock;
+  private ZKDistributedNonblockingLock _lock;
   private String _userId;
   private HelixLockScope _participantScope;
 
@@ -61,7 +61,7 @@ public class TestZKHelixNonblockingLock extends ZkTestBase {
 
     _participantScope = new HelixLockScope(HelixLockScope.LockScopeProperty.CLUSTER, pathKeys);
     _lockPath = _participantScope.getPath();
-    _lock = new ZKHelixNonblockingLock(_participantScope, ZK_ADDR, Long.MAX_VALUE, _lockMessage,
+    _lock = new ZKDistributedNonblockingLock(_participantScope, ZK_ADDR, Long.MAX_VALUE, _lockMessage,
         _userId);
   }
 
@@ -137,8 +137,8 @@ public class TestZKHelixNonblockingLock extends ZkTestBase {
   public void testSimultaneousAcquire() {
     List<Callable<Boolean>> threads = new ArrayList<>();
     for (int i = 0; i < 2; i++) {
-      ZKHelixNonblockingLock lock =
-          new ZKHelixNonblockingLock(_participantScope, ZK_ADDR, Long.MAX_VALUE, _lockMessage,
+      ZKDistributedNonblockingLock lock =
+          new ZKDistributedNonblockingLock(_participantScope, ZK_ADDR, Long.MAX_VALUE, _lockMessage,
               UUID.randomUUID().toString());
       threads.add(new TestSimultaneousAcquireLock(lock));
     }
@@ -148,9 +148,9 @@ public class TestZKHelixNonblockingLock extends ZkTestBase {
   }
 
   private static class TestSimultaneousAcquireLock implements Callable<Boolean> {
-    final ZKHelixNonblockingLock _lock;
+    final ZKDistributedNonblockingLock _lock;
 
-    TestSimultaneousAcquireLock(ZKHelixNonblockingLock lock) {
+    TestSimultaneousAcquireLock(ZKDistributedNonblockingLock lock) {
       _lock = lock;
     }
 


[helix] 15/19: refactor LockInfo and some updates on the HelixLockScope

Posted by jx...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

jxue pushed a commit to branch distributed-lock
in repository https://gitbox.apache.org/repos/asf/helix.git

commit ed3b444f260fb3f05e799d8d80e77b5c04e203a3
Author: Molly Gao <mg...@mgao-mn1.linkedin.biz>
AuthorDate: Mon Feb 10 18:31:26 2020 -0800

    refactor LockInfo and some updates on the HelixLockScope
---
 .../main/java/org/apache/helix/lock/HelixLock.java |   2 +-
 .../java/org/apache/helix/lock/HelixLockScope.java |  27 ++--
 .../main/java/org/apache/helix/lock/LockInfo.java  | 152 +++++++++++++++++++--
 .../apache/helix/lock/ZKHelixNonblockingLock.java  |  87 ++++++------
 .../helix/lock/ZKHelixNonblockingLockInfo.java     |  92 -------------
 .../helix/lock/TestZKHelixNonblockingLock.java     |  21 ++-
 6 files changed, 203 insertions(+), 178 deletions(-)

diff --git a/helix-lock/src/main/java/org/apache/helix/lock/HelixLock.java b/helix-lock/src/main/java/org/apache/helix/lock/HelixLock.java
index ec425f6..1cd9e60 100644
--- a/helix-lock/src/main/java/org/apache/helix/lock/HelixLock.java
+++ b/helix-lock/src/main/java/org/apache/helix/lock/HelixLock.java
@@ -41,7 +41,7 @@ public interface HelixLock {
    * Retrieve the lock information, e.g. lock timeout, lock message, etc.
    * @return lock metadata information
    */
-  <K, V> LockInfo<K, V> getLockInfo();
+  LockInfo getLockInfo();
 
   /**
    * If the user is current lock owner
diff --git a/helix-lock/src/main/java/org/apache/helix/lock/HelixLockScope.java b/helix-lock/src/main/java/org/apache/helix/lock/HelixLockScope.java
index e31991a..3df0e7e 100644
--- a/helix-lock/src/main/java/org/apache/helix/lock/HelixLockScope.java
+++ b/helix-lock/src/main/java/org/apache/helix/lock/HelixLockScope.java
@@ -34,20 +34,23 @@ public class HelixLockScope {
    */
   public enum LockScopeProperty {
 
-    CLUSTER(1, 0),
+    CLUSTER(1),
 
-    PARTICIPANT(2, 1),
+    PARTICIPANT(2),
 
-    RESOURCE(3, 2),
+    RESOURCE(3),
 
-    PARTITION(4, 3);
+    PARTITION(4);
 
+    //the number of arguments required to generate a full path for the specific scope
     final int _zkPathArgNum;
-    final int _argumentPos;
 
-    private LockScopeProperty(int zkPathArgNum, int argumentPos) {
+    /**
+     * Initialize a LockScopeProperty
+     * @param zkPathArgNum the number of arguments required to generate a full path for the specific scope
+\     */
+    private LockScopeProperty(int zkPathArgNum) {
       _zkPathArgNum = zkPathArgNum;
-      _argumentPos = argumentPos;
     }
 
     /**
@@ -63,7 +66,7 @@ public class HelixLockScope {
      * @return the number of position of value for this property in the list of keys input
      */
     public int getArgumentPos() {
-      return _argumentPos;
+      return _zkPathArgNum - 1;
     }
   }
 
@@ -73,13 +76,13 @@ public class HelixLockScope {
   private static final StringTemplate template = new StringTemplate();
 
   static {
-    template.addEntry(LockScopeProperty.CLUSTER, 1, "/{clusterName}");
+    template.addEntry(LockScopeProperty.CLUSTER, 1, "/{clusterName}/LOCK");
     template.addEntry(HelixLockScope.LockScopeProperty.PARTICIPANT, 2,
-        "/{clusterName}/{participantName}");
+        "/{clusterName}/LOCK/{participantName}");
     template.addEntry(HelixLockScope.LockScopeProperty.RESOURCE, 3,
-        "/{clusterName}/{participantName}/{resourceName}");
+        "/{clusterName}/LOCK/{participantName}/{resourceName}");
     template.addEntry(HelixLockScope.LockScopeProperty.PARTITION, 4,
-        "/{clusterName}/{participantName}/{resourceName}/{partitionName}");
+        "/{clusterName}/LOCK/{participantName}/{resourceName}/{partitionName}");
   }
 
   private final HelixLockScope.LockScopeProperty _type;
diff --git a/helix-lock/src/main/java/org/apache/helix/lock/LockInfo.java b/helix-lock/src/main/java/org/apache/helix/lock/LockInfo.java
index afd4c00..3829d40 100644
--- a/helix-lock/src/main/java/org/apache/helix/lock/LockInfo.java
+++ b/helix-lock/src/main/java/org/apache/helix/lock/LockInfo.java
@@ -19,29 +19,153 @@
 
 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 <K> The type of the LockInfo key
- * @param <V> the type of the LockInfo value
+ * Structure represents a lock node information, implemented using ZNRecord
  */
-public interface LockInfo<K, V> {
+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);
+    resetLockInfo();
+  }
+
+  /**
+   * Initialize a LockInfo with a ZNRecord, set all info fields to default data
+   * @param znRecord The ZNRecord contains lock node data that used to initialize the LockInfo
+   */
+  public LockInfo(ZNRecord znRecord) {
+    super(znRecord);
+    setNullLockInfoFieldsToDefault();
+  }
+
+  /**
+   * Initialize a LockInfo with data for each field, set all null info fields to default data
+   * @param ownerId value of OWNER attribute
+   * @param message value of MESSAGE attribute
+   * @param timeout value of TIMEOUT attribute
+   */
+  public LockInfo(String ownerId, String message, long timeout) {
+    this(ownerId);
+    setLockInfoFields(ownerId, message, timeout);
+  }
+
+  /**
+   * Build a LOCKINFO instance that represents an unlocked lock states
+   * @return the unlocked lock node LockInfo instance
+   */
+  public static LockInfo buildUnlockedLockInfo() {
+    return new LockInfo("");
+  }
+
+  /**
+   * Set each field of lock info to user provided values if the values are not null, null values are set to default values
+   * @param ownerId value of OWNER attribute
+   * @param message value of MESSAGE attribute
+   * @param timeout value of TIMEOUT attribute
+   */
+  public void setLockInfoFields(String ownerId, String message, Long timeout) {
+    _record.setSimpleField(LockInfoAttribute.OWNER.name(),
+        ownerId == null ? DEFAULT_OWNER_TEXT : ownerId);
+    _record.setSimpleField(LockInfoAttribute.MESSAGE.name(),
+        message == null ? DEFAULT_MESSAGE_TEXT : message);
+    _record.setLongField(LockInfoAttribute.TIMEOUT.name(),
+        timeout == null ? DEFAULT_TIMEOUT_LONG : timeout);
+  }
+
+  /**
+   * Set all null values to default values in LockInfo, keep non-null values
+   */
+  private void setNullLockInfoFieldsToDefault() {
+    setLockInfoFields(getOwner(), getMessage(), getTimeout());
+  }
+
+  /**
+   * Reset the lock info to unlocked lock state
+   */
+  public void resetLockInfo() {
+    setLockInfoFields(DEFAULT_OWNER_TEXT, DEFAULT_MESSAGE_TEXT, DEFAULT_TIMEOUT_LONG);
+  }
+
+  /**
+   * Get the value for OWNER attribute of the lock
+   * @return the owner id of the lock, empty string if there is no owner id set
+   */
+  public String getOwner() {
+    String owner = _record.getSimpleField(LockInfoAttribute.OWNER.name());
+    return owner == null ? DEFAULT_OWNER_TEXT : owner;
+  }
+
+  /**
+   * Get the value for MESSAGE attribute of the lock
+   * @return the message of the lock, empty string if there is no message set
+   */
+  public String getMessage() {
+    String message = _record.getSimpleField(LockInfoAttribute.MESSAGE.name());
+    return message == null ? DEFAULT_MESSAGE_TEXT : message;
+  }
+
+  /**
+   * Get the value for TIMEOUT attribute of the lock
+   * @return the expiring time of the lock, -1 if there is no timeout set
+   */
+  public Long getTimeout() {
+    return _record.getLongField(LockInfoAttribute.TIMEOUT.name(), DEFAULT_TIMEOUT_LONG);
+  }
+
+  /**
+   * Get the value for OWNER attribute of the lock from a ZNRecord
+   * @return the owner id of the lock, empty string if there is no owner id set
+   */
+  static String getOwner(ZNRecord znRecord) {
+    if (znRecord == null) {
+      return DEFAULT_OWNER_TEXT;
+    }
+    String owner = znRecord.getSimpleField(LockInfoAttribute.OWNER.name());
+    return owner == null ? DEFAULT_OWNER_TEXT : owner;
+  }
 
   /**
-   * 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
+   * Get the value for MESSAGE attribute of the lock from a ZNRecord
+   * @return the message of the lock, empty string if there is no message set
    */
-  void setInfoValue(K infoKey, V infoValue);
+  static String getMessage(ZNRecord znRecord) {
+    if (znRecord == null) {
+      return DEFAULT_MESSAGE_TEXT;
+    }
+    String message = znRecord.getSimpleField(LockInfoAttribute.MESSAGE.name());
+    return message == null ? DEFAULT_MESSAGE_TEXT : message;
+  }
 
   /**
-   * 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
+   * Get the value for TIMEOUT attribute of the lock from a ZNRecord
+   * @return the expiring time of the lock, -1 if there is no timeout set
    */
-  V getInfoValue(K infoKey);
+  static long getTimeout(ZNRecord znRecord) {
+    if (znRecord == null) {
+      return DEFAULT_TIMEOUT_LONG;
+    }
+    return znRecord.getLongField(LockInfoAttribute.TIMEOUT.name(), DEFAULT_TIMEOUT_LONG);
+  }
 }
diff --git a/helix-lock/src/main/java/org/apache/helix/lock/ZKHelixNonblockingLock.java b/helix-lock/src/main/java/org/apache/helix/lock/ZKHelixNonblockingLock.java
index 18cac1a..b0cc8f1 100644
--- a/helix-lock/src/main/java/org/apache/helix/lock/ZKHelixNonblockingLock.java
+++ b/helix-lock/src/main/java/org/apache/helix/lock/ZKHelixNonblockingLock.java
@@ -20,6 +20,7 @@
 package org.apache.helix.lock;
 
 import java.util.Date;
+import java.util.concurrent.locks.Lock;
 
 import org.I0Itec.zkclient.DataUpdater;
 import org.apache.helix.AccessOption;
@@ -29,8 +30,9 @@ 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;
+import static org.apache.helix.lock.LockInfo.DEFAULT_OWNER_TEXT;
+import static org.apache.helix.lock.LockInfo.DEFAULT_TIMEOUT_LONG;
+import static org.apache.helix.lock.LockInfo.defaultLockInfo;
 
 
 /**
@@ -40,7 +42,6 @@ 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;
@@ -49,16 +50,15 @@ public class ZKHelixNonblockingLock implements HelixLock {
 
   /**
    * 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);
+  public ZKHelixNonblockingLock(HelixLockScope scope, String zkAddress, Long timeout,
+      String lockMsg, String userId) {
+    this(scope.getZkPath(), zkAddress, timeout, lockMsg, userId);
   }
 
   /**
@@ -82,10 +82,6 @@ public class ZKHelixNonblockingLock implements HelixLock {
   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 deadline;
     // Prevent value overflow
     if (_timeout > Long.MAX_VALUE - System.currentTimeMillis()) {
@@ -93,7 +89,8 @@ public class ZKHelixNonblockingLock implements HelixLock {
     } else {
       deadline = System.currentTimeMillis() + _timeout;
     }
-    lockInfo.setLongField(ZKHelixNonblockingLockInfo.InfoKey.TIMEOUT.name(), deadline);
+    LockInfo lockInfo = new LockInfo(_userId);
+    lockInfo.setLockInfoFields(_userId, _lockMsg, deadline);
 
     LockUpdater updater = new LockUpdater(lockInfo);
     return _baseDataAccessor.update(_lockPath, updater, AccessOption.PERSISTENT);
@@ -101,70 +98,64 @@ public class ZKHelixNonblockingLock implements HelixLock {
 
   @Override
   public boolean releaseLock() {
-    ZNRecord newLockInfo = new ZKHelixNonblockingLockInfo<>().toZNRecord();
-    LockUpdater updater = new LockUpdater(newLockInfo);
+    // 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 ZKHelixNonblockingLockInfo getLockInfo() {
+  public LockInfo getLockInfo() {
     ZNRecord curLockInfo = _baseDataAccessor.get(_lockPath, null, AccessOption.PERSISTENT);
-    return new ZKHelixNonblockingLockInfo(curLockInfo);
+    return new LockInfo(curLockInfo);
   }
 
   @Override
   public boolean isOwner() {
-    ZNRecord curLockInfo = _baseDataAccessor.get(_lockPath, null, AccessOption.PERSISTENT);
-    return userIdMatches(curLockInfo) && !hasTimedOut(curLockInfo);
+    LockInfo lockInfo = getLockInfo();
+    return userIdMatches(lockInfo) && !hasTimedOut(lockInfo);
   }
 
   /**
    * Check if a lock has timed out
-   * @param record the current lock information in ZNRecord format
    * @return return true if the lock has timed out, otherwise return false.
    */
-  private boolean hasTimedOut(ZNRecord record) {
-    if (record == null) {
-      return false;
-    }
-    long timeout = record
-        .getLongField(ZKHelixNonblockingLockInfo.InfoKey.TIMEOUT.name(), DEFAULT_TIMEOUT_LONG);
-    return System.currentTimeMillis() >= timeout;
+  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 System.currentTimeMillis() >= LockInfo.getTimeout(znRecord);
   }
 
   /**
    * Check if the current user Id matches with the owner Id in a lock info
-   * @param record the lock information in ZNRecord format
    * @return return true if the two ids match, otherwise return false.
    */
-  private boolean userIdMatches(ZNRecord record) {
-    if (record == null) {
-      return false;
-    }
-    String ownerId = record.getSimpleField(ZKHelixNonblockingLockInfo.InfoKey.OWNER.name());
-    return ownerId.equals(_userId);
+  private boolean userIdMatches(LockInfo lockInfo) {
+    return lockInfo.getOwner().equals(_userId);
   }
 
   /**
-   * Check if the lock node has a owner id field in the lock information
-   * @param record Lock information in format of ZNRecord
-   * @return true if the lock has a owner id field that the ownership can be or not be timed out, otherwise false
+   * Check if a user id in the ZNRecord matches current user's id
+   * @param znRecord ZNRecord contains lock information
+   * @return true if the ids match, false if not or ZNRecord does not contain user id information
    */
-  private boolean hasOwnerField(ZNRecord record) {
-    if (record == null) {
-      return false;
-    }
-    String ownerId = record.getSimpleField(ZKHelixNonblockingLockInfo.InfoKey.OWNER.name());
-    return ownerId != null && !ownerId.equals(DEFAULT_OWNER_TEXT);
+  private boolean userIdMatches(ZNRecord znRecord) {
+    return LockInfo.getOwner(znRecord).equals(_userId);
   }
 
   /**
    * Check if the lock node has a current owner
-   * @param record Lock information in format of ZNRecord
+   * @param znRecord Lock information in format of ZNRecord
    * @return true if the lock has a current owner that the ownership has not be timed out, otherwise false
    */
-  private boolean hasNonExpiredOwner(ZNRecord record) {
-    return hasOwnerField(record) && !hasTimedOut(record);
+  private boolean hasNonExpiredOwner(ZNRecord znRecord) {
+    String owner = LockInfo.getOwner(znRecord);
+    return !owner.equals(DEFAULT_OWNER_TEXT) && !hasTimedOut(znRecord);
   }
 
   /**
@@ -175,10 +166,10 @@ public class ZKHelixNonblockingLock implements HelixLock {
 
     /**
      * Initialize a structure for lock user to update a lock node value
-     * @param record the lock node value will be updated in ZNRecord format
+     * @param lockInfo the lock node value will be used to update the lock
      */
-    public LockUpdater(ZNRecord record) {
-      _record = record;
+    public LockUpdater(LockInfo lockInfo) {
+      _record = lockInfo.getRecord();
     }
 
     @Override
diff --git a/helix-lock/src/main/java/org/apache/helix/lock/ZKHelixNonblockingLockInfo.java b/helix-lock/src/main/java/org/apache/helix/lock/ZKHelixNonblockingLockInfo.java
deleted file mode 100644
index 6e62717..0000000
--- a/helix-lock/src/main/java/org/apache/helix/lock/ZKHelixNonblockingLockInfo.java
+++ /dev/null
@@ -1,92 +0,0 @@
-/*
- * 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<K extends ZKHelixNonblockingLockInfo.InfoKey, V extends String> implements LockInfo<K, V> {
-
-  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<InfoKey, String> 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) {
-    this();
-    if (znRecord == null) {
-      return;
-    }
-    Map<String, String> simpleFields = znRecord.getSimpleFields();
-    lockInfo
-        .put(InfoKey.OWNER, simpleFields.getOrDefault(InfoKey.OWNER.name(), DEFAULT_OWNER_TEXT));
-    lockInfo.put(InfoKey.MESSAGE,
-        simpleFields.getOrDefault(InfoKey.MESSAGE.name(), DEFAULT_MESSAGE_TEXT));
-    lockInfo.put(InfoKey.TIMEOUT,
-        simpleFields.getOrDefault(InfoKey.TIMEOUT.name(), DEFAULT_TIMEOUT_TEXT));
-  }
-
-  @Override
-  public void setInfoValue(InfoKey infoKey, String infoValue) {
-    lockInfo.put(infoKey, infoValue);
-  }
-
-  @Override
-  public String getInfoValue(InfoKey infoKey) {
-    return lockInfo.get(infoKey);
-  }
-
-  /**
-   * Method to convert a ZKHelixNonblockingLockInfo to ZNRecord
-   * @return a ZNRecord format data contains lock information in its simple fields
-   */
-  public ZNRecord toZNRecord() {
-    ZNRecord znRecord = new ZNRecord("");
-    znRecord.setSimpleField(InfoKey.OWNER.name(),
-        lockInfo.getOrDefault(InfoKey.OWNER, DEFAULT_OWNER_TEXT));
-    znRecord.setSimpleField(InfoKey.MESSAGE.name(),
-        lockInfo.getOrDefault(InfoKey.MESSAGE, DEFAULT_MESSAGE_TEXT));
-    znRecord.setSimpleField(InfoKey.TIMEOUT.name(),
-        lockInfo.getOrDefault(InfoKey.TIMEOUT, DEFAULT_TIMEOUT_TEXT));
-    return znRecord;
-  }
-}
diff --git a/helix-lock/src/test/java/org/apache/helix/lock/TestZKHelixNonblockingLock.java b/helix-lock/src/test/java/org/apache/helix/lock/TestZKHelixNonblockingLock.java
index 090a43d..5e28688 100644
--- a/helix-lock/src/test/java/org/apache/helix/lock/TestZKHelixNonblockingLock.java
+++ b/helix-lock/src/test/java/org/apache/helix/lock/TestZKHelixNonblockingLock.java
@@ -39,7 +39,6 @@ import org.testng.annotations.Test;
 public class TestZKHelixNonblockingLock extends ZkTestBase {
 
   private final String _clusterName = TestHelper.getTestClassName();
-  private final String _lockRoot = "/LOCK";
   private final String _lockMessage = "Test";
   private String _lockPath;
   private ZKHelixNonblockingLock _lock;
@@ -62,8 +61,8 @@ public class TestZKHelixNonblockingLock extends ZkTestBase {
     pathKeys.add("partition_name");
 
     _participantScope = new HelixLockScope(HelixLockScope.LockScopeProperty.PARTITION, pathKeys);
-    _lockPath = "/" + _clusterName.toUpperCase() + _lockRoot + _participantScope.getZkPath();
-    _lock = new ZKHelixNonblockingLock(_clusterName, _participantScope, ZK_ADDR, Long.MAX_VALUE,
+    _lockPath = _participantScope.getZkPath();
+    _lock = new ZKHelixNonblockingLock(_participantScope, ZK_ADDR, Long.MAX_VALUE,
         _lockMessage, _userId);
   }
 
@@ -81,9 +80,9 @@ public class TestZKHelixNonblockingLock extends ZkTestBase {
     Assert.assertTrue(_gZkClient.exists(_lockPath));
 
     // Get lock information
-    LockInfo<ZKHelixNonblockingLockInfo.InfoKey, String> record = _lock.getLockInfo();
-    Assert.assertEquals(record.getInfoValue(ZKHelixNonblockingLockInfo.InfoKey.OWNER), _userId);
-    Assert.assertEquals(record.getInfoValue(ZKHelixNonblockingLockInfo.InfoKey.MESSAGE),
+    LockInfo lockInfo = _lock.getLockInfo();
+    Assert.assertEquals(lockInfo.getOwner(), _userId);
+    Assert.assertEquals(lockInfo.getMessage(),
         _lockMessage);
 
     // Check if the user is lock owner
@@ -100,8 +99,8 @@ public class TestZKHelixNonblockingLock extends ZkTestBase {
     // Fake condition when the lock owner is not current user
     String fakeUserID = UUID.randomUUID().toString();
     ZNRecord fakeRecord = new ZNRecord(fakeUserID);
-    fakeRecord.setSimpleField(ZKHelixNonblockingLockInfo.InfoKey.OWNER.name(), fakeUserID);
-    fakeRecord.setSimpleField(ZKHelixNonblockingLockInfo.InfoKey.TIMEOUT.name(),
+    fakeRecord.setSimpleField(LockInfo.LockInfoAttribute.OWNER.name(), fakeUserID);
+    fakeRecord.setSimpleField(LockInfo.LockInfoAttribute.TIMEOUT.name(),
         String.valueOf(Long.MAX_VALUE));
     _gZkClient.create(_lockPath, fakeRecord, CreateMode.PERSISTENT);
 
@@ -122,8 +121,8 @@ public class TestZKHelixNonblockingLock extends ZkTestBase {
     // Fake condition when the current lock already expired
     String fakeUserID = UUID.randomUUID().toString();
     ZNRecord fakeRecord = new ZNRecord(fakeUserID);
-    fakeRecord.setSimpleField(ZKHelixNonblockingLockInfo.InfoKey.OWNER.name(), fakeUserID);
-    fakeRecord.setSimpleField(ZKHelixNonblockingLockInfo.InfoKey.TIMEOUT.name(),
+    fakeRecord.setSimpleField(LockInfo.LockInfoAttribute.OWNER.name(), fakeUserID);
+    fakeRecord.setSimpleField(LockInfo.LockInfoAttribute.TIMEOUT.name(),
         String.valueOf(System.currentTimeMillis()));
     _gZkClient.create(_lockPath, fakeRecord, CreateMode.PERSISTENT);
 
@@ -141,7 +140,7 @@ public class TestZKHelixNonblockingLock extends ZkTestBase {
     List<Callable<Boolean>> threads = new ArrayList<>();
     for (int i = 0; i < 2; i++) {
       ZKHelixNonblockingLock lock =
-          new ZKHelixNonblockingLock(_clusterName, _participantScope, ZK_ADDR, Long.MAX_VALUE,
+          new ZKHelixNonblockingLock(_participantScope, ZK_ADDR, Long.MAX_VALUE,
               _lockMessage, UUID.randomUUID().toString());
       threads.add(new TestSimultaneousAcquireLock(lock));
     }


[helix] 16/19: Created LockScope interface

Posted by jx...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

jxue pushed a commit to branch distributed-lock
in repository https://gitbox.apache.org/repos/asf/helix.git

commit 62a0c4b5a9f2486da4814b44a1c4dc702fd63d8e
Author: Molly Gao <mg...@mgao-mn1.linkedin.biz>
AuthorDate: Fri Feb 14 13:28:07 2020 -0800

    Created LockScope interface
---
 .../main/java/org/apache/helix/lock/LockInfo.java  | 15 +++++--
 .../main/java/org/apache/helix/lock/LockScope.java | 25 ++++++++++++
 .../helix/lock/{ => helix}/HelixLockScope.java     | 47 +++++++++++-----------
 .../lock/{ => helix}/ZKHelixNonblockingLock.java   | 19 ++++-----
 .../{ => helix}/TestZKHelixNonblockingLock.java    |  5 ++-
 5 files changed, 72 insertions(+), 39 deletions(-)

diff --git a/helix-lock/src/main/java/org/apache/helix/lock/LockInfo.java b/helix-lock/src/main/java/org/apache/helix/lock/LockInfo.java
index 3829d40..212c93c 100644
--- a/helix-lock/src/main/java/org/apache/helix/lock/LockInfo.java
+++ b/helix-lock/src/main/java/org/apache/helix/lock/LockInfo.java
@@ -138,7 +138,7 @@ public class LockInfo extends HelixProperty {
    * Get the value for OWNER attribute of the lock from a ZNRecord
    * @return the owner id of the lock, empty string if there is no owner id set
    */
-  static String getOwner(ZNRecord znRecord) {
+  public static String getOwner(ZNRecord znRecord) {
     if (znRecord == null) {
       return DEFAULT_OWNER_TEXT;
     }
@@ -150,7 +150,7 @@ public class LockInfo extends HelixProperty {
    * Get the value for MESSAGE attribute of the lock from a ZNRecord
    * @return the message of the lock, empty string if there is no message set
    */
-  static String getMessage(ZNRecord znRecord) {
+  public static String getMessage(ZNRecord znRecord) {
     if (znRecord == null) {
       return DEFAULT_MESSAGE_TEXT;
     }
@@ -162,10 +162,19 @@ public class LockInfo extends HelixProperty {
    * Get the value for TIMEOUT attribute of the lock from a ZNRecord
    * @return the expiring time of the lock, -1 if there is no timeout set
    */
-  static long getTimeout(ZNRecord znRecord) {
+  public static long getTimeout(ZNRecord znRecord) {
     if (znRecord == null) {
       return DEFAULT_TIMEOUT_LONG;
     }
     return znRecord.getLongField(LockInfoAttribute.TIMEOUT.name(), DEFAULT_TIMEOUT_LONG);
   }
+
+  /**
+   * Check if the lock has a owner id set
+   * @return true if an owner id is set, false if not
+   */
+  public static boolean ownerIdSet(ZNRecord znRecord) {
+    String ownerId = getOwner(znRecord);
+    return !ownerId.equals(DEFAULT_OWNER_TEXT);
+  }
 }
diff --git a/helix-lock/src/main/java/org/apache/helix/lock/LockScope.java b/helix-lock/src/main/java/org/apache/helix/lock/LockScope.java
new file mode 100644
index 0000000..ff2d329
--- /dev/null
+++ b/helix-lock/src/main/java/org/apache/helix/lock/LockScope.java
@@ -0,0 +1,25 @@
+/*
+ * 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;
+
+public interface LockScope {
+
+  String getPath();
+}
diff --git a/helix-lock/src/main/java/org/apache/helix/lock/HelixLockScope.java b/helix-lock/src/main/java/org/apache/helix/lock/helix/HelixLockScope.java
similarity index 80%
rename from helix-lock/src/main/java/org/apache/helix/lock/HelixLockScope.java
rename to helix-lock/src/main/java/org/apache/helix/lock/helix/HelixLockScope.java
index 3df0e7e..77907ea 100644
--- a/helix-lock/src/main/java/org/apache/helix/lock/HelixLockScope.java
+++ b/helix-lock/src/main/java/org/apache/helix/lock/helix/HelixLockScope.java
@@ -17,17 +17,18 @@
  * under the License.
  */
 
-package org.apache.helix.lock;
+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 {
+public class HelixLockScope implements LockScope {
 
   /**
    * Define various properties of Helix lock, and associate them with the number of arguments required for getting znode path
@@ -43,30 +44,30 @@ public class HelixLockScope {
     PARTITION(4);
 
     //the number of arguments required to generate a full path for the specific scope
-    final int _zkPathArgNum;
+    final int _pathArgNum;
 
     /**
      * Initialize a LockScopeProperty
-     * @param zkPathArgNum the number of arguments required to generate a full path for the specific scope
-\     */
-    private LockScopeProperty(int zkPathArgNum) {
-      _zkPathArgNum = zkPathArgNum;
+     * @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 getZkPathArgNum() {
-      return _zkPathArgNum;
+    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 getArgumentPos() {
-      return _zkPathArgNum - 1;
+    public int getArgPos() {
+      return _pathArgNum - 1;
     }
   }
 
@@ -98,33 +99,33 @@ public class HelixLockScope {
    * @param type the scope
    * @param zkPathKeys keys identifying a ZNode location
    */
-  public HelixLockScope(HelixLockScope.LockScopeProperty type, List<String> zkPathKeys) {
+  private HelixLockScope(HelixLockScope.LockScopeProperty type, List<String> zkPathKeys) {
 
-    if (zkPathKeys.size() != type.getZkPathArgNum()) {
+    if (zkPathKeys.size() != type.getPathArgNum()) {
       throw new IllegalArgumentException(
-          type + " requires " + type.getZkPathArgNum() + " arguments to get znode, but was: "
+          type + " requires " + type.getPathArgNum() + " arguments to get znode, but was: "
               + zkPathKeys);
     }
 
     _type = type;
 
     //Initialize the name fields for various scope
-    _clusterName = zkPathKeys.get(LockScopeProperty.CLUSTER.getArgumentPos());
+    _clusterName = zkPathKeys.get(LockScopeProperty.CLUSTER.getArgPos());
 
-    if (type.getZkPathArgNum() >= LockScopeProperty.PARTICIPANT.getZkPathArgNum()) {
-      _participantName = zkPathKeys.get(LockScopeProperty.PARTICIPANT.getArgumentPos());
+    if (type.getPathArgNum() >= LockScopeProperty.PARTICIPANT.getPathArgNum()) {
+      _participantName = zkPathKeys.get(LockScopeProperty.PARTICIPANT.getArgPos());
     } else {
       _participantName = null;
     }
 
-    if (type.getZkPathArgNum() >= LockScopeProperty.RESOURCE.getZkPathArgNum()) {
-      _resourceName = zkPathKeys.get(LockScopeProperty.RESOURCE.getArgumentPos());
+    if (type.getPathArgNum() >= LockScopeProperty.RESOURCE.getPathArgNum()) {
+      _resourceName = zkPathKeys.get(LockScopeProperty.RESOURCE.getArgPos());
     } else {
       _resourceName = null;
     }
 
-    if (type.getZkPathArgNum() >= LockScopeProperty.PARTITION.getZkPathArgNum()) {
-      _partitionName = zkPathKeys.get(LockScopeProperty.PARTITION.getArgumentPos());
+    if (type.getPathArgNum() >= LockScopeProperty.PARTITION.getPathArgNum()) {
+      _partitionName = zkPathKeys.get(LockScopeProperty.PARTITION.getArgPos());
     } else {
       _partitionName = null;
     }
@@ -172,11 +173,11 @@ public class HelixLockScope {
     return _partitionName;
   }
 
+  @Override
   /**
    * Get the path to the corresponding ZNode
    * @return a Zookeeper path
-   */
-  public String getZkPath() {
+   */ public String getPath() {
     return _zkPath;
   }
 }
diff --git a/helix-lock/src/main/java/org/apache/helix/lock/ZKHelixNonblockingLock.java b/helix-lock/src/main/java/org/apache/helix/lock/helix/ZKHelixNonblockingLock.java
similarity index 91%
rename from helix-lock/src/main/java/org/apache/helix/lock/ZKHelixNonblockingLock.java
rename to helix-lock/src/main/java/org/apache/helix/lock/helix/ZKHelixNonblockingLock.java
index b0cc8f1..478be95 100644
--- a/helix-lock/src/main/java/org/apache/helix/lock/ZKHelixNonblockingLock.java
+++ b/helix-lock/src/main/java/org/apache/helix/lock/helix/ZKHelixNonblockingLock.java
@@ -17,23 +17,21 @@
  * under the License.
  */
 
-package org.apache.helix.lock;
+package org.apache.helix.lock.helix;
 
 import java.util.Date;
-import java.util.concurrent.locks.Lock;
 
 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;
 
-import static org.apache.helix.lock.LockInfo.DEFAULT_OWNER_TEXT;
-import static org.apache.helix.lock.LockInfo.DEFAULT_TIMEOUT_LONG;
-import static org.apache.helix.lock.LockInfo.defaultLockInfo;
-
 
 /**
  * Helix nonblocking lock implementation based on Zookeeper
@@ -56,9 +54,9 @@ public class ZKHelixNonblockingLock implements HelixLock {
    * @param lockMsg the reason for having this lock
    * @param userId a universal unique userId for lock owner identity
    */
-  public ZKHelixNonblockingLock(HelixLockScope scope, String zkAddress, Long timeout,
-      String lockMsg, String userId) {
-    this(scope.getZkPath(), zkAddress, timeout, lockMsg, userId);
+  public ZKHelixNonblockingLock(LockScope scope, String zkAddress, Long timeout, String lockMsg,
+      String userId) {
+    this(scope.getPath(), zkAddress, timeout, lockMsg, userId);
   }
 
   /**
@@ -154,8 +152,7 @@ public class ZKHelixNonblockingLock implements HelixLock {
    * @return true if the lock has a current owner that the ownership has not be timed out, otherwise false
    */
   private boolean hasNonExpiredOwner(ZNRecord znRecord) {
-    String owner = LockInfo.getOwner(znRecord);
-    return !owner.equals(DEFAULT_OWNER_TEXT) && !hasTimedOut(znRecord);
+    return LockInfo.ownerIdSet(znRecord) && !hasTimedOut(znRecord);
   }
 
   /**
diff --git a/helix-lock/src/test/java/org/apache/helix/lock/TestZKHelixNonblockingLock.java b/helix-lock/src/test/java/org/apache/helix/lock/helix/TestZKHelixNonblockingLock.java
similarity index 97%
rename from helix-lock/src/test/java/org/apache/helix/lock/TestZKHelixNonblockingLock.java
rename to helix-lock/src/test/java/org/apache/helix/lock/helix/TestZKHelixNonblockingLock.java
index 5e28688..e79922d 100644
--- a/helix-lock/src/test/java/org/apache/helix/lock/TestZKHelixNonblockingLock.java
+++ b/helix-lock/src/test/java/org/apache/helix/lock/helix/TestZKHelixNonblockingLock.java
@@ -17,7 +17,7 @@
  * under the License.
  */
 
-package org.apache.helix.lock;
+package org.apache.helix.lock.helix;
 
 import java.util.ArrayList;
 import java.util.Date;
@@ -29,6 +29,7 @@ import java.util.concurrent.Callable;
 import org.apache.helix.TestHelper;
 import org.apache.helix.ZNRecord;
 import org.apache.helix.common.ZkTestBase;
+import org.apache.helix.lock.LockInfo;
 import org.apache.zookeeper.CreateMode;
 import org.testng.Assert;
 import org.testng.annotations.BeforeClass;
@@ -61,7 +62,7 @@ public class TestZKHelixNonblockingLock extends ZkTestBase {
     pathKeys.add("partition_name");
 
     _participantScope = new HelixLockScope(HelixLockScope.LockScopeProperty.PARTITION, pathKeys);
-    _lockPath = _participantScope.getZkPath();
+    _lockPath = _participantScope.getPath();
     _lock = new ZKHelixNonblockingLock(_participantScope, ZK_ADDR, Long.MAX_VALUE,
         _lockMessage, _userId);
   }


[helix] 11/19: Changed method doc for releaseLock in HelixLock interface

Posted by jx...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

jxue pushed a commit to branch distributed-lock
in repository https://gitbox.apache.org/repos/asf/helix.git

commit 33bceca0b6151b47da2cda1a65be140d53ad5eb8
Author: Molly Gao <mg...@mgao-mn1.linkedin.biz>
AuthorDate: Thu Feb 6 12:02:00 2020 -0800

    Changed method doc for releaseLock in HelixLock interface
---
 helix-lock/src/main/java/org/apache/helix/lock/HelixLock.java | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/helix-lock/src/main/java/org/apache/helix/lock/HelixLock.java b/helix-lock/src/main/java/org/apache/helix/lock/HelixLock.java
index 2d7e318..ec425f6 100644
--- a/helix-lock/src/main/java/org/apache/helix/lock/HelixLock.java
+++ b/helix-lock/src/main/java/org/apache/helix/lock/HelixLock.java
@@ -32,9 +32,8 @@ public interface HelixLock {
 
   /**
    * Blocking call to release a lock
-   * @return true if the lock was successfully released,
-   * false if the locked is not locked or is not locked by the user,
-   * or the lock could not be released
+   * @return true if the lock was successfully released or if the locked is not currently locked,
+   * false if the lock is not locked by the user or the release operation failed
    */
   boolean releaseLock();
 


[helix] 05/19: Added details in comments

Posted by jx...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

jxue pushed a commit to branch distributed-lock
in repository https://gitbox.apache.org/repos/asf/helix.git

commit e01c8d5cc9c50f788cc70167a423ee6877a5f34c
Author: Molly Gao <mg...@mgao-mn1.linkedin.biz>
AuthorDate: Thu Jan 30 09:51:55 2020 -0800

    Added details in comments
---
 helix-lock/src/main/java/org/apache/helix/lock/HelixLock.java | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/helix-lock/src/main/java/org/apache/helix/lock/HelixLock.java b/helix-lock/src/main/java/org/apache/helix/lock/HelixLock.java
index d163718..01ef63b 100644
--- a/helix-lock/src/main/java/org/apache/helix/lock/HelixLock.java
+++ b/helix-lock/src/main/java/org/apache/helix/lock/HelixLock.java
@@ -20,7 +20,7 @@ package org.apache.helix.lock;
  */
 
 /**
- * Generic interface for Helix distributed lock for both nonblocking and blocking calls
+ * Generic interface for Helix distributed lock
  */
 public interface HelixLock<T> {
   /**


[helix] 17/19: Clean up code

Posted by jx...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

jxue pushed a commit to branch distributed-lock
in repository https://gitbox.apache.org/repos/asf/helix.git

commit 59842679faedaf4208cfab7aeacd10fbeda8daae
Author: Molly Gao <mg...@mgao-mn1.linkedin.biz>
AuthorDate: Fri Feb 14 18:51:48 2020 -0800

    Clean up code
---
 .../main/java/org/apache/helix/lock/HelixLock.java |   8 +-
 .../main/java/org/apache/helix/lock/LockInfo.java  |  93 ++++-------------
 .../main/java/org/apache/helix/lock/LockScope.java |   7 ++
 .../apache/helix/lock/helix/HelixLockScope.java    | 115 +++------------------
 .../helix/lock/helix/ZKHelixNonblockingLock.java   |  63 ++---------
 .../lock/helix/TestZKHelixNonblockingLock.java     |  35 +++----
 6 files changed, 74 insertions(+), 247 deletions(-)

diff --git a/helix-lock/src/main/java/org/apache/helix/lock/HelixLock.java b/helix-lock/src/main/java/org/apache/helix/lock/HelixLock.java
index 1cd9e60..97e2c35 100644
--- a/helix-lock/src/main/java/org/apache/helix/lock/HelixLock.java
+++ b/helix-lock/src/main/java/org/apache/helix/lock/HelixLock.java
@@ -38,15 +38,15 @@ public interface HelixLock {
   boolean releaseLock();
 
   /**
-   * Retrieve the lock information, e.g. lock timeout, lock message, etc.
+   * Retrieve the information of the current lock on the resource this lock object specifies, e.g. lock timeout, lock message, etc.
    * @return lock metadata information
    */
-  LockInfo getLockInfo();
+  LockInfo getCurrentLockInfo();
 
   /**
-   * If the user is current lock owner
+   * If the user is current lock owner of the resource
    * @return true if the user is the lock owner,
    * false if the user is not the lock owner or the lock doesn't have a owner
    */
-  boolean isOwner();
+  boolean isCurrentOwner();
 }
diff --git a/helix-lock/src/main/java/org/apache/helix/lock/LockInfo.java b/helix-lock/src/main/java/org/apache/helix/lock/LockInfo.java
index 212c93c..250fb71 100644
--- a/helix-lock/src/main/java/org/apache/helix/lock/LockInfo.java
+++ b/helix-lock/src/main/java/org/apache/helix/lock/LockInfo.java
@@ -34,7 +34,8 @@ public class LockInfo extends HelixProperty {
   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("");
+  public static final LockInfo defaultLockInfo =
+      new LockInfo(DEFAULT_OWNER_TEXT, DEFAULT_MESSAGE_TEXT, DEFAULT_TIMEOUT_LONG);
 
   /**
    * The keys to lock information
@@ -44,11 +45,11 @@ public class LockInfo extends HelixProperty {
   }
 
   /**
-   * Initialize a LockInfo with a ZNRecord id, set all info fields to default data
+   * Initialize a default LockInfo instance
    */
-  public LockInfo(String id) {
-    super(id);
-    resetLockInfo();
+  private LockInfo() {
+    super("LOCK");
+    setLockInfoFields(null, null, DEFAULT_TIMEOUT_LONG);
   }
 
   /**
@@ -56,8 +57,13 @@ public class LockInfo extends HelixProperty {
    * @param znRecord The ZNRecord contains lock node data that used to initialize the LockInfo
    */
   public LockInfo(ZNRecord znRecord) {
-    super(znRecord);
-    setNullLockInfoFieldsToDefault();
+    this();
+    if (znRecord != null) {
+      String ownerId = znRecord.getSimpleField(LockInfoAttribute.OWNER.name());
+      String message = znRecord.getSimpleField(LockInfoAttribute.MESSAGE.name());
+      long timeout = znRecord.getLongField(LockInfoAttribute.TIMEOUT.name(), DEFAULT_TIMEOUT_LONG);
+      setLockInfoFields(ownerId, message, timeout);
+    }
   }
 
   /**
@@ -67,45 +73,22 @@ public class LockInfo extends HelixProperty {
    * @param timeout value of TIMEOUT attribute
    */
   public LockInfo(String ownerId, String message, long timeout) {
-    this(ownerId);
+    this();
     setLockInfoFields(ownerId, message, timeout);
   }
 
   /**
-   * Build a LOCKINFO instance that represents an unlocked lock states
-   * @return the unlocked lock node LockInfo instance
-   */
-  public static LockInfo buildUnlockedLockInfo() {
-    return new LockInfo("");
-  }
-
-  /**
    * Set each field of lock info to user provided values if the values are not null, null values are set to default values
    * @param ownerId value of OWNER attribute
    * @param message value of MESSAGE attribute
    * @param timeout value of TIMEOUT attribute
    */
-  public void setLockInfoFields(String ownerId, String message, Long timeout) {
+  private void setLockInfoFields(String ownerId, String message, long timeout) {
     _record.setSimpleField(LockInfoAttribute.OWNER.name(),
         ownerId == null ? DEFAULT_OWNER_TEXT : ownerId);
     _record.setSimpleField(LockInfoAttribute.MESSAGE.name(),
         message == null ? DEFAULT_MESSAGE_TEXT : message);
-    _record.setLongField(LockInfoAttribute.TIMEOUT.name(),
-        timeout == null ? DEFAULT_TIMEOUT_LONG : timeout);
-  }
-
-  /**
-   * Set all null values to default values in LockInfo, keep non-null values
-   */
-  private void setNullLockInfoFieldsToDefault() {
-    setLockInfoFields(getOwner(), getMessage(), getTimeout());
-  }
-
-  /**
-   * Reset the lock info to unlocked lock state
-   */
-  public void resetLockInfo() {
-    setLockInfoFields(DEFAULT_OWNER_TEXT, DEFAULT_MESSAGE_TEXT, DEFAULT_TIMEOUT_LONG);
+    _record.setLongField(LockInfoAttribute.TIMEOUT.name(), timeout);
   }
 
   /**
@@ -135,46 +118,10 @@ public class LockInfo extends HelixProperty {
   }
 
   /**
-   * Get the value for OWNER attribute of the lock from a ZNRecord
-   * @return the owner id of the lock, empty string if there is no owner id set
-   */
-  public static String getOwner(ZNRecord znRecord) {
-    if (znRecord == null) {
-      return DEFAULT_OWNER_TEXT;
-    }
-    String owner = znRecord.getSimpleField(LockInfoAttribute.OWNER.name());
-    return owner == null ? DEFAULT_OWNER_TEXT : owner;
-  }
-
-  /**
-   * Get the value for MESSAGE attribute of the lock from a ZNRecord
-   * @return the message of the lock, empty string if there is no message set
-   */
-  public static String getMessage(ZNRecord znRecord) {
-    if (znRecord == null) {
-      return DEFAULT_MESSAGE_TEXT;
-    }
-    String message = znRecord.getSimpleField(LockInfoAttribute.MESSAGE.name());
-    return message == null ? DEFAULT_MESSAGE_TEXT : message;
-  }
-
-  /**
-   * Get the value for TIMEOUT attribute of the lock from a ZNRecord
-   * @return the expiring time of the lock, -1 if there is no timeout set
-   */
-  public static long getTimeout(ZNRecord znRecord) {
-    if (znRecord == null) {
-      return DEFAULT_TIMEOUT_LONG;
-    }
-    return znRecord.getLongField(LockInfoAttribute.TIMEOUT.name(), DEFAULT_TIMEOUT_LONG);
-  }
-
-  /**
-   * Check if the lock has a owner id set
-   * @return true if an owner id is set, false if not
+   * Check if a lock has timed out
+   * @return return true if the lock has timed out, otherwise return false.
    */
-  public static boolean ownerIdSet(ZNRecord znRecord) {
-    String ownerId = getOwner(znRecord);
-    return !ownerId.equals(DEFAULT_OWNER_TEXT);
+  public boolean hasNotExpired() {
+    return System.currentTimeMillis() < getTimeout();
   }
 }
diff --git a/helix-lock/src/main/java/org/apache/helix/lock/LockScope.java b/helix-lock/src/main/java/org/apache/helix/lock/LockScope.java
index ff2d329..0fa5cfc 100644
--- a/helix-lock/src/main/java/org/apache/helix/lock/LockScope.java
+++ b/helix-lock/src/main/java/org/apache/helix/lock/LockScope.java
@@ -19,7 +19,14 @@
 
 package org.apache.helix.lock;
 
+/**
+ * A predefined class to generate the lock path based on user input
+ */
 public interface LockScope {
 
+  /**
+   * Get the lock path
+   * @return the path of the lock
+   */
   String getPath();
 }
diff --git a/helix-lock/src/main/java/org/apache/helix/lock/helix/HelixLockScope.java b/helix-lock/src/main/java/org/apache/helix/lock/helix/HelixLockScope.java
index 77907ea..8eec472 100644
--- a/helix-lock/src/main/java/org/apache/helix/lock/helix/HelixLockScope.java
+++ b/helix-lock/src/main/java/org/apache/helix/lock/helix/HelixLockScope.java
@@ -22,6 +22,7 @@ package org.apache.helix.lock.helix;
 import java.util.List;
 
 import org.apache.helix.lock.LockScope;
+import org.apache.helix.model.HelixConfigScope;
 import org.apache.helix.util.StringTemplate;
 
 
@@ -35,13 +36,11 @@ public class HelixLockScope implements LockScope {
    */
   public enum LockScopeProperty {
 
-    CLUSTER(1),
+    CLUSTER(2),
 
     PARTICIPANT(2),
 
-    RESOURCE(3),
-
-    PARTITION(4);
+    RESOURCE(2);
 
     //the number of arguments required to generate a full path for the specific scope
     final int _pathArgNum;
@@ -61,14 +60,6 @@ public class HelixLockScope implements LockScope {
     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;
-    }
   }
 
   /**
@@ -76,108 +67,34 @@ public class HelixLockScope implements LockScope {
    */
   private static final StringTemplate template = new StringTemplate();
 
+  //TODO: Enrich the logic of path generation once we have a more detailed design
   static {
-    template.addEntry(LockScopeProperty.CLUSTER, 1, "/{clusterName}/LOCK");
+    template.addEntry(LockScopeProperty.CLUSTER, 2, "/{clusterName}/LOCK/CLUSTER/{clusterName}");
     template.addEntry(HelixLockScope.LockScopeProperty.PARTICIPANT, 2,
-        "/{clusterName}/LOCK/{participantName}");
-    template.addEntry(HelixLockScope.LockScopeProperty.RESOURCE, 3,
-        "/{clusterName}/LOCK/{participantName}/{resourceName}");
-    template.addEntry(HelixLockScope.LockScopeProperty.PARTITION, 4,
-        "/{clusterName}/LOCK/{participantName}/{resourceName}/{partitionName}");
+        "/{clusterName}/LOCK/PARTICIPANT/{participantName}");
+    template.addEntry(HelixLockScope.LockScopeProperty.RESOURCE, 2,
+        "/{clusterName}/LOCK/RESOURCE/{resourceName}");
   }
 
-  private final HelixLockScope.LockScopeProperty _type;
-  private final String _clusterName;
-  private final String _participantName;
-  private final String _resourceName;
-  private final String _partitionName;
-
-  private final String _zkPath;
+  private final String _path;
 
   /**
    * Initialize with a type of scope and unique identifiers
    * @param type the scope
-   * @param zkPathKeys keys identifying a ZNode location
+   * @param pathKeys keys identifying a ZNode location
    */
-  private HelixLockScope(HelixLockScope.LockScopeProperty type, List<String> zkPathKeys) {
+  public HelixLockScope(HelixLockScope.LockScopeProperty type, List<String> pathKeys) {
 
-    if (zkPathKeys.size() != type.getPathArgNum()) {
+    if (pathKeys.size() != type.getPathArgNum()) {
       throw new IllegalArgumentException(
           type + " requires " + type.getPathArgNum() + " arguments to get znode, but was: "
-              + zkPathKeys);
-    }
-
-    _type = type;
-
-    //Initialize the name fields for various scope
-    _clusterName = zkPathKeys.get(LockScopeProperty.CLUSTER.getArgPos());
-
-    if (type.getPathArgNum() >= LockScopeProperty.PARTICIPANT.getPathArgNum()) {
-      _participantName = zkPathKeys.get(LockScopeProperty.PARTICIPANT.getArgPos());
-    } else {
-      _participantName = null;
-    }
-
-    if (type.getPathArgNum() >= LockScopeProperty.RESOURCE.getPathArgNum()) {
-      _resourceName = zkPathKeys.get(LockScopeProperty.RESOURCE.getArgPos());
-    } else {
-      _resourceName = null;
-    }
-
-    if (type.getPathArgNum() >= LockScopeProperty.PARTITION.getPathArgNum()) {
-      _partitionName = zkPathKeys.get(LockScopeProperty.PARTITION.getArgPos());
-    } else {
-      _partitionName = null;
+              + pathKeys);
     }
-
-    _zkPath = template.instantiate(type, zkPathKeys.toArray(new String[0])).toUpperCase();
-  }
-
-  /**
-   * Get the scope
-   * @return the type of scope
-   */
-  public HelixLockScope.LockScopeProperty getType() {
-    return _type;
-  }
-
-  /**
-   * Get the cluster name if it exists
-   * @return the cluster name
-   */
-  public String getClusterName() {
-    return _clusterName;
-  }
-
-  /**
-   * Get the participant name if it exists
-   * @return the participant name
-   */
-  public String getParticipantName() {
-    return _participantName;
-  }
-
-  /**
-   * Get the resource name if it exists
-   * @return the resource name
-   */
-  public String getResourceName() {
-    return _resourceName;
-  }
-
-  /**
-   * Get the partition name if it exists
-   * @return the partition name
-   */
-  public String getPartitionName() {
-    return _partitionName;
+    _path = template.instantiate(type, pathKeys.toArray(new String[0]));
   }
 
   @Override
-  /**
-   * Get the path to the corresponding ZNode
-   * @return a Zookeeper path
-   */ public String getPath() {
-    return _zkPath;
+  public String getPath() {
+    return _path;
   }
 }
diff --git a/helix-lock/src/main/java/org/apache/helix/lock/helix/ZKHelixNonblockingLock.java b/helix-lock/src/main/java/org/apache/helix/lock/helix/ZKHelixNonblockingLock.java
index 478be95..41de4a7 100644
--- a/helix-lock/src/main/java/org/apache/helix/lock/helix/ZKHelixNonblockingLock.java
+++ b/helix-lock/src/main/java/org/apache/helix/lock/helix/ZKHelixNonblockingLock.java
@@ -70,6 +70,9 @@ public class ZKHelixNonblockingLock implements HelixLock {
   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;
@@ -87,10 +90,7 @@ public class ZKHelixNonblockingLock implements HelixLock {
     } else {
       deadline = System.currentTimeMillis() + _timeout;
     }
-    LockInfo lockInfo = new LockInfo(_userId);
-    lockInfo.setLockInfoFields(_userId, _lockMsg, deadline);
-
-    LockUpdater updater = new LockUpdater(lockInfo);
+    LockUpdater updater = new LockUpdater(new LockInfo(_userId, _lockMsg, deadline));
     return _baseDataAccessor.update(_lockPath, updater, AccessOption.PERSISTENT);
   }
 
@@ -102,57 +102,15 @@ public class ZKHelixNonblockingLock implements HelixLock {
   }
 
   @Override
-  public LockInfo getLockInfo() {
+  public LockInfo getCurrentLockInfo() {
     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 System.currentTimeMillis() >= LockInfo.getTimeout(znRecord);
-  }
-
-  /**
-   * Check if the current user Id matches with the owner Id in a lock info
-   * @return return true if the two ids match, otherwise return false.
-   */
-  private boolean userIdMatches(LockInfo lockInfo) {
-    return lockInfo.getOwner().equals(_userId);
-  }
-
-  /**
-   * Check if a user id in the ZNRecord matches current user's id
-   * @param znRecord ZNRecord contains lock information
-   * @return true if the ids match, false if not or ZNRecord does not contain user id information
-   */
-  private boolean userIdMatches(ZNRecord znRecord) {
-    return LockInfo.getOwner(znRecord).equals(_userId);
-  }
-
-  /**
-   * Check if the lock node has a current owner
-   * @param znRecord Lock information in format of ZNRecord
-   * @return true if the lock has a current owner that the ownership has not be timed out, otherwise false
-   */
-  private boolean hasNonExpiredOwner(ZNRecord znRecord) {
-    return LockInfo.ownerIdSet(znRecord) && !hasTimedOut(znRecord);
+  public boolean isCurrentOwner() {
+    LockInfo lockInfo = getCurrentLockInfo();
+    return lockInfo.getOwner().equals(_userId) && lockInfo.hasNotExpired();
   }
 
   /**
@@ -172,8 +130,9 @@ public class ZKHelixNonblockingLock implements HelixLock {
     @Override
     public ZNRecord update(ZNRecord current) {
       // If no one owns the lock, allow the update
-      // If the lock owner id matches user id, allow the update
-      if (!hasNonExpiredOwner(current) || userIdMatches(current)) {
+      // If the user is the current lock owner, allow the update
+      LockInfo curLockInfo = new LockInfo(current);
+      if (!curLockInfo.hasNotExpired() || isCurrentOwner()) {
         return _record;
       }
       // For users who are not the lock owner and try to do an update on a lock that is held by someone else, exception thrown is to be caught by data accessor, and return false for the update
diff --git a/helix-lock/src/test/java/org/apache/helix/lock/helix/TestZKHelixNonblockingLock.java b/helix-lock/src/test/java/org/apache/helix/lock/helix/TestZKHelixNonblockingLock.java
index e79922d..39205af 100644
--- a/helix-lock/src/test/java/org/apache/helix/lock/helix/TestZKHelixNonblockingLock.java
+++ b/helix-lock/src/test/java/org/apache/helix/lock/helix/TestZKHelixNonblockingLock.java
@@ -57,14 +57,12 @@ public class TestZKHelixNonblockingLock extends ZkTestBase {
 
     List<String> pathKeys = new ArrayList<>();
     pathKeys.add(_clusterName);
-    pathKeys.add("participant_name");
-    pathKeys.add("resource_name");
-    pathKeys.add("partition_name");
+    pathKeys.add(_clusterName);
 
-    _participantScope = new HelixLockScope(HelixLockScope.LockScopeProperty.PARTITION, pathKeys);
+    _participantScope = new HelixLockScope(HelixLockScope.LockScopeProperty.CLUSTER, pathKeys);
     _lockPath = _participantScope.getPath();
-    _lock = new ZKHelixNonblockingLock(_participantScope, ZK_ADDR, Long.MAX_VALUE,
-        _lockMessage, _userId);
+    _lock = new ZKHelixNonblockingLock(_participantScope, ZK_ADDR, Long.MAX_VALUE, _lockMessage,
+        _userId);
   }
 
   @BeforeMethod
@@ -81,17 +79,16 @@ public class TestZKHelixNonblockingLock extends ZkTestBase {
     Assert.assertTrue(_gZkClient.exists(_lockPath));
 
     // Get lock information
-    LockInfo lockInfo = _lock.getLockInfo();
+    LockInfo lockInfo = _lock.getCurrentLockInfo();
     Assert.assertEquals(lockInfo.getOwner(), _userId);
-    Assert.assertEquals(lockInfo.getMessage(),
-        _lockMessage);
+    Assert.assertEquals(lockInfo.getMessage(), _lockMessage);
 
     // Check if the user is lock owner
-    Assert.assertTrue(_lock.isOwner());
+    Assert.assertTrue(_lock.isCurrentOwner());
 
     // Release lock
     _lock.releaseLock();
-    Assert.assertFalse(_lock.isOwner());
+    Assert.assertFalse(_lock.isCurrentOwner());
   }
 
   @Test
@@ -101,16 +98,16 @@ public class TestZKHelixNonblockingLock extends ZkTestBase {
     String fakeUserID = UUID.randomUUID().toString();
     ZNRecord fakeRecord = new ZNRecord(fakeUserID);
     fakeRecord.setSimpleField(LockInfo.LockInfoAttribute.OWNER.name(), fakeUserID);
-    fakeRecord.setSimpleField(LockInfo.LockInfoAttribute.TIMEOUT.name(),
-        String.valueOf(Long.MAX_VALUE));
+    fakeRecord
+        .setSimpleField(LockInfo.LockInfoAttribute.TIMEOUT.name(), String.valueOf(Long.MAX_VALUE));
     _gZkClient.create(_lockPath, fakeRecord, CreateMode.PERSISTENT);
 
     // Check if the user is lock owner
-    Assert.assertFalse(_lock.isOwner());
+    Assert.assertFalse(_lock.isCurrentOwner());
 
     // Acquire lock
     Assert.assertFalse(_lock.acquireLock());
-    Assert.assertFalse(_lock.isOwner());
+    Assert.assertFalse(_lock.isCurrentOwner());
 
     // Release lock
     Assert.assertFalse(_lock.releaseLock());
@@ -129,11 +126,11 @@ public class TestZKHelixNonblockingLock extends ZkTestBase {
 
     // Acquire lock
     Assert.assertTrue(_lock.acquireLock());
-    Assert.assertTrue(_lock.isOwner());
+    Assert.assertTrue(_lock.isCurrentOwner());
 
     // Release lock
     Assert.assertTrue(_lock.releaseLock());
-    Assert.assertFalse(_lock.isOwner());
+    Assert.assertFalse(_lock.isCurrentOwner());
   }
 
   @Test
@@ -141,8 +138,8 @@ public class TestZKHelixNonblockingLock extends ZkTestBase {
     List<Callable<Boolean>> threads = new ArrayList<>();
     for (int i = 0; i < 2; i++) {
       ZKHelixNonblockingLock lock =
-          new ZKHelixNonblockingLock(_participantScope, ZK_ADDR, Long.MAX_VALUE,
-              _lockMessage, UUID.randomUUID().toString());
+          new ZKHelixNonblockingLock(_participantScope, ZK_ADDR, Long.MAX_VALUE, _lockMessage,
+              UUID.randomUUID().toString());
       threads.add(new TestSimultaneousAcquireLock(lock));
     }
     Map<String, Boolean> resultMap = TestHelper.startThreadsConcurrently(threads, 1000);


[helix] 12/19: Fixed lock path generation

Posted by jx...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

jxue pushed a commit to branch distributed-lock
in repository https://gitbox.apache.org/repos/asf/helix.git

commit 1038473156605040a5548744c66442f5c80776a7
Author: Molly Gao <mg...@mgao-ld1.linkedin.biz>
AuthorDate: Thu Feb 6 15:46:12 2020 -0800

    Fixed lock path generation
---
 .../java/org/apache/helix/lock/HelixLockScope.java | 160 +++++++++++++++++++++
 .../apache/helix/lock/ZKHelixNonblockingLock.java  |  10 +-
 .../helix/lock/TestZKHelixNonblockingLock.java     |  30 ++--
 3 files changed, 179 insertions(+), 21 deletions(-)

diff --git a/helix-lock/src/main/java/org/apache/helix/lock/HelixLockScope.java b/helix-lock/src/main/java/org/apache/helix/lock/HelixLockScope.java
new file mode 100644
index 0000000..9fc12fc
--- /dev/null
+++ b/helix-lock/src/main/java/org/apache/helix/lock/HelixLockScope.java
@@ -0,0 +1,160 @@
+/*
+ * 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 {
+
+  /**
+   * Define various properties of Helix lock, and associate them with the number of arguments required for getting znode path
+   */
+  public enum LockScopeProperty {
+
+    PARTICIPANT(1, 0),
+
+    RESOURCE(2, 1),
+
+    PARTITION(3, 2);
+
+    final int _zkPathArgNum;
+    final int _argumentPos;
+
+    private LockScopeProperty(int zkPathArgNum, int argumentPos) {
+      _zkPathArgNum = zkPathArgNum;
+      _argumentPos = argumentPos;
+    }
+
+    /**
+     * Get the number of template arguments required to generate a full path
+     * @return number of template arguments in the path
+     */
+    public int getZkPathArgNum() {
+      return _zkPathArgNum;
+    }
+
+    /**
+     * 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 getArgumentPos() {
+      return _argumentPos;
+    }
+  }
+
+  /**
+   * string templates to generate znode path
+   */
+  private static final StringTemplate template = new StringTemplate();
+
+  static {
+    template.addEntry(HelixLockScope.LockScopeProperty.PARTICIPANT, 1, "/{participantName}");
+    template.addEntry(HelixLockScope.LockScopeProperty.RESOURCE, 2,
+        "/{participantName}/{resourceName}");
+    template.addEntry(HelixLockScope.LockScopeProperty.PARTITION, 3,
+        "/{participantName}/{resourceName}/{partitionName}");
+  }
+
+  private final HelixLockScope.LockScopeProperty _type;
+  private final String _participantName;
+  private final String _resourceName;
+  private final String _partitionName;
+
+  private final String _zkPath;
+
+  /**
+   * Initialize with a type of scope and unique identifiers
+   * @param type the scope
+   * @param zkPathKeys keys identifying a ZNode location
+   */
+  public HelixLockScope(HelixLockScope.LockScopeProperty type, List<String> zkPathKeys) {
+
+    if (zkPathKeys.size() != type.getZkPathArgNum()) {
+      throw new IllegalArgumentException(
+          type + " requires " + type.getZkPathArgNum() + " arguments to get znode, but was: "
+              + zkPathKeys);
+    }
+
+    _type = type;
+
+    //Initialize the name fields for various scope
+    _participantName = zkPathKeys.get(LockScopeProperty.PARTICIPANT.getArgumentPos());
+
+    if (type.getZkPathArgNum() >= LockScopeProperty.RESOURCE.getZkPathArgNum()) {
+      _resourceName = zkPathKeys.get(LockScopeProperty.RESOURCE.getArgumentPos());
+    } else {
+      _resourceName = null;
+    }
+
+    if (type.getZkPathArgNum() >= LockScopeProperty.PARTITION.getZkPathArgNum()) {
+      _partitionName = zkPathKeys.get(LockScopeProperty.PARTITION.getArgumentPos());
+    } else {
+      _partitionName = null;
+    }
+
+    _zkPath = template.instantiate(type, zkPathKeys.toArray(new String[0]));
+  }
+
+  /**
+   * Get the scope
+   * @return the type of scope
+   */
+  public HelixLockScope.LockScopeProperty getType() {
+    return _type;
+  }
+
+  /**
+   * Get the participant name if it exists
+   * @return the participant name
+   */
+  public String getParticipantName() {
+    return _participantName;
+  }
+
+  /**
+   * Get the resource name if it exists
+   * @return the resource name
+   */
+  public String getResourceName() {
+    return _resourceName;
+  }
+
+  /**
+   * Get the partition name if it exists
+   * @return the partition name
+   */
+  public String getPartitionName() {
+    return _partitionName;
+  }
+
+  /**
+   * Get the path to the corresponding ZNode
+   * @return a Zookeeper path
+   */
+  public String getZkPath() {
+    return _zkPath;
+  }
+}
diff --git a/helix-lock/src/main/java/org/apache/helix/lock/ZKHelixNonblockingLock.java b/helix-lock/src/main/java/org/apache/helix/lock/ZKHelixNonblockingLock.java
index 0424177..0573228 100644
--- a/helix-lock/src/main/java/org/apache/helix/lock/ZKHelixNonblockingLock.java
+++ b/helix-lock/src/main/java/org/apache/helix/lock/ZKHelixNonblockingLock.java
@@ -28,6 +28,8 @@ import org.I0Itec.zkclient.exception.ZkNoNodeException;
 import org.apache.helix.AccessOption;
 import org.apache.helix.BaseDataAccessor;
 import org.apache.helix.HelixException;
+import org.apache.helix.PropertyPathBuilder;
+import org.apache.helix.PropertyType;
 import org.apache.helix.ZNRecord;
 import org.apache.helix.api.exceptions.HelixMetaDataAccessException;
 import org.apache.helix.manager.zk.ZkBaseDataAccessor;
@@ -48,8 +50,7 @@ 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 LOCK_ROOT = "/LOCK";
   private final String _lockPath;
   private final String _userId;
   private final long _timeout;
@@ -65,10 +66,9 @@ public class ZKHelixNonblockingLock implements HelixLock {
    * @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,
+  public ZKHelixNonblockingLock(String clusterName, HelixLockScope scope, String zkAddress,
       Long timeout, String lockMsg, String userId) {
-    this(PATH_DELIMITER +  String.join(PATH_DELIMITER, clusterName, LOCK_ROOT, scope.getZkPath()), zkAddress, timeout, lockMsg,
-        userId);
+    this("/" + clusterName + LOCK_ROOT + scope.getZkPath(), zkAddress, timeout, lockMsg, userId);
   }
 
   /**
diff --git a/helix-lock/src/test/java/org/apache/helix/lock/TestZKHelixNonblockingLock.java b/helix-lock/src/test/java/org/apache/helix/lock/TestZKHelixNonblockingLock.java
index 29f84ae..96a15b3 100644
--- a/helix-lock/src/test/java/org/apache/helix/lock/TestZKHelixNonblockingLock.java
+++ b/helix-lock/src/test/java/org/apache/helix/lock/TestZKHelixNonblockingLock.java
@@ -26,17 +26,12 @@ import java.util.Map;
 import java.util.UUID;
 import java.util.concurrent.Callable;
 
-import org.apache.helix.AccessOption;
 import org.apache.helix.PropertyPathBuilder;
+import org.apache.helix.PropertyType;
 import org.apache.helix.TestHelper;
 import org.apache.helix.ZNRecord;
 import org.apache.helix.ZkUnitTestBase;
-import org.apache.helix.manager.zk.TestWtCacheAsyncOpMultiThread;
-import org.apache.helix.manager.zk.ZKHelixDataAccessor;
-import org.apache.helix.manager.zk.ZkCacheBaseDataAccessor;
-import org.apache.helix.model.HelixConfigScope;
-import org.apache.helix.model.HelixConfigScope.ConfigScopeProperty;
-import org.apache.helix.model.builder.HelixConfigScopeBuilder;
+import org.apache.helix.common.ZkTestBase;
 import org.apache.zookeeper.CreateMode;
 import org.testng.Assert;
 import org.testng.annotations.BeforeClass;
@@ -44,16 +39,15 @@ import org.testng.annotations.BeforeMethod;
 import org.testng.annotations.Test;
 
 
-public class TestZKHelixNonblockingLock extends ZkUnitTestBase {
+public class TestZKHelixNonblockingLock extends ZkTestBase {
 
-  private final String _className = TestHelper.getTestClassName();
-  private final String _methodName = TestHelper.getTestMethodName();
-  private final String _clusterName = _className + "_" + _methodName;
+  private final String _clusterName = TestHelper.getTestClassName();
+  private final String _lockRoot = "/LOCK";
   private final String _lockMessage = "Test";
   private String _lockPath;
   private ZKHelixNonblockingLock _lock;
   private String _userId;
-  private HelixConfigScope _participantScope;
+  private HelixLockScope _participantScope;
 
   @BeforeClass
   public void beforeClass() throws Exception {
@@ -63,10 +57,14 @@ public class TestZKHelixNonblockingLock extends ZkUnitTestBase {
     TestHelper.setupCluster(_clusterName, ZK_ADDR, 12918, "localhost", "TestDB", 1, 10, 5, 3,
         "MasterSlave", true);
     _userId = UUID.randomUUID().toString();
-    _participantScope =
-        new HelixConfigScopeBuilder(ConfigScopeProperty.PARTICIPANT).forCluster(_clusterName)
-            .forParticipant("localhost_12918").build();
-    _lockPath = "/" + _clusterName + '/' + "LOCKS" + '/' + _participantScope.getZkPath();
+
+    List<String> pathKeys = new ArrayList<>();
+    pathKeys.add("participant_name");
+    pathKeys.add("resource_name");
+    pathKeys.add("partition_name");
+
+    _participantScope = new HelixLockScope(HelixLockScope.LockScopeProperty.PARTITION, pathKeys);
+    _lockPath = "/" + _clusterName + _lockRoot + _participantScope.getZkPath();
     _lock = new ZKHelixNonblockingLock(_clusterName, _participantScope, ZK_ADDR, Long.MAX_VALUE,
         _lockMessage, _userId);
   }


[helix] 06/19: created Helix nonblocking lock based on zk

Posted by jx...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

jxue pushed a commit to branch distributed-lock
in repository https://gitbox.apache.org/repos/asf/helix.git

commit 86c4b300996b02d61fbc08556865c684cf88e15a
Author: Molly Gao <mg...@mgao-mn1.linkedin.biz>
AuthorDate: Tue Jan 28 18:16:45 2020 -0800

    created Helix nonblocking lock based on zk
---
 .../main/java/org/apache/helix/lock/HelixLock.java |   4 +-
 .../apache/helix/lock/ZKHelixNonblockingLock.java  | 135 +++++++++++++++++++++
 .../helix/lock/ZKHelixNonblockingLockInfo.java     |  50 ++++++++
 3 files changed, 187 insertions(+), 2 deletions(-)

diff --git a/helix-lock/src/main/java/org/apache/helix/lock/HelixLock.java b/helix-lock/src/main/java/org/apache/helix/lock/HelixLock.java
index 01ef63b..71b1ca6 100644
--- a/helix-lock/src/main/java/org/apache/helix/lock/HelixLock.java
+++ b/helix-lock/src/main/java/org/apache/helix/lock/HelixLock.java
@@ -22,7 +22,7 @@ package org.apache.helix.lock;
 /**
  * Generic interface for Helix distributed lock
  */
-public interface HelixLock<T> {
+public interface HelixLock {
   /**
    * Blocking call to acquire a lock
    * @return true if the lock was successfully acquired,
@@ -42,7 +42,7 @@ public interface HelixLock<T> {
    * Retrieve the lock information, e.g. lock timeout, lock message, etc.
    * @return lock metadata information
    */
-  LockInfo<T> getLockInfo();
+  <T> LockInfo<T> getLockInfo();
 
   /**
    * If the user is current lock owner
diff --git a/helix-lock/src/main/java/org/apache/helix/lock/ZKHelixNonblockingLock.java b/helix-lock/src/main/java/org/apache/helix/lock/ZKHelixNonblockingLock.java
new file mode 100644
index 0000000..75dd0e5
--- /dev/null
+++ b/helix-lock/src/main/java/org/apache/helix/lock/ZKHelixNonblockingLock.java
@@ -0,0 +1,135 @@
+/*
+ * 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.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;
+
+
+/**
+ * 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<ZNRecord> _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<ZNRecord>(zkClient.getServers());
+  }
+
+  @Override
+  public boolean acquireLock() {
+
+    // Set lock information fields
+    ZNRecord lockInfo = new ZNRecord(_userID);
+    lockInfo.setSimpleField("Owner", _userID);
+    lockInfo.setSimpleField("message", _lockMsg);
+    long timeout = System.currentTimeMillis() + _timeout;
+    lockInfo.setSimpleField("timeout", String.valueOf(timeout));
+
+    // Try to create the lock node
+    boolean success = _baseDataAccessor.create(_lockPath, lockInfo, AccessOption.PERSISTENT);
+
+    // If fail to create the lock node, compare the timeout timestamp of current lock node with current time, if already passes the timeout, delete current lock node and try to create lock node again
+    if (!success) {
+      ZNRecord curLock = _baseDataAccessor.get(_lockPath, null, AccessOption.PERSISTENT);
+      long curTimeout = Long.parseLong(curLock.getSimpleField("timeout"));
+      if (System.currentTimeMillis() >= curTimeout) {
+        _baseDataAccessor.remove(_lockPath, AccessOption.PERSISTENT);
+        return _baseDataAccessor.create(_lockPath, lockInfo, AccessOption.PERSISTENT);
+      }
+    }
+    return success;
+  }
+
+  @Override
+  public boolean releaseLock() {
+    if (isOwner()) {
+      return _baseDataAccessor.remove(_lockPath, AccessOption.PERSISTENT);
+    }
+    return false;
+  }
+
+  @Override
+  public LockInfo<String> getLockInfo() {
+    ZKHelixNonblockingLockInfo<String> lockInfo = new ZKHelixNonblockingLockInfo<>();
+    ZNRecord curLockInfo = _baseDataAccessor.get(_lockPath, null, AccessOption.PERSISTENT);
+    lockInfo.setLockInfoFields(curLockInfo);
+    return lockInfo;
+  }
+
+  @Override
+  public boolean isOwner() {
+    ZNRecord curLockInfo = _baseDataAccessor.get(_lockPath, null, AccessOption.PERSISTENT);
+    if (curLockInfo == null) {
+      return false;
+    }
+    String ownerID = curLockInfo.getSimpleField("owner");
+    if (ownerID == null) {
+      return false;
+    }
+    return ownerID.equals(_userID);
+  }
+}
diff --git a/helix-lock/src/main/java/org/apache/helix/lock/ZKHelixNonblockingLockInfo.java b/helix-lock/src/main/java/org/apache/helix/lock/ZKHelixNonblockingLockInfo.java
new file mode 100644
index 0000000..d54c386
--- /dev/null
+++ b/helix-lock/src/main/java/org/apache/helix/lock/ZKHelixNonblockingLockInfo.java
@@ -0,0 +1,50 @@
+/*
+ * 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<T extends String> implements LockInfo<T> {
+
+  private Map<String, String>  lockInfo;
+
+  public ZKHelixNonblockingLockInfo() {
+    lockInfo = new HashMap<>();
+  }
+
+  @Override
+  public void setInfoValue(String infoKey, String infoValue) {
+    lockInfo.put(infoKey, infoValue);
+  }
+
+  @Override
+  public T getInfoValue(String infoKey) {
+    return (T)lockInfo.get(infoKey);
+  }
+
+  public void setLockInfoFields(ZNRecord record) {
+    Map<String, String> recordSimpleFields = record.getSimpleFields();
+    lockInfo.putAll(recordSimpleFields);
+  }
+}


[helix] 10/19: A few fixes on syntax

Posted by jx...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

jxue pushed a commit to branch distributed-lock
in repository https://gitbox.apache.org/repos/asf/helix.git

commit 6868eb61d5aadf02b3c934f5f903ad5e3f4779fa
Author: Molly Gao <mg...@mgao-mn1.linkedin.biz>
AuthorDate: Wed Feb 5 18:07:00 2020 -0800

    A few fixes on syntax
---
 .../apache/helix/lock/ZKHelixNonblockingLock.java  | 94 +++++++++++++---------
 .../helix/lock/ZKHelixNonblockingLockInfo.java     | 38 ++++++++-
 .../helix/lock/TestZKHelixNonblockingLock.java     |  3 +-
 3 files changed, 92 insertions(+), 43 deletions(-)

diff --git a/helix-lock/src/main/java/org/apache/helix/lock/ZKHelixNonblockingLock.java b/helix-lock/src/main/java/org/apache/helix/lock/ZKHelixNonblockingLock.java
index 57ff9aa..0424177 100644
--- a/helix-lock/src/main/java/org/apache/helix/lock/ZKHelixNonblockingLock.java
+++ b/helix-lock/src/main/java/org/apache/helix/lock/ZKHelixNonblockingLock.java
@@ -19,14 +19,17 @@
 
 package org.apache.helix.lock;
 
+import java.util.Date;
 import java.util.UUID;
 
 import com.google.common.annotations.VisibleForTesting;
 import org.I0Itec.zkclient.DataUpdater;
+import org.I0Itec.zkclient.exception.ZkNoNodeException;
 import org.apache.helix.AccessOption;
 import org.apache.helix.BaseDataAccessor;
 import org.apache.helix.HelixException;
 import org.apache.helix.ZNRecord;
+import org.apache.helix.api.exceptions.HelixMetaDataAccessException;
 import org.apache.helix.manager.zk.ZkBaseDataAccessor;
 import org.apache.helix.manager.zk.ZkClient;
 import org.apache.helix.manager.zk.client.HelixZkClient;
@@ -34,6 +37,9 @@ import org.apache.helix.model.HelixConfigScope;
 import org.apache.log4j.Logger;
 import org.apache.zookeeper.data.Stat;
 
+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
@@ -44,8 +50,6 @@ public class ZKHelixNonblockingLock implements HelixLock {
 
   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;
@@ -63,8 +67,8 @@ public class ZKHelixNonblockingLock implements HelixLock {
    */
   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);
+    this(PATH_DELIMITER +  String.join(PATH_DELIMITER, clusterName, LOCK_ROOT, scope.getZkPath()), zkAddress, timeout, lockMsg,
+        userId);
   }
 
   /**
@@ -77,15 +81,11 @@ public class ZKHelixNonblockingLock implements HelixLock {
    */
   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<ZNRecord>(zkClient.getServers());
+    _baseDataAccessor = new ZkBaseDataAccessor<>(zkAddress);
   }
 
   @Override
@@ -95,16 +95,15 @@ public class ZKHelixNonblockingLock implements HelixLock {
     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;
+    long deadline;
+    // Prevent value overflow
+    if (_timeout > Long.MAX_VALUE - System.currentTimeMillis()) {
+      deadline = Long.MAX_VALUE;
     } else {
-      timeout = System.currentTimeMillis() + _timeout;
+      deadline = System.currentTimeMillis() + _timeout;
     }
-    lockInfo
-        .setSimpleField(ZKHelixNonblockingLockInfo.InfoKey.TIMEOUT.name(), String.valueOf(timeout));
+    lockInfo.setLongField(ZKHelixNonblockingLockInfo.InfoKey.TIMEOUT.name(), deadline);
 
     // Try to create the lock node
     boolean success = _baseDataAccessor.create(_lockPath, lockInfo, AccessOption.PERSISTENT);
@@ -113,9 +112,7 @@ public class ZKHelixNonblockingLock implements HelixLock {
     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) {
+      if (hasTimedOut(curLock)) {
         // set may fail when the znode version changes in between the get and set, meaning there are some changes in the lock
         success =
             _baseDataAccessor.set(_lockPath, lockInfo, stat.getVersion(), AccessOption.PERSISTENT);
@@ -126,22 +123,13 @@ public class ZKHelixNonblockingLock implements HelixLock {
 
   @Override
   public boolean releaseLock() {
-    ZNRecord newLockInfo = new ZNRecord(_userId);
-    newLockInfo.setSimpleField(ZKHelixNonblockingLockInfo.InfoKey.OWNER.name(),
-        ZKHelixNonblockingLockInfo.DEFAULT_OWNER_TEXT);
-    newLockInfo.setSimpleField(ZKHelixNonblockingLockInfo.InfoKey.MESSAGE.name(),
-        ZKHelixNonblockingLockInfo.DEFAULT_MESSAGE_TEXT);
-    newLockInfo.setSimpleField(ZKHelixNonblockingLockInfo.InfoKey.TIMEOUT.name(),
-        ZKHelixNonblockingLockInfo.DEFAULT_TIMEOUT_TEXT);
+    ZNRecord newLockInfo = new ZKHelixNonblockingLockInfo<>().toZNRecord();
     LockUpdater updater = new LockUpdater(newLockInfo);
     return _baseDataAccessor.update(_lockPath, updater, AccessOption.PERSISTENT);
   }
 
   @Override
   public ZKHelixNonblockingLockInfo getLockInfo() {
-    if (!_baseDataAccessor.exists(_lockPath, AccessOption.PERSISTENT)) {
-      return new ZKHelixNonblockingLockInfo();
-    }
     ZNRecord curLockInfo = _baseDataAccessor.get(_lockPath, null, AccessOption.PERSISTENT);
     return new ZKHelixNonblockingLockInfo(curLockInfo);
   }
@@ -149,9 +137,6 @@ public class ZKHelixNonblockingLock implements HelixLock {
   @Override
   public boolean isOwner() {
     ZNRecord curLockInfo = _baseDataAccessor.get(_lockPath, null, AccessOption.PERSISTENT);
-    if (curLockInfo == null) {
-      return false;
-    }
     return userIdMatches(curLockInfo) && !hasTimedOut(curLockInfo);
   }
 
@@ -161,8 +146,12 @@ public class ZKHelixNonblockingLock implements HelixLock {
    * @return return true if the lock has timed out, otherwise return false.
    */
   private boolean hasTimedOut(ZNRecord record) {
-    String timeoutStr = record.getSimpleField(ZKHelixNonblockingLockInfo.InfoKey.TIMEOUT.name());
-    return System.currentTimeMillis() >= Long.parseLong(timeoutStr);
+    if (record == null) {
+      return false;
+    }
+    long timeout = record
+        .getLongField(ZKHelixNonblockingLockInfo.InfoKey.TIMEOUT.name(), DEFAULT_TIMEOUT_LONG);
+    return System.currentTimeMillis() >= timeout;
   }
 
   /**
@@ -171,18 +160,43 @@ public class ZKHelixNonblockingLock implements HelixLock {
    * @return return true if the two ids match, otherwise return false.
    */
   private boolean userIdMatches(ZNRecord record) {
+    if (record == null) {
+      return false;
+    }
     String ownerId = record.getSimpleField(ZKHelixNonblockingLockInfo.InfoKey.OWNER.name());
     return ownerId.equals(_userId);
   }
 
   /**
-   * Class that specifies how a lock node should be updated with another lock node for a lock owner only
+   * Check if the lock node has a owner id field in the lock information
+   * @param record Lock information in format of ZNRecord
+   * @return true if the lock has a owner id field that the ownership can be or not be timed out, otherwise false
+   */
+  private boolean hasOwnerField(ZNRecord record) {
+    if (record == null) {
+      return false;
+    }
+    String ownerId = record.getSimpleField(ZKHelixNonblockingLockInfo.InfoKey.OWNER.name());
+    return ownerId != null && !ownerId.equals(DEFAULT_OWNER_TEXT);
+  }
+
+  /**
+   * Check if the lock node has a current owner
+   * @param record Lock information in format of ZNRecord
+   * @return true if the lock has a current owner that the ownership has not be timed out, otherwise false
+   */
+  private boolean hasNonExpiredOwner(ZNRecord record) {
+    return hasOwnerField(record) && !hasTimedOut(record);
+  }
+
+  /**
+   * Class that specifies how a lock node should be updated with another lock node
    */
   private class LockUpdater implements DataUpdater<ZNRecord> {
     final ZNRecord _record;
 
     /**
-     * Initialize a structure for lock owner to update a lock node value
+     * Initialize a structure for lock user to update a lock node value
      * @param record the lock node value will be updated in ZNRecord format
      */
     public LockUpdater(ZNRecord record) {
@@ -191,9 +205,15 @@ public class ZKHelixNonblockingLock implements HelixLock {
 
     @Override
     public ZNRecord update(ZNRecord current) {
-      if (current != null && userIdMatches(current) && !hasTimedOut(current)) {
+      // If no one owns the lock, allow the update
+      // If the lock owner id matches user id, allow the update
+      if (!hasNonExpiredOwner(current) || userIdMatches(current)) {
         return _record;
       }
+      // For users who are not the lock owner and try to do an update on a lock that is held by someone else, exception thrown is to be caught by data accessor, and return false for the update
+      LOG.error(
+          "User " + _userId + "tried to update the lock at " + new Date(System.currentTimeMillis())
+              + ". Lock path: " + _lockPath);
       throw new HelixException("User is not authorized to perform this operation.");
     }
   }
diff --git a/helix-lock/src/main/java/org/apache/helix/lock/ZKHelixNonblockingLockInfo.java b/helix-lock/src/main/java/org/apache/helix/lock/ZKHelixNonblockingLockInfo.java
index 50daa03..6e62717 100644
--- a/helix-lock/src/main/java/org/apache/helix/lock/ZKHelixNonblockingLockInfo.java
+++ b/helix-lock/src/main/java/org/apache/helix/lock/ZKHelixNonblockingLockInfo.java
@@ -29,13 +29,17 @@ public class ZKHelixNonblockingLockInfo<K extends ZKHelixNonblockingLockInfo.Inf
 
   public static final String DEFAULT_OWNER_TEXT = "";
   public static final String DEFAULT_MESSAGE_TEXT = "";
-  public static final String DEFAULT_TIMEOUT_TEXT = String.valueOf(-1);
+  public static final long DEFAULT_TIMEOUT_LONG = -1L;
+  public static final String DEFAULT_TIMEOUT_TEXT = String.valueOf(DEFAULT_TIMEOUT_LONG);
   private Map<InfoKey, String> 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);
@@ -43,11 +47,22 @@ public class ZKHelixNonblockingLockInfo<K extends ZKHelixNonblockingLockInfo.Inf
     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) {
     this();
-    lockInfo.put(InfoKey.OWNER, znRecord.getSimpleField(InfoKey.OWNER.name()));
-    lockInfo.put(InfoKey.MESSAGE, znRecord.getSimpleField(InfoKey.MESSAGE.name()));
-    lockInfo.put(InfoKey.TIMEOUT, znRecord.getSimpleField(InfoKey.TIMEOUT.name()));
+    if (znRecord == null) {
+      return;
+    }
+    Map<String, String> simpleFields = znRecord.getSimpleFields();
+    lockInfo
+        .put(InfoKey.OWNER, simpleFields.getOrDefault(InfoKey.OWNER.name(), DEFAULT_OWNER_TEXT));
+    lockInfo.put(InfoKey.MESSAGE,
+        simpleFields.getOrDefault(InfoKey.MESSAGE.name(), DEFAULT_MESSAGE_TEXT));
+    lockInfo.put(InfoKey.TIMEOUT,
+        simpleFields.getOrDefault(InfoKey.TIMEOUT.name(), DEFAULT_TIMEOUT_TEXT));
   }
 
   @Override
@@ -59,4 +74,19 @@ public class ZKHelixNonblockingLockInfo<K extends ZKHelixNonblockingLockInfo.Inf
   public String getInfoValue(InfoKey infoKey) {
     return lockInfo.get(infoKey);
   }
+
+  /**
+   * Method to convert a ZKHelixNonblockingLockInfo to ZNRecord
+   * @return a ZNRecord format data contains lock information in its simple fields
+   */
+  public ZNRecord toZNRecord() {
+    ZNRecord znRecord = new ZNRecord("");
+    znRecord.setSimpleField(InfoKey.OWNER.name(),
+        lockInfo.getOrDefault(InfoKey.OWNER, DEFAULT_OWNER_TEXT));
+    znRecord.setSimpleField(InfoKey.MESSAGE.name(),
+        lockInfo.getOrDefault(InfoKey.MESSAGE, DEFAULT_MESSAGE_TEXT));
+    znRecord.setSimpleField(InfoKey.TIMEOUT.name(),
+        lockInfo.getOrDefault(InfoKey.TIMEOUT, DEFAULT_TIMEOUT_TEXT));
+    return znRecord;
+  }
 }
diff --git a/helix-lock/src/test/java/org/apache/helix/lock/TestZKHelixNonblockingLock.java b/helix-lock/src/test/java/org/apache/helix/lock/TestZKHelixNonblockingLock.java
index f2308c1..29f84ae 100644
--- a/helix-lock/src/test/java/org/apache/helix/lock/TestZKHelixNonblockingLock.java
+++ b/helix-lock/src/test/java/org/apache/helix/lock/TestZKHelixNonblockingLock.java
@@ -66,8 +66,7 @@ public class TestZKHelixNonblockingLock extends ZkUnitTestBase {
     _participantScope =
         new HelixConfigScopeBuilder(ConfigScopeProperty.PARTICIPANT).forCluster(_clusterName)
             .forParticipant("localhost_12918").build();
-
-    _lockPath = "/" + _clusterName + '/' + "LOCKS" + '/' + _participantScope;
+    _lockPath = "/" + _clusterName + '/' + "LOCKS" + '/' + _participantScope.getZkPath();
     _lock = new ZKHelixNonblockingLock(_clusterName, _participantScope, ZK_ADDR, Long.MAX_VALUE,
         _lockMessage, _userId);
   }


[helix] 03/19: Created Helix distributed lock design (apache#702)

Posted by jx...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

jxue pushed a commit to branch distributed-lock
in repository https://gitbox.apache.org/repos/asf/helix.git

commit f0a3bb2b226dd2c5ef4189fd64fc56206fb81ffa
Author: Molly Gao <mg...@mgao-ld1.linkedin.biz>
AuthorDate: Thu Jan 23 11:30:59 2020 -0800

    Created Helix distributed lock design (apache#702)
---
 helix-lock/src/main/java/org/apache/helix/lock/HelixLock.java | 1 +
 1 file changed, 1 insertion(+)

diff --git a/helix-lock/src/main/java/org/apache/helix/lock/HelixLock.java b/helix-lock/src/main/java/org/apache/helix/lock/HelixLock.java
index 01ef63b..a04ac01 100644
--- a/helix-lock/src/main/java/org/apache/helix/lock/HelixLock.java
+++ b/helix-lock/src/main/java/org/apache/helix/lock/HelixLock.java
@@ -22,6 +22,7 @@ package org.apache.helix.lock;
 /**
  * Generic interface for Helix distributed lock
  */
+<<<<<<< HEAD
 public interface HelixLock<T> {
   /**
    * Blocking call to acquire a lock


[helix] 02/19: Created Helix distributed lock interface (#703)

Posted by jx...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

jxue pushed a commit to branch distributed-lock
in repository https://gitbox.apache.org/repos/asf/helix.git

commit 5bea978211dad46fd20aaf6f16f751e72b8b5c89
Author: mgao0 <31...@users.noreply.github.com>
AuthorDate: Thu Jan 30 13:39:16 2020 -0800

    Created Helix distributed lock interface (#703)
    
    Added LockInfo interfaces.
---
 .../main/java/org/apache/helix/lock/HelixLock.java | 53 ++++++++++++++++++++++
 .../main/java/org/apache/helix/lock/LockInfo.java  | 46 +++++++++++++++++++
 2 files changed, 99 insertions(+)

diff --git a/helix-lock/src/main/java/org/apache/helix/lock/HelixLock.java b/helix-lock/src/main/java/org/apache/helix/lock/HelixLock.java
new file mode 100644
index 0000000..01ef63b
--- /dev/null
+++ b/helix-lock/src/main/java/org/apache/helix/lock/HelixLock.java
@@ -0,0 +1,53 @@
+package org.apache.helix.lock;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/**
+ * Generic interface for Helix distributed lock
+ */
+public interface HelixLock<T> {
+  /**
+   * Blocking call to acquire a lock
+   * @return true if the lock was successfully acquired,
+   * false if the lock could not be acquired
+   */
+  boolean acquireLock();
+
+  /**
+   * Blocking call to release a lock
+   * @return true if the lock was successfully released,
+   * false if the locked is not locked or is not locked by the user,
+   * or the lock could not be released
+   */
+  boolean releaseLock();
+
+  /**
+   * Retrieve the lock information, e.g. lock timeout, lock message, etc.
+   * @return lock metadata information
+   */
+  LockInfo<T> getLockInfo();
+
+  /**
+   * If the user is current lock owner
+   * @return true if the user is the lock owner,
+   * false if the user is not the lock owner or the lock doesn't have a owner
+   */
+  boolean isOwner();
+}
diff --git a/helix-lock/src/main/java/org/apache/helix/lock/LockInfo.java b/helix-lock/src/main/java/org/apache/helix/lock/LockInfo.java
new file mode 100644
index 0000000..30322bb
--- /dev/null
+++ b/helix-lock/src/main/java/org/apache/helix/lock/LockInfo.java
@@ -0,0 +1,46 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.helix.lock;
+
+import java.util.Map;
+
+
+/**
+ * Generic interface for a map contains the Helix lock information
+ * @param <T> The type of the LockInfo value
+ */
+public interface LockInfo<T> {
+
+  //TODO: add specific setter and getter for any field that is determined to be universal for all implementations of HelixLock
+
+  /**
+   * 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
+   */
+  void setInfoValue(String infoKey, T infoValue);
+
+  /**
+   * 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
+   */
+  T getInfoValue(String infoKey);
+}


[helix] 07/19: Added unit tests for Helix nonblocking lock

Posted by jx...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

jxue pushed a commit to branch distributed-lock
in repository https://gitbox.apache.org/repos/asf/helix.git

commit d58fbe6b2a87b6416330a4468c0a94710616602f
Author: Molly Gao <mg...@mgao-mn1.linkedin.biz>
AuthorDate: Mon Feb 3 14:26:41 2020 -0800

    Added unit tests for Helix nonblocking lock
---
 .../apache/helix/lock/ZKHelixNonblockingLock.java  |  50 ++++--
 .../helix/lock/ZKHelixNonblockingLockInfo.java     |  27 +++-
 .../helix/lock/TestZKHelixNonblockingLock.java     | 176 +++++++++++++++++++++
 3 files changed, 236 insertions(+), 17 deletions(-)

diff --git a/helix-lock/src/main/java/org/apache/helix/lock/ZKHelixNonblockingLock.java b/helix-lock/src/main/java/org/apache/helix/lock/ZKHelixNonblockingLock.java
index 75dd0e5..5be84bb 100644
--- a/helix-lock/src/main/java/org/apache/helix/lock/ZKHelixNonblockingLock.java
+++ b/helix-lock/src/main/java/org/apache/helix/lock/ZKHelixNonblockingLock.java
@@ -26,6 +26,7 @@ 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;
@@ -33,6 +34,7 @@ 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;
 
 
 /**
@@ -80,32 +82,44 @@ public class ZKHelixNonblockingLock implements HelixLock {
   }
 
   @Override
-  public boolean acquireLock() {
+  /**
+   * 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("Owner", _userID);
-    lockInfo.setSimpleField("message", _lockMsg);
+    lockInfo.setSimpleField(ZKHelixNonblockingLockInfo.InfoKey.OWNER.name(), _userID);
+    lockInfo.setSimpleField(ZKHelixNonblockingLockInfo.InfoKey.MESSAGE.name(), _lockMsg);
     long timeout = System.currentTimeMillis() + _timeout;
-    lockInfo.setSimpleField("timeout", String.valueOf(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, compare the timeout timestamp of current lock node with current time, if already passes the timeout, delete current lock node and try to create lock node again
+    // 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) {
-      ZNRecord curLock = _baseDataAccessor.get(_lockPath, null, AccessOption.PERSISTENT);
-      long curTimeout = Long.parseLong(curLock.getSimpleField("timeout"));
+      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) {
-        _baseDataAccessor.remove(_lockPath, AccessOption.PERSISTENT);
-        return _baseDataAccessor.create(_lockPath, lockInfo, AccessOption.PERSISTENT);
+        success =
+            _baseDataAccessor.set(_lockPath, lockInfo, stat.getVersion(), AccessOption.PERSISTENT);
       }
     }
     return success;
   }
 
   @Override
-  public boolean releaseLock() {
+  /**
+   * Blocking call to release a lock
+   * @return true if the lock was successfully released,
+   * false if the locked is not locked or is not locked by the user,
+   * or the lock could not be released
+   */ public boolean releaseLock() {
     if (isOwner()) {
       return _baseDataAccessor.remove(_lockPath, AccessOption.PERSISTENT);
     }
@@ -113,7 +127,13 @@ public class ZKHelixNonblockingLock implements HelixLock {
   }
 
   @Override
-  public LockInfo<String> getLockInfo() {
+  /**
+   * Retrieve the lock information, e.g. lock timeout, lock message, etc.
+   * @return lock metadata information, return null if there is no lock node for the path provided
+   */ public LockInfo<String> getLockInfo() {
+    if (!_baseDataAccessor.exists(_lockPath, AccessOption.PERSISTENT)) {
+      return null;
+    }
     ZKHelixNonblockingLockInfo<String> lockInfo = new ZKHelixNonblockingLockInfo<>();
     ZNRecord curLockInfo = _baseDataAccessor.get(_lockPath, null, AccessOption.PERSISTENT);
     lockInfo.setLockInfoFields(curLockInfo);
@@ -121,12 +141,16 @@ public class ZKHelixNonblockingLock implements HelixLock {
   }
 
   @Override
-  public boolean isOwner() {
+  /**
+   * Check if the user is current lock owner
+   * @return true if the user is the lock owner,
+   * false if the user is not the lock owner or the lock doesn't have a owner
+   */ public boolean isOwner() {
     ZNRecord curLockInfo = _baseDataAccessor.get(_lockPath, null, AccessOption.PERSISTENT);
     if (curLockInfo == null) {
       return false;
     }
-    String ownerID = curLockInfo.getSimpleField("owner");
+    String ownerID = curLockInfo.getSimpleField(ZKHelixNonblockingLockInfo.InfoKey.OWNER.name());
     if (ownerID == null) {
       return false;
     }
diff --git a/helix-lock/src/main/java/org/apache/helix/lock/ZKHelixNonblockingLockInfo.java b/helix-lock/src/main/java/org/apache/helix/lock/ZKHelixNonblockingLockInfo.java
index d54c386..fb86187 100644
--- a/helix-lock/src/main/java/org/apache/helix/lock/ZKHelixNonblockingLockInfo.java
+++ b/helix-lock/src/main/java/org/apache/helix/lock/ZKHelixNonblockingLockInfo.java
@@ -27,23 +27,42 @@ import org.apache.helix.ZNRecord;
 
 public class ZKHelixNonblockingLockInfo<T extends String> implements LockInfo<T> {
 
-  private Map<String, String>  lockInfo;
+  private Map<String, String> lockInfo;
+
+  enum InfoKey {
+    OWNER, MESSAGE, TIMEOUT
+  }
 
   public ZKHelixNonblockingLockInfo() {
     lockInfo = new HashMap<>();
   }
 
   @Override
-  public void setInfoValue(String infoKey, String infoValue) {
+  /**
+   * 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
-  public T getInfoValue(String infoKey) {
-    return (T)lockInfo.get(infoKey);
+  /**
+   * 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) {
+    if (record == null) {
+      return;
+    }
     Map<String, String> recordSimpleFields = record.getSimpleFields();
     lockInfo.putAll(recordSimpleFields);
   }
diff --git a/helix-lock/src/test/java/org/apache/helix/lock/TestZKHelixNonblockingLock.java b/helix-lock/src/test/java/org/apache/helix/lock/TestZKHelixNonblockingLock.java
new file mode 100644
index 0000000..95c41ac
--- /dev/null
+++ b/helix-lock/src/test/java/org/apache/helix/lock/TestZKHelixNonblockingLock.java
@@ -0,0 +1,176 @@
+/*
+ * 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 java.util.UUID;
+
+import org.apache.helix.TestHelper;
+import org.apache.helix.ZNRecord;
+import org.apache.helix.ZkUnitTestBase;
+import org.apache.helix.model.HelixConfigScope;
+import org.apache.helix.model.HelixConfigScope.ConfigScopeProperty;
+import org.apache.helix.model.builder.HelixConfigScopeBuilder;
+import org.apache.zookeeper.CreateMode;
+import org.testng.Assert;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.Test;
+
+
+public class TestZKHelixNonblockingLock extends ZkUnitTestBase {
+
+  private final String _lockPath = "/testLockPath";
+  private final String _className = TestHelper.getTestClassName();
+  private final String _methodName = TestHelper.getTestMethodName();
+  private final String _clusterName = _className + "_" + _methodName;
+  private final String _lockMessage = "Test";
+
+  @BeforeClass
+  public void beforeClass() throws Exception {
+
+    System.out.println("START " + _clusterName + " at " + new Date(System.currentTimeMillis()));
+
+    TestHelper.setupCluster(_clusterName, ZK_ADDR, 12918, "localhost", "TestDB", 1, 10, 5, 3,
+        "MasterSlave", true);
+  }
+
+  @Test
+  public void testAcquireLockWithParticipantScope() {
+
+    // Initialize lock with participant config scope
+    HelixConfigScope participantScope =
+        new HelixConfigScopeBuilder(ConfigScopeProperty.PARTICIPANT).forCluster(_clusterName)
+            .forParticipant("localhost_12918").build();
+
+    String userID = UUID.randomUUID().toString();
+
+    ZKHelixNonblockingLock lock =
+        new ZKHelixNonblockingLock(_clusterName, participantScope, ZK_ADDR, Long.MAX_VALUE,
+            _lockMessage, userID);
+
+    // Acquire lock
+    lock.acquireLock();
+    String lockPath = "/" + _clusterName + '/' + "LOCKS" + '/' + participantScope;
+    Assert.assertTrue(_gZkClient.exists(lockPath));
+
+    // Get lock information
+    LockInfo<String> record = lock.getLockInfo();
+    Assert
+        .assertEquals(record.getInfoValue(ZKHelixNonblockingLockInfo.InfoKey.OWNER.name()), userID);
+    Assert.assertEquals(record.getInfoValue(ZKHelixNonblockingLockInfo.InfoKey.MESSAGE.name()),
+        _lockMessage);
+
+    // Check if the user is lock owner
+    Assert.assertTrue(lock.isOwner());
+
+    // Release lock
+    lock.releaseLock();
+    Assert.assertFalse(_gZkClient.exists(lockPath));
+  }
+
+  @Test
+  public void testAcquireLockWithUserProvidedPath() {
+
+    // Initialize lock with user provided path
+    String userID = UUID.randomUUID().toString();
+
+    ZKHelixNonblockingLock lock =
+        new ZKHelixNonblockingLock(_lockPath, ZK_ADDR, Long.MAX_VALUE, _lockMessage, userID);
+
+    //Acquire lock
+    lock.acquireLock();
+    Assert.assertTrue(_gZkClient.exists(_lockPath));
+
+    // Get lock information
+    LockInfo<String> record = lock.getLockInfo();
+    Assert
+        .assertEquals(record.getInfoValue(ZKHelixNonblockingLockInfo.InfoKey.OWNER.name()), userID);
+    Assert.assertEquals(record.getInfoValue(ZKHelixNonblockingLockInfo.InfoKey.MESSAGE.name()),
+        _lockMessage);
+
+    // Check if the user is lock owner
+    Assert.assertTrue(lock.isOwner());
+
+    // Release lock
+    lock.releaseLock();
+    Assert.assertFalse(_gZkClient.exists(_lockPath));
+  }
+
+  @Test
+  public void testAcquireLockWhenExistingLockNotExpired() {
+
+    // Initialize lock with user provided path
+    String ownerID = UUID.randomUUID().toString();
+
+    ZKHelixNonblockingLock lock =
+        new ZKHelixNonblockingLock(_lockPath, ZK_ADDR, 0L, _lockMessage, ownerID);
+
+    // Fake condition when the lock owner is not current user
+    String fakeUserID = UUID.randomUUID().toString();
+    ZNRecord fakeRecord = new ZNRecord(fakeUserID);
+    fakeRecord.setSimpleField(ZKHelixNonblockingLockInfo.InfoKey.OWNER.name(), fakeUserID);
+    fakeRecord.setSimpleField(ZKHelixNonblockingLockInfo.InfoKey.TIMEOUT.name(),
+        String.valueOf(Long.MAX_VALUE));
+    _gZkClient.create(_lockPath, fakeRecord, CreateMode.PERSISTENT);
+
+    // Check if the user is lock owner
+    Assert.assertFalse(lock.isOwner());
+
+    // Acquire lock
+    Assert.assertFalse(lock.acquireLock());
+    Assert.assertFalse(lock.isOwner());
+
+    // Release lock
+    Assert.assertFalse(lock.releaseLock());
+    Assert.assertTrue(_gZkClient.exists(_lockPath));
+
+    _gZkClient.delete(_lockPath);
+  }
+
+  @Test
+  public void testAcquireLockWhenExistingLockExpired() {
+
+    // Initialize lock with user provided path
+    String ownerID = UUID.randomUUID().toString();
+
+    ZKHelixNonblockingLock lock =
+        new ZKHelixNonblockingLock(_lockPath, ZK_ADDR, Long.MAX_VALUE, _lockMessage, ownerID);
+
+    // Fake condition when the current lock already expired
+    String fakeUserID = UUID.randomUUID().toString();
+    ZNRecord fakeRecord = new ZNRecord(fakeUserID);
+    fakeRecord.setSimpleField(ZKHelixNonblockingLockInfo.InfoKey.OWNER.name(), fakeUserID);
+    fakeRecord.setSimpleField(ZKHelixNonblockingLockInfo.InfoKey.TIMEOUT.name(),
+        String.valueOf(System.currentTimeMillis()));
+    _gZkClient.create(_lockPath, fakeRecord, CreateMode.PERSISTENT);
+
+    // Acquire lock
+    Assert.assertTrue(lock.acquireLock());
+    Assert.assertTrue(_gZkClient.exists(_lockPath));
+
+    // Check if the current user is the lock owner
+    Assert.assertTrue(lock.isOwner());
+
+    // Release lock
+    Assert.assertTrue(lock.releaseLock());
+    Assert.assertFalse(_gZkClient.exists(_lockPath));
+  }
+}
+


[helix] 09/19: Added test to acquire lock simultaneously

Posted by jx...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

jxue pushed a commit to branch distributed-lock
in repository https://gitbox.apache.org/repos/asf/helix.git

commit ba3bec9d0f0ab70eb26162d4e08902ae887a4ac0
Author: Molly Gao <mg...@mgao-mn1.linkedin.biz>
AuthorDate: Tue Feb 4 17:38:13 2020 -0800

    Added test to acquire lock simultaneously
---
 .../apache/helix/lock/ZKHelixNonblockingLock.java  |  5 +--
 .../helix/lock/TestZKHelixNonblockingLock.java     | 39 +++++++++++++++++++---
 2 files changed, 36 insertions(+), 8 deletions(-)

diff --git a/helix-lock/src/main/java/org/apache/helix/lock/ZKHelixNonblockingLock.java b/helix-lock/src/main/java/org/apache/helix/lock/ZKHelixNonblockingLock.java
index 44b5aa9..57ff9aa 100644
--- a/helix-lock/src/main/java/org/apache/helix/lock/ZKHelixNonblockingLock.java
+++ b/helix-lock/src/main/java/org/apache/helix/lock/ZKHelixNonblockingLock.java
@@ -152,10 +152,7 @@ public class ZKHelixNonblockingLock implements HelixLock {
     if (curLockInfo == null) {
       return false;
     }
-    if (userIdMatches(curLockInfo)) {
-      return !hasTimedOut(curLockInfo);
-    }
-    return false;
+    return userIdMatches(curLockInfo) && !hasTimedOut(curLockInfo);
   }
 
   /**
diff --git a/helix-lock/src/test/java/org/apache/helix/lock/TestZKHelixNonblockingLock.java b/helix-lock/src/test/java/org/apache/helix/lock/TestZKHelixNonblockingLock.java
index 37daf7b..f2308c1 100644
--- a/helix-lock/src/test/java/org/apache/helix/lock/TestZKHelixNonblockingLock.java
+++ b/helix-lock/src/test/java/org/apache/helix/lock/TestZKHelixNonblockingLock.java
@@ -19,12 +19,21 @@
 
 package org.apache.helix.lock;
 
+import java.util.ArrayList;
 import java.util.Date;
+import java.util.List;
+import java.util.Map;
 import java.util.UUID;
+import java.util.concurrent.Callable;
 
+import org.apache.helix.AccessOption;
+import org.apache.helix.PropertyPathBuilder;
 import org.apache.helix.TestHelper;
 import org.apache.helix.ZNRecord;
 import org.apache.helix.ZkUnitTestBase;
+import org.apache.helix.manager.zk.TestWtCacheAsyncOpMultiThread;
+import org.apache.helix.manager.zk.ZKHelixDataAccessor;
+import org.apache.helix.manager.zk.ZkCacheBaseDataAccessor;
 import org.apache.helix.model.HelixConfigScope;
 import org.apache.helix.model.HelixConfigScope.ConfigScopeProperty;
 import org.apache.helix.model.builder.HelixConfigScopeBuilder;
@@ -44,6 +53,7 @@ public class TestZKHelixNonblockingLock extends ZkUnitTestBase {
   private String _lockPath;
   private ZKHelixNonblockingLock _lock;
   private String _userId;
+  private HelixConfigScope _participantScope;
 
   @BeforeClass
   public void beforeClass() throws Exception {
@@ -53,14 +63,13 @@ public class TestZKHelixNonblockingLock extends ZkUnitTestBase {
     TestHelper.setupCluster(_clusterName, ZK_ADDR, 12918, "localhost", "TestDB", 1, 10, 5, 3,
         "MasterSlave", true);
     _userId = UUID.randomUUID().toString();
-    HelixConfigScope participantScope =
+    _participantScope =
         new HelixConfigScopeBuilder(ConfigScopeProperty.PARTICIPANT).forCluster(_clusterName)
             .forParticipant("localhost_12918").build();
 
-    _lockPath = "/" + _clusterName + '/' + "LOCKS" + '/' + participantScope;
-    _lock = new ZKHelixNonblockingLock(_clusterName, participantScope, ZK_ADDR, Long.MAX_VALUE,
+    _lockPath = "/" + _clusterName + '/' + "LOCKS" + '/' + _participantScope;
+    _lock = new ZKHelixNonblockingLock(_clusterName, _participantScope, ZK_ADDR, Long.MAX_VALUE,
         _lockMessage, _userId);
-
   }
 
   @BeforeMethod
@@ -134,7 +143,29 @@ public class TestZKHelixNonblockingLock extends ZkUnitTestBase {
 
   @Test
   public void testSimultaneousAcquire() {
+    List<Callable<Boolean>> threads = new ArrayList<>();
+    for (int i = 0; i < 2; i++) {
+      ZKHelixNonblockingLock lock =
+          new ZKHelixNonblockingLock(_clusterName, _participantScope, ZK_ADDR, Long.MAX_VALUE,
+              _lockMessage, UUID.randomUUID().toString());
+      threads.add(new TestSimultaneousAcquireLock(lock));
+    }
+    Map<String, Boolean> resultMap = TestHelper.startThreadsConcurrently(threads, 1000);
+    Assert.assertEquals(resultMap.size(), 2);
+    Assert.assertEqualsNoOrder(resultMap.values().toArray(), new Boolean[]{true, false});
+  }
+
+  private static class TestSimultaneousAcquireLock implements Callable<Boolean> {
+    final ZKHelixNonblockingLock _lock;
+
+    TestSimultaneousAcquireLock(ZKHelixNonblockingLock lock) {
+      _lock = lock;
+    }
 
+    @Override
+    public Boolean call() throws Exception {
+      return _lock.acquireLock();
+    }
   }
 }
 


[helix] 18/19: Remove dependency of LockInfo on HelixProperty

Posted by jx...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

jxue pushed a commit to branch distributed-lock
in repository https://gitbox.apache.org/repos/asf/helix.git

commit 1dc7e5932d0b9347af397308cf5b1cf6c6f834fc
Author: Molly Gao <mg...@mgao-mn1.linkedin.biz>
AuthorDate: Tue Feb 18 14:21:15 2020 -0800

    Remove dependency of LockInfo on HelixProperty
---
 .../main/java/org/apache/helix/lock/LockInfo.java  | 29 ++++++++++------------
 .../helix/lock/helix/ZKHelixNonblockingLock.java   |  7 +++---
 2 files changed, 17 insertions(+), 19 deletions(-)

diff --git a/helix-lock/src/main/java/org/apache/helix/lock/LockInfo.java b/helix-lock/src/main/java/org/apache/helix/lock/LockInfo.java
index 250fb71..2c9a24f 100644
--- a/helix-lock/src/main/java/org/apache/helix/lock/LockInfo.java
+++ b/helix-lock/src/main/java/org/apache/helix/lock/LockInfo.java
@@ -19,14 +19,13 @@
 
 package org.apache.helix.lock;
 
-import org.apache.helix.HelixProperty;
 import org.apache.helix.ZNRecord;
 
 
 /**
  * Structure represents a lock node information, implemented using ZNRecord
  */
-public class LockInfo extends HelixProperty {
+public class LockInfo {
 
   // Default values for each attribute if there are no current values set by user
   public static final String DEFAULT_OWNER_TEXT = "";
@@ -37,6 +36,8 @@ public class LockInfo extends HelixProperty {
   public static final LockInfo defaultLockInfo =
       new LockInfo(DEFAULT_OWNER_TEXT, DEFAULT_MESSAGE_TEXT, DEFAULT_TIMEOUT_LONG);
 
+  private ZNRecord record;
+
   /**
    * The keys to lock information
    */
@@ -48,8 +49,8 @@ public class LockInfo extends HelixProperty {
    * Initialize a default LockInfo instance
    */
   private LockInfo() {
-    super("LOCK");
-    setLockInfoFields(null, null, DEFAULT_TIMEOUT_LONG);
+    record = new ZNRecord("LOCK");
+    setLockInfoFields(DEFAULT_OWNER_TEXT, DEFAULT_MESSAGE_TEXT, DEFAULT_TIMEOUT_LONG);
   }
 
   /**
@@ -84,11 +85,11 @@ public class LockInfo extends HelixProperty {
    * @param timeout value of TIMEOUT attribute
    */
   private void setLockInfoFields(String ownerId, String message, long timeout) {
-    _record.setSimpleField(LockInfoAttribute.OWNER.name(),
+    record.setSimpleField(LockInfoAttribute.OWNER.name(),
         ownerId == null ? DEFAULT_OWNER_TEXT : ownerId);
-    _record.setSimpleField(LockInfoAttribute.MESSAGE.name(),
+    record.setSimpleField(LockInfoAttribute.MESSAGE.name(),
         message == null ? DEFAULT_MESSAGE_TEXT : message);
-    _record.setLongField(LockInfoAttribute.TIMEOUT.name(), timeout);
+    record.setLongField(LockInfoAttribute.TIMEOUT.name(), timeout);
   }
 
   /**
@@ -96,7 +97,7 @@ public class LockInfo extends HelixProperty {
    * @return the owner id of the lock, empty string if there is no owner id set
    */
   public String getOwner() {
-    String owner = _record.getSimpleField(LockInfoAttribute.OWNER.name());
+    String owner = record.getSimpleField(LockInfoAttribute.OWNER.name());
     return owner == null ? DEFAULT_OWNER_TEXT : owner;
   }
 
@@ -105,7 +106,7 @@ public class LockInfo extends HelixProperty {
    * @return the message of the lock, empty string if there is no message set
    */
   public String getMessage() {
-    String message = _record.getSimpleField(LockInfoAttribute.MESSAGE.name());
+    String message = record.getSimpleField(LockInfoAttribute.MESSAGE.name());
     return message == null ? DEFAULT_MESSAGE_TEXT : message;
   }
 
@@ -114,14 +115,10 @@ public class LockInfo extends HelixProperty {
    * @return the expiring time of the lock, -1 if there is no timeout set
    */
   public Long getTimeout() {
-    return _record.getLongField(LockInfoAttribute.TIMEOUT.name(), DEFAULT_TIMEOUT_LONG);
+    return record.getLongField(LockInfoAttribute.TIMEOUT.name(), DEFAULT_TIMEOUT_LONG);
   }
 
-  /**
-   * Check if a lock has timed out
-   * @return return true if the lock has timed out, otherwise return false.
-   */
-  public boolean hasNotExpired() {
-    return System.currentTimeMillis() < getTimeout();
+  public ZNRecord getRecord() {
+    return record;
   }
 }
diff --git a/helix-lock/src/main/java/org/apache/helix/lock/helix/ZKHelixNonblockingLock.java b/helix-lock/src/main/java/org/apache/helix/lock/helix/ZKHelixNonblockingLock.java
index 41de4a7..cbfe9da 100644
--- a/helix-lock/src/main/java/org/apache/helix/lock/helix/ZKHelixNonblockingLock.java
+++ b/helix-lock/src/main/java/org/apache/helix/lock/helix/ZKHelixNonblockingLock.java
@@ -110,7 +110,8 @@ public class ZKHelixNonblockingLock implements HelixLock {
   @Override
   public boolean isCurrentOwner() {
     LockInfo lockInfo = getCurrentLockInfo();
-    return lockInfo.getOwner().equals(_userId) && lockInfo.hasNotExpired();
+    return lockInfo.getOwner().equals(_userId) && (System.currentTimeMillis() < lockInfo
+        .getTimeout());
   }
 
   /**
@@ -132,12 +133,12 @@ public class ZKHelixNonblockingLock implements HelixLock {
       // If no one owns the lock, allow the update
       // If the user is the current lock owner, allow the update
       LockInfo curLockInfo = new LockInfo(current);
-      if (!curLockInfo.hasNotExpired() || isCurrentOwner()) {
+      if (!(System.currentTimeMillis() < curLockInfo.getTimeout()) || isCurrentOwner()) {
         return _record;
       }
       // For users who are not the lock owner and try to do an update on a lock that is held by someone else, exception thrown is to be caught by data accessor, and return false for the update
       LOG.error(
-          "User " + _userId + "tried to update the lock at " + new Date(System.currentTimeMillis())
+          "User " + _userId + " tried to update the lock at " + new Date(System.currentTimeMillis())
               + ". Lock path: " + _lockPath);
       throw new HelixException("User is not authorized to perform this operation.");
     }