tillrohrmann commented on a change in pull request #13864:
URL: https://github.com/apache/flink/pull/13864#discussion_r517940542
##########
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:
Sure, the thing is that one of the `StateHandleStore` implementations
needs this pattern and the other can live w/o it. Since we want to support both
variants at the moment, wouldn't the superset of functionality be the easier
solution than injecting some callbacks to modify the behaviour? I will take a
look at the current solution to see how maintainable it is.
----------------------------------------------------------------
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]
