wangyang0918 commented on a change in pull request #13864:
URL: https://github.com/apache/flink/pull/13864#discussion_r517744674
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/jobmanager/ZooKeeperJobGraphStore.java
##########
@@ -63,35 +45,16 @@
* <p>The root path is watched to detect concurrent modifications in corner
situations where
* multiple instances operate concurrently. The job manager acts as a {@link
JobGraphListener}
* to react to such situations.
+ *
+ * <p>NOTICE: The only reason we still have this class is that we need to
release the lock. If we completely get
+ * rid of current lock-and-lease to avoid concurrent modification, like
Kubernetes, then this class could be
+ * directly removed.
*/
-public class ZooKeeperJobGraphStore implements JobGraphStore {
-
- private static final Logger LOG =
LoggerFactory.getLogger(ZooKeeperJobGraphStore.class);
-
- /** Lock to synchronize with the {@link JobGraphListener}. */
- private final Object cacheLock = new Object();
-
- /** The set of IDs of all added job graphs. */
- private final Set<JobID> addedJobGraphs = new HashSet<>();
+public class ZooKeeperJobGraphStore extends DefaultJobGraphStore {
Review comment:
Hmm. In the first version of implementation, I have defined `release` in
the `StateHandleStore`. But after struggling for some time, I think it is not
very clean since `lock-and-release` is not a common operation for different
implementation. Also maybe in the future we could re-implement the ZooKeeper HA
related service to get rid of `lock-and-release`. For example, we use a same
ZooKeeper node for contending and storing leader information. Then we
"check-leadership-and-update" ZooKeeper node with versioned write. I have not
dig into the details but I think we could do 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.
For queries about this service, please contact Infrastructure at:
[email protected]
