flink-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [flink] XComp commented on a change in pull request #15832: [FLINK-22494][ha] Introduce PossibleInconsistentStateException
Date Tue, 11 May 2021 09:52:09 GMT

XComp commented on a change in pull request #15832:
URL: https://github.com/apache/flink/pull/15832#discussion_r630021216

File path: flink-kubernetes/src/main/java/org/apache/flink/kubernetes/highavailability/KubernetesStateHandleStore.java
@@ -114,21 +118,28 @@ public KubernetesStateHandleStore(
      * @param key Key in ConfigMap
      * @param state State to be added
      * @throws AlreadyExistException if the name already exists
+     * @throws PossibleInconsistentStateException if the write-to-Kubernetes operation failed.
+     *     indicates that it's not clear whether the new state was successfully written to
+     *     Kubernetes or not. No state was discarded. Proper error handling has to be applied
on the
+     *     caller's side.
      * @throws Exception if persisting state or writing state handle failed
-    public RetrievableStateHandle<T> addAndLock(String key, T state) throws Exception
+    public RetrievableStateHandle<T> addAndLock(String key, T state)
+            throws PossibleInconsistentStateException, Exception {
         checkNotNull(key, "Key in ConfigMap.");
         checkNotNull(state, "State.");
         final RetrievableStateHandle<T> storeHandle = storage.store(state);
-        boolean success = false;
+        final byte[] serializedStoreHandle = serializeStateHandle(storeHandle);
+        // initialize flag to serve the failure case
+        boolean discardState = true;
         try {
-            final byte[] serializedStoreHandle = InstantiationUtil.serializeObject(storeHandle);
-            success =
-                    kubeClient
+            // a successful operation will result in the state not being discarded
+            discardState =
+                    !kubeClient

Review comment:
       I thought about it once more. The initial motivation to change that came from the fact
that we are introducing an error case which is handled as if the method was successful. Using
the `success` flag in that case would have been misleading. Alternatively, I could have created
a new flag to workaround this double meaning of the same flag. But I'm gonna keep it like
that for now...

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:

View raw message