dmvk commented on a change in pull request #18901:
URL: https://github.com/apache/flink/pull/18901#discussion_r814783286
##########
File path:
flink-kubernetes/src/main/java/org/apache/flink/kubernetes/highavailability/KubernetesStateHandleStore.java
##########
@@ -303,7 +359,12 @@ public StringResourceVersion exists(String key) throws
Exception {
if (optional.isPresent()) {
Review comment:
That sounds incorrect, because then it could backfire in the combination
with addAndLock that is used by the job graph store 🤔
In pseudo code the usage is as follows:
```
while (!success) {
final R currentVersion = jobGraphStateHandleStore.exists(name);
if (!currentVersion.isExisting()) {
try {
jobGraphStateHandleStore.addAndLock(name, jobGraph);
success = true;
} catch (StateHandleStore.AlreadyExistException ignored) {
LOG.warn("{} already exists in {}.", jobGraph,
jobGraphStateHandleStore);
}
} else {
try {
jobGraphStateHandleStore.replace(name, currentVersion,
jobGraph);
success = true;
} catch (StateHandleStore.NotExistException ignored) {
LOG.warn("{} does not exists in {}.", jobGraph,
jobGraphStateHandleStore);
}
}
}
```
I'm also wondering how the replace operation should behave in this case 🤔
Need to dive into this bit more.
--
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: issues-unsubscr...@flink.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
