You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@helix.apache.org by "Marcosrico (via GitHub)" <gi...@apache.org> on 2023/03/27 15:36:11 UTC
[GitHub] [helix] Marcosrico opened a new pull request, #2421: MetaClient LockClient Interface
Marcosrico opened a new pull request, #2421:
URL: https://github.com/apache/helix/pull/2421
### Issues
- [ ] My PR addresses the following Helix issues and references them in the PR description:
#2237
### Description
- [ ] Here are some details about my PR, including screenshots of any UI changes:
Added LockClient interface with key methods. Added a LockInfo object class that will be implemented in a following PR. For now it is just a placeholder.
### Tests
- [ ] The following tests are written for this issue:
(List the names of added unit/integration tests)
- The following is the result of the "mvn test" command on the appropriate module:
(If CI test fails due to known issue, please specify the issue and test PR locally. Then copy & paste the result of "mvn test" to here.)
### Code Quality
- My diff has been formatted using helix-style.xml
(helix-style-intellij.xml if IntelliJ IDE is used)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] mgao0 commented on a diff in pull request #2421: MetaClient LockClient Interface
Posted by "mgao0 (via GitHub)" <gi...@apache.org>.
mgao0 commented on code in PR #2421:
URL: https://github.com/apache/helix/pull/2421#discussion_r1152526696
##########
meta-client/src/main/java/org/apache/helix/metaclient/recipes/lock/LockClientInterface.java:
##########
@@ -0,0 +1,58 @@
+package org.apache.helix.metaclient.recipes.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.
+ */
+
+import org.apache.helix.metaclient.api.MetaClientInterface;
+
+public interface LockClientInterface {
+ /**
+ * Acquires a lock at key.
+ * @param key key to identify the entry
+ * @param info Metadata of the lock
+ * @param mode EntryMode identifying if the entry will be deleted upon client disconnect
Review Comment:
Maybe we can mention what are the entry modes, might be easier to understand.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] qqu0127 commented on a diff in pull request #2421: MetaClient LockClient Interface
Posted by "qqu0127 (via GitHub)" <gi...@apache.org>.
qqu0127 commented on code in PR #2421:
URL: https://github.com/apache/helix/pull/2421#discussion_r1152135781
##########
meta-client/src/main/java/org/apache/helix/metaclient/recipes/lock/LockClientInterface.java:
##########
@@ -0,0 +1,62 @@
+package org.apache.helix.metaclient.recipes.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.
+ */
+
+import org.apache.helix.metaclient.api.MetaClientInterface;
+
+public interface LockClientInterface {
+ /**
+ * Acquires a lock by creating a node at entry key and lockinfo info.
+ * Will fail and return False if path and lockinfo is invalid.
+ * @param key key to identify the entry
+ * @param info Metadata of the lock
+ * @param mode EntryMode identifying if the entry will be deleted upon client disconnect
+ * @return True if the lock is acquired. False if failed to acquire (catches exception).
+ */
+ boolean acquireLock(String key, LockInfo info, MetaClientInterface.EntryMode mode);
+
+ /**
+ * Renews lock for a TTL Node.
+ * Will fail if key is an invalid path or isn't of type TTL.
+ * @param key key to identify the entry
+ * @return True if the lock was renews. False if failed to renew (catches exception).
+ */
+ boolean renewTTLLock(String key);
+
+ /**
+ * Releases the lock by deleted the node at entry key.
+ * Will fail if key is an invalid path.
+ * @param key key to identify the entry
+ * @return True if the lock was released. False if failed to release (catches exception).
+ */
+ boolean releaseLock(String key);
Review Comment:
@Marcosrico thanks for the reply.
Yes, there could be different implementations. But the idea is, we are now defining the interface, thus now is the time we define the behavior and semantics.
Back to the `releaseLock` behavior, idempotent API will have some benefits for simplifying logic especially in case of retry. It could be that, as long as the lock has been released, it returns true or success.
IMO, it's just one way to define the behavior, and there are tradeoff. But the bottomline is we should document the expected behavior clearly (or clearly say it's completely up to the impl). We can discuss more on this 😄
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] Marcosrico commented on a diff in pull request #2421: MetaClient LockClient Interface
Posted by "Marcosrico (via GitHub)" <gi...@apache.org>.
Marcosrico commented on code in PR #2421:
URL: https://github.com/apache/helix/pull/2421#discussion_r1151015702
##########
meta-client/src/main/java/org/apache/helix/metaclient/recipes/lock/LockClientInterface.java:
##########
@@ -0,0 +1,62 @@
+package org.apache.helix.metaclient.recipes.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.
+ */
+
+import org.apache.helix.metaclient.api.MetaClientInterface;
+
+public interface LockClientInterface {
+ /**
+ * Acquires a lock by creating a node at entry key and lockinfo info.
+ * Will fail and return False if path and lockinfo is invalid.
+ * @param key key to identify the entry
+ * @param info Metadata of the lock
+ * @param mode EntryMode identifying if the entry will be deleted upon client disconnect
+ * @return True if the lock is acquired. False if failed to acquire (catches exception).
+ */
+ boolean acquireLock(String key, LockInfo info, MetaClientInterface.EntryMode mode);
+
+ /**
+ * Renews lock for a TTL Node.
+ * Will fail if key is an invalid path or isn't of type TTL.
+ * @param key key to identify the entry
+ * @return True if the lock was renews. False if failed to renew (catches exception).
+ */
+ boolean renewTTLLock(String key);
+
+ /**
+ * Releases the lock by deleted the node at entry key.
+ * Will fail if key is an invalid path.
+ * @param key key to identify the entry
+ * @return True if the lock was released. False if failed to release (catches exception).
+ */
+ boolean releaseLock(String key);
+
+ /**
+ * Obtains the metadata of a lock (the LockInfo).
+ * @param key key to identify the entry
+ * @return LockInfo object of the node at key. If key doesn't exist, raise exception.
Review Comment:
Sounds good. I think returning null makes most sense
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] Marcosrico commented on a diff in pull request #2421: MetaClient LockClient Interface
Posted by "Marcosrico (via GitHub)" <gi...@apache.org>.
Marcosrico commented on code in PR #2421:
URL: https://github.com/apache/helix/pull/2421#discussion_r1152093145
##########
meta-client/src/main/java/org/apache/helix/metaclient/recipes/lock/LockClientInterface.java:
##########
@@ -0,0 +1,62 @@
+package org.apache.helix.metaclient.recipes.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.
+ */
+
+import org.apache.helix.metaclient.api.MetaClientInterface;
+
+public interface LockClientInterface {
+ /**
+ * Acquires a lock by creating a node at entry key and lockinfo info.
+ * Will fail and return False if path and lockinfo is invalid.
+ * @param key key to identify the entry
+ * @param info Metadata of the lock
+ * @param mode EntryMode identifying if the entry will be deleted upon client disconnect
+ * @return True if the lock is acquired. False if failed to acquire (catches exception).
+ */
+ boolean acquireLock(String key, LockInfo info, MetaClientInterface.EntryMode mode);
+
+ /**
+ * Renews lock for a TTL Node.
+ * Will fail if key is an invalid path or isn't of type TTL.
+ * @param key key to identify the entry
+ * @return True if the lock was renews. False if failed to renew (catches exception).
+ */
+ boolean renewTTLLock(String key);
+
+ /**
+ * Releases the lock by deleted the node at entry key.
+ * Will fail if key is an invalid path.
+ * @param key key to identify the entry
+ * @return True if the lock was released. False if failed to release (catches exception).
+ */
+ boolean releaseLock(String key);
Review Comment:
@qqu0127 This I think depends on the implementation of release lock, though the expected behavior of releasing a lock on one that either doesn't exist or already has been released should error. As per the design, releaslock releases the lock using transactional support to avoid race conditions.
@xyuanlu I think this is related to @mgao0 question above on creating a separate class to surface exceptions/errors. Depending on the implementation, that will or will not differentiate between those two errors (though I think it is a good idea to too). With a separate error/result wrapper class we can capture this error and return it, regardless of the underlying datastore.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] Marcosrico commented on a diff in pull request #2421: MetaClient LockClient Interface
Posted by "Marcosrico (via GitHub)" <gi...@apache.org>.
Marcosrico commented on code in PR #2421:
URL: https://github.com/apache/helix/pull/2421#discussion_r1151016108
##########
meta-client/src/main/java/org/apache/helix/metaclient/recipes/lock/LockClientInterface.java:
##########
@@ -0,0 +1,62 @@
+package org.apache.helix.metaclient.recipes.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.
+ */
+
+import org.apache.helix.metaclient.api.MetaClientInterface;
+
+public interface LockClientInterface {
+ /**
+ * Acquires a lock by creating a node at entry key and lockinfo info.
Review Comment:
Got it, I'll generalize the descriptions. Thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] xyuanlu commented on a diff in pull request #2421: MetaClient LockClient Interface
Posted by "xyuanlu (via GitHub)" <gi...@apache.org>.
xyuanlu commented on code in PR #2421:
URL: https://github.com/apache/helix/pull/2421#discussion_r1151288970
##########
meta-client/src/main/java/org/apache/helix/metaclient/recipes/lock/LockClientInterface.java:
##########
@@ -0,0 +1,62 @@
+package org.apache.helix.metaclient.recipes.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.
+ */
+
+import org.apache.helix.metaclient.api.MetaClientInterface;
+
+public interface LockClientInterface {
+ /**
+ * Acquires a lock by creating a node at entry key and lockinfo info.
+ * Will fail and return False if path and lockinfo is invalid.
+ * @param key key to identify the entry
+ * @param info Metadata of the lock
+ * @param mode EntryMode identifying if the entry will be deleted upon client disconnect
+ * @return True if the lock is acquired. False if failed to acquire (catches exception).
+ */
+ boolean acquireLock(String key, LockInfo info, MetaClientInterface.EntryMode mode);
+
+ /**
+ * Renews lock for a TTL Node.
+ * Will fail if key is an invalid path or isn't of type TTL.
+ * @param key key to identify the entry
+ * @return True if the lock was renews. False if failed to renew (catches exception).
+ */
+ boolean renewTTLLock(String key);
+
+ /**
+ * Releases the lock by deleted the node at entry key.
+ * Will fail if key is an invalid path.
+ * @param key key to identify the entry
+ * @return True if the lock was released. False if failed to release (catches exception).
+ */
+ boolean releaseLock(String key);
Review Comment:
+1 on qqu's comment.
Probably we want more detail regarding what does "invalid path" mean here.
When releasing an lock doe second time, do we want to differentiate already released lock or non-existing lock? It is hard to do when underlying datastore does remove lock entry when released.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] mgao0 commented on a diff in pull request #2421: MetaClient LockClient Interface
Posted by "mgao0 (via GitHub)" <gi...@apache.org>.
mgao0 commented on code in PR #2421:
URL: https://github.com/apache/helix/pull/2421#discussion_r1149822130
##########
meta-client/src/main/java/org/apache/helix/metaclient/recipes/lock/LockClientInterface.java:
##########
@@ -0,0 +1,62 @@
+package org.apache.helix.metaclient.recipes.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.
+ */
+
+import org.apache.helix.metaclient.api.MetaClientInterface;
+
+public interface LockClientInterface {
+ /**
+ * Acquires a lock by creating a node at entry key and lockinfo info.
+ * Will fail and return False if path and lockinfo is invalid.
+ * @param key key to identify the entry
+ * @param info Metadata of the lock
+ * @param mode EntryMode identifying if the entry will be deleted upon client disconnect
+ * @return True if the lock is acquired. False if failed to acquire (catches exception).
+ */
+ boolean acquireLock(String key, LockInfo info, MetaClientInterface.EntryMode mode);
+
+ /**
+ * Renews lock for a TTL Node.
+ * Will fail if key is an invalid path or isn't of type TTL.
+ * @param key key to identify the entry
+ * @return True if the lock was renews. False if failed to renew (catches exception).
Review Comment:
nit: did you mean "renewed"?
##########
meta-client/src/main/java/org/apache/helix/metaclient/recipes/lock/LockClientInterface.java:
##########
@@ -0,0 +1,62 @@
+package org.apache.helix.metaclient.recipes.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.
+ */
+
+import org.apache.helix.metaclient.api.MetaClientInterface;
+
+public interface LockClientInterface {
+ /**
+ * Acquires a lock by creating a node at entry key and lockinfo info.
+ * Will fail and return False if path and lockinfo is invalid.
+ * @param key key to identify the entry
+ * @param info Metadata of the lock
+ * @param mode EntryMode identifying if the entry will be deleted upon client disconnect
+ * @return True if the lock is acquired. False if failed to acquire (catches exception).
+ */
+ boolean acquireLock(String key, LockInfo info, MetaClientInterface.EntryMode mode);
+
+ /**
+ * Renews lock for a TTL Node.
+ * Will fail if key is an invalid path or isn't of type TTL.
+ * @param key key to identify the entry
+ * @return True if the lock was renews. False if failed to renew (catches exception).
+ */
+ boolean renewTTLLock(String key);
+
+ /**
+ * Releases the lock by deleted the node at entry key.
Review Comment:
Same as the first comment: Delete the node or not I think it depends on the underlying data store. Users just need to know that lock is released.
##########
meta-client/src/main/java/org/apache/helix/metaclient/recipes/lock/LockClientInterface.java:
##########
@@ -0,0 +1,62 @@
+package org.apache.helix.metaclient.recipes.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.
+ */
+
+import org.apache.helix.metaclient.api.MetaClientInterface;
+
+public interface LockClientInterface {
+ /**
+ * Acquires a lock by creating a node at entry key and lockinfo info.
+ * Will fail and return False if path and lockinfo is invalid.
+ * @param key key to identify the entry
+ * @param info Metadata of the lock
+ * @param mode EntryMode identifying if the entry will be deleted upon client disconnect
+ * @return True if the lock is acquired. False if failed to acquire (catches exception).
+ */
+ boolean acquireLock(String key, LockInfo info, MetaClientInterface.EntryMode mode);
+
+ /**
+ * Renews lock for a TTL Node.
+ * Will fail if key is an invalid path or isn't of type TTL.
+ * @param key key to identify the entry
+ * @return True if the lock was renews. False if failed to renew (catches exception).
+ */
+ boolean renewTTLLock(String key);
+
+ /**
+ * Releases the lock by deleted the node at entry key.
+ * Will fail if key is an invalid path.
+ * @param key key to identify the entry
+ * @return True if the lock was released. False if failed to release (catches exception).
+ */
+ boolean releaseLock(String key);
+
+ /**
+ * Obtains the metadata of a lock (the LockInfo).
+ * @param key key to identify the entry
+ * @return LockInfo object of the node at key. If key doesn't exist, raise exception.
Review Comment:
We can discuss about this - I think we just need to return an empty LockInfo object or null if the key doesn't exist, usually it's nothing alerting.
##########
meta-client/src/main/java/org/apache/helix/metaclient/recipes/lock/LockClientInterface.java:
##########
@@ -0,0 +1,62 @@
+package org.apache.helix.metaclient.recipes.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.
+ */
+
+import org.apache.helix.metaclient.api.MetaClientInterface;
+
+public interface LockClientInterface {
+ /**
+ * Acquires a lock by creating a node at entry key and lockinfo info.
+ * Will fail and return False if path and lockinfo is invalid.
+ * @param key key to identify the entry
+ * @param info Metadata of the lock
+ * @param mode EntryMode identifying if the entry will be deleted upon client disconnect
+ * @return True if the lock is acquired. False if failed to acquire (catches exception).
+ */
+ boolean acquireLock(String key, LockInfo info, MetaClientInterface.EntryMode mode);
Review Comment:
There is nothing wrong with your code, this matches our current design. But I had a second thought about it and want to start a discussion with @Marcosrico @xyuanlu , do you think it's worthy to create a class for the operation result (two fields: boolean succeed, enum for return code)? Since we won't be able to surface the failure reason to users in this current way.
##########
meta-client/src/main/java/org/apache/helix/metaclient/recipes/lock/LockClientInterface.java:
##########
@@ -0,0 +1,62 @@
+package org.apache.helix.metaclient.recipes.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.
+ */
+
+import org.apache.helix.metaclient.api.MetaClientInterface;
+
+public interface LockClientInterface {
+ /**
+ * Acquires a lock by creating a node at entry key and lockinfo info.
Review Comment:
The internal logic of lock client can vary for different data stores, for example, for some data stores, we create a node for acquiring a lock, and for others, we may just update the znode data for acquiring a lock. So you may modify this statement a little.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] Marcosrico commented on pull request #2421: MetaClient LockClient Interface
Posted by "Marcosrico (via GitHub)" <gi...@apache.org>.
Marcosrico commented on PR #2421:
URL: https://github.com/apache/helix/pull/2421#issuecomment-1490639837
Pull Request approved by @mgao0
Commit Message:
MetaClient LockClient Interface
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] Marcosrico commented on a diff in pull request #2421: MetaClient LockClient Interface
Posted by "Marcosrico (via GitHub)" <gi...@apache.org>.
Marcosrico commented on code in PR #2421:
URL: https://github.com/apache/helix/pull/2421#discussion_r1153548707
##########
meta-client/src/main/java/org/apache/helix/metaclient/recipes/lock/LockClientInterface.java:
##########
@@ -0,0 +1,58 @@
+package org.apache.helix.metaclient.recipes.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.
+ */
+
+import org.apache.helix.metaclient.api.MetaClientInterface;
+
+public interface LockClientInterface {
+ /**
+ * Acquires a lock at key.
+ * @param key key to identify the entry
+ * @param info Metadata of the lock
+ * @param mode EntryMode identifying if the entry will be deleted upon client disconnect
Review Comment:
Sounds good, addressed!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] qqu0127 commented on a diff in pull request #2421: MetaClient LockClient Interface
Posted by "qqu0127 (via GitHub)" <gi...@apache.org>.
qqu0127 commented on code in PR #2421:
URL: https://github.com/apache/helix/pull/2421#discussion_r1150737923
##########
meta-client/src/main/java/org/apache/helix/metaclient/recipes/lock/LockClientInterface.java:
##########
@@ -0,0 +1,62 @@
+package org.apache.helix.metaclient.recipes.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.
+ */
+
+import org.apache.helix.metaclient.api.MetaClientInterface;
+
+public interface LockClientInterface {
+ /**
+ * Acquires a lock by creating a node at entry key and lockinfo info.
+ * Will fail and return False if path and lockinfo is invalid.
+ * @param key key to identify the entry
+ * @param info Metadata of the lock
+ * @param mode EntryMode identifying if the entry will be deleted upon client disconnect
+ * @return True if the lock is acquired. False if failed to acquire (catches exception).
+ */
+ boolean acquireLock(String key, LockInfo info, MetaClientInterface.EntryMode mode);
+
+ /**
+ * Renews lock for a TTL Node.
+ * Will fail if key is an invalid path or isn't of type TTL.
+ * @param key key to identify the entry
+ * @return True if the lock was renews. False if failed to renew (catches exception).
+ */
+ boolean renewTTLLock(String key);
+
+ /**
+ * Releases the lock by deleted the node at entry key.
+ * Will fail if key is an invalid path.
+ * @param key key to identify the entry
+ * @return True if the lock was released. False if failed to release (catches exception).
+ */
+ boolean releaseLock(String key);
+
+ /**
+ * Obtains the metadata of a lock (the LockInfo).
+ * @param key key to identify the entry
+ * @return LockInfo object of the node at key. If key doesn't exist, raise exception.
+ */
+ LockInfo retrieveLock(String key);
+
+ /**
+ * Closes the underlying client.
+ */
+ void close();
Review Comment:
iMHO, this doesn't need to be included. We can have it by implementing `AutoCloseable`
##########
meta-client/src/main/java/org/apache/helix/metaclient/recipes/lock/LockClientInterface.java:
##########
@@ -0,0 +1,62 @@
+package org.apache.helix.metaclient.recipes.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.
+ */
+
+import org.apache.helix.metaclient.api.MetaClientInterface;
+
+public interface LockClientInterface {
+ /**
+ * Acquires a lock by creating a node at entry key and lockinfo info.
+ * Will fail and return False if path and lockinfo is invalid.
+ * @param key key to identify the entry
+ * @param info Metadata of the lock
+ * @param mode EntryMode identifying if the entry will be deleted upon client disconnect
+ * @return True if the lock is acquired. False if failed to acquire (catches exception).
+ */
+ boolean acquireLock(String key, LockInfo info, MetaClientInterface.EntryMode mode);
+
+ /**
+ * Renews lock for a TTL Node.
+ * Will fail if key is an invalid path or isn't of type TTL.
+ * @param key key to identify the entry
+ * @return True if the lock was renews. False if failed to renew (catches exception).
+ */
+ boolean renewTTLLock(String key);
+
+ /**
+ * Releases the lock by deleted the node at entry key.
+ * Will fail if key is an invalid path.
+ * @param key key to identify the entry
+ * @return True if the lock was released. False if failed to release (catches exception).
+ */
+ boolean releaseLock(String key);
Review Comment:
Is this method idempotent? What will happen if user release a lock multiple times?
##########
meta-client/src/main/java/org/apache/helix/metaclient/recipes/lock/LockClientInterface.java:
##########
@@ -0,0 +1,62 @@
+package org.apache.helix.metaclient.recipes.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.
+ */
+
+import org.apache.helix.metaclient.api.MetaClientInterface;
+
+public interface LockClientInterface {
+ /**
+ * Acquires a lock by creating a node at entry key and lockinfo info.
+ * Will fail and return False if path and lockinfo is invalid.
+ * @param key key to identify the entry
+ * @param info Metadata of the lock
+ * @param mode EntryMode identifying if the entry will be deleted upon client disconnect
+ * @return True if the lock is acquired. False if failed to acquire (catches exception).
+ */
+ boolean acquireLock(String key, LockInfo info, MetaClientInterface.EntryMode mode);
Review Comment:
+1 on this. There could be different reasons a lock can't be acquired. In the future with LockIn as a service, we may want to match the result to a finer status code too.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] xyuanlu merged pull request #2421: MetaClient LockClient Interface
Posted by "xyuanlu (via GitHub)" <gi...@apache.org>.
xyuanlu merged PR #2421:
URL: https://github.com/apache/helix/pull/2421
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] qqu0127 commented on a diff in pull request #2421: MetaClient LockClient Interface
Posted by "qqu0127 (via GitHub)" <gi...@apache.org>.
qqu0127 commented on code in PR #2421:
URL: https://github.com/apache/helix/pull/2421#discussion_r1150752536
##########
meta-client/src/main/java/org/apache/helix/metaclient/recipes/lock/LockClientInterface.java:
##########
@@ -0,0 +1,62 @@
+package org.apache.helix.metaclient.recipes.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.
+ */
+
+import org.apache.helix.metaclient.api.MetaClientInterface;
+
+public interface LockClientInterface {
+ /**
+ * Acquires a lock by creating a node at entry key and lockinfo info.
+ * Will fail and return False if path and lockinfo is invalid.
+ * @param key key to identify the entry
+ * @param info Metadata of the lock
+ * @param mode EntryMode identifying if the entry will be deleted upon client disconnect
+ * @return True if the lock is acquired. False if failed to acquire (catches exception).
+ */
+ boolean acquireLock(String key, LockInfo info, MetaClientInterface.EntryMode mode);
Review Comment:
+1 on this. There could be different reasons a lock can't be acquired. In the future with distributed lock as a service, we may want to match the result to a finer status code too.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] Marcosrico commented on a diff in pull request #2421: MetaClient LockClient Interface
Posted by "Marcosrico (via GitHub)" <gi...@apache.org>.
Marcosrico commented on code in PR #2421:
URL: https://github.com/apache/helix/pull/2421#discussion_r1151100409
##########
meta-client/src/main/java/org/apache/helix/metaclient/recipes/lock/LockClientInterface.java:
##########
@@ -0,0 +1,62 @@
+package org.apache.helix.metaclient.recipes.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.
+ */
+
+import org.apache.helix.metaclient.api.MetaClientInterface;
+
+public interface LockClientInterface {
+ /**
+ * Acquires a lock by creating a node at entry key and lockinfo info.
+ * Will fail and return False if path and lockinfo is invalid.
+ * @param key key to identify the entry
+ * @param info Metadata of the lock
+ * @param mode EntryMode identifying if the entry will be deleted upon client disconnect
+ * @return True if the lock is acquired. False if failed to acquire (catches exception).
+ */
+ boolean acquireLock(String key, LockInfo info, MetaClientInterface.EntryMode mode);
+
+ /**
+ * Renews lock for a TTL Node.
+ * Will fail if key is an invalid path or isn't of type TTL.
+ * @param key key to identify the entry
+ * @return True if the lock was renews. False if failed to renew (catches exception).
+ */
+ boolean renewTTLLock(String key);
+
+ /**
+ * Releases the lock by deleted the node at entry key.
+ * Will fail if key is an invalid path.
+ * @param key key to identify the entry
+ * @return True if the lock was released. False if failed to release (catches exception).
+ */
+ boolean releaseLock(String key);
+
+ /**
+ * Obtains the metadata of a lock (the LockInfo).
+ * @param key key to identify the entry
+ * @return LockInfo object of the node at key. If key doesn't exist, raise exception.
+ */
+ LockInfo retrieveLock(String key);
+
+ /**
+ * Closes the underlying client.
+ */
+ void close();
Review Comment:
Sounds good.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] Marcosrico commented on a diff in pull request #2421: MetaClient LockClient Interface
Posted by "Marcosrico (via GitHub)" <gi...@apache.org>.
Marcosrico commented on code in PR #2421:
URL: https://github.com/apache/helix/pull/2421#discussion_r1150628083
##########
meta-client/src/main/java/org/apache/helix/metaclient/recipes/lock/LockClientInterface.java:
##########
@@ -0,0 +1,62 @@
+package org.apache.helix.metaclient.recipes.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.
+ */
+
+import org.apache.helix.metaclient.api.MetaClientInterface;
+
+public interface LockClientInterface {
+ /**
+ * Acquires a lock by creating a node at entry key and lockinfo info.
+ * Will fail and return False if path and lockinfo is invalid.
+ * @param key key to identify the entry
+ * @param info Metadata of the lock
+ * @param mode EntryMode identifying if the entry will be deleted upon client disconnect
+ * @return True if the lock is acquired. False if failed to acquire (catches exception).
+ */
+ boolean acquireLock(String key, LockInfo info, MetaClientInterface.EntryMode mode);
+
+ /**
+ * Renews lock for a TTL Node.
+ * Will fail if key is an invalid path or isn't of type TTL.
+ * @param key key to identify the entry
+ * @return True if the lock was renews. False if failed to renew (catches exception).
Review Comment:
Yup, typo. Thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org