tillrohrmann commented on a change in pull request #13864:
URL: https://github.com/apache/flink/pull/13864#discussion_r518839277
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/zookeeper/ZooKeeperStateHandleStore.java
##########
@@ -328,7 +336,8 @@ public int exists(String pathInZooKeeper) throws Exception {
* @return True if the state handle could be released
* @throws Exception If the ZooKeeper operation or discarding the state
handle fails
*/
- public boolean releaseAndTryRemove(String pathInZooKeeper) throws
Exception {
+ @Override
+ public boolean removeHandleAndDiscardState(String pathInZooKeeper)
throws Exception {
Review comment:
Maybe rename to `releaseAndTryRemove` or `releaseAndTryDiscard`
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/zookeeper/ZooKeeperStateHandleStore.java
##########
@@ -389,7 +399,8 @@ public void releaseAndTryRemoveAll() throws Exception {
* @param pathInZooKeeper Path describing the ZooKeeper node
* @throws Exception if the delete operation of the lock node fails
*/
- public void release(String pathInZooKeeper) throws Exception {
+ @Override
+ public void releaseHandle(String pathInZooKeeper) throws Exception {
Review comment:
Maybe we should call this `releaseLock` or `release`.
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/zookeeper/ZooKeeperStateHandleStore.java
##########
@@ -406,14 +417,15 @@ public void release(String pathInZooKeeper) throws
Exception {
*
* @throws Exception if the delete operation of a lock file fails
*/
- public void releaseAll() throws Exception {
- Collection<String> children = getAllPaths();
+ @Override
+ public void releaseAllHandles() throws Exception {
Review comment:
Maybe better to call them `releaseAllLocks` or `releaseAll`.
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/zookeeper/ZooKeeperStateHandleStore.java
##########
@@ -429,12 +441,18 @@ public void releaseAll() throws Exception {
*
* @throws Exception ZK errors
*/
- public void deleteChildren() throws Exception {
+ @Override
+ public void removeAllHandles() throws Exception {
Review comment:
Maybe `clearEntries`
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/zookeeper/ZooKeeperStateHandleStore.java
##########
@@ -365,14 +374,15 @@ public boolean releaseAndTryRemove(String
pathInZooKeeper) throws Exception {
*
* @throws Exception if the delete operation fails
*/
- public void releaseAndTryRemoveAll() throws Exception {
- Collection<String> children = getAllPaths();
+ @Override
+ public void removeAllHandlesAndDiscardState() throws Exception {
Review comment:
same here: `releaseAndTryRemoveAll` or `releaseAndTryDiscardAll` or
`releaseLockAndTryRemoveAll` could be better given that we have the
`getAndLock` method again.
----------------------------------------------------------------
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]
