XComp commented on a change in pull request #18869:
URL: https://github.com/apache/flink/pull/18869#discussion_r813931306
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/zookeeper/ZooKeeperStateHandleStore.java
##########
@@ -529,6 +579,22 @@ public String toString() {
// Protected methods
//
---------------------------------------------------------------------------------------------------------
+ /**
+ * Checks whether a lock is created for this instance on the passed
ZooKeeper node.
+ *
+ * @param rootPath The node that shall be checked.
+ * @return {@code true} if the lock exists; {@code false} otherwise.
+ */
+ private boolean hasLock(String rootPath) throws Exception {
+ final String normalizedRootPath = normalizePath(rootPath);
+ try {
+ return
client.checkExists().forPath(getLockPath(normalizedRootPath)) != null;
Review comment:
This was meant for hardening the implementation. In the current setup it
cannot happen, because this method is only called by `replace` which is only
used in `DefaultJobGraphStore` when the corresponding `JobGraph` is already
added to `DefaultJobGraphStore.addedJobGraphs` (which means that the JobGraph
was already locked before through `getAndLock` in
`DefaultJobGraphStore.recoverJobGraph` or `addAndLock` in
`DefaultJobGraphStore.putJobGraph`)
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/zookeeper/ZooKeeperStateHandleStore.java
##########
@@ -529,6 +579,22 @@ public String toString() {
// Protected methods
//
---------------------------------------------------------------------------------------------------------
+ /**
+ * Checks whether a lock is created for this instance on the passed
ZooKeeper node.
+ *
+ * @param rootPath The node that shall be checked.
+ * @return {@code true} if the lock exists; {@code false} otherwise.
+ */
+ private boolean hasLock(String rootPath) throws Exception {
+ final String normalizedRootPath = normalizePath(rootPath);
+ try {
+ return
client.checkExists().forPath(getLockPath(normalizedRootPath)) != null;
Review comment:
This was meant for hardening the implementation. In the current setup it
cannot happen, because this method is only called by `replace` which is only
used in `DefaultJobGraphStore` when the corresponding `JobGraph` is already
added to `DefaultJobGraphStore.addedJobGraphs` (which means that the JobGraph
was already locked before through `getAndLock` in
`DefaultJobGraphStore.recoverJobGraph` or `addAndLock` in
`DefaultJobGraphStore.putJobGraph`)
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
