saurabhkanawadenice commented on code in PR #1239:
URL: 
https://github.com/apache/flink-kubernetes-operator/pull/1239#discussion_r4436129533


##########
flink-kubernetes-operator/src/test/java/org/apache/flink/kubernetes/operator/controller/FlinkDeploymentControllerTest.java:
##########
@@ -438,6 +439,61 @@ public void 
verifyUpgradeFromSavepointLegacyMode(FlinkVersion flinkVersion) thro
         assertEquals(0, jobs.size());
     }
 
+    @Test
+    public void testFailedJobRestoredFromLatestSavepointAfterUpgrade() throws 
Exception {

Review Comment:
   
   Review point: The fix only covered the legacy snapshot mode. With 
FlinkStateSnapshot resources (the default), jobs still restore
   from the old upgrade savepoint, or silently from old state if the file still 
exists.
   What I did: Fixed. While the job runs, upgradeSavepointPath now moves to the 
newest completed periodic savepoint that was triggered
   after the job started. Older ones came from a previous job and are ignored. 
I kept restoreJob as it is: at restore time it can't tell which savepoint is 
newer.
   
   Review point: The last-state marker is kept even when the HA metadata is 
lost. Is that intended?
   What I did: Yes. I reworded the code comment: stopping for a manual restore 
is safer than picking a savepoint that may be older than
   the last checkpoint.
   
   Review point: Add tests for kubernetes.operator.job.restart.failed and 
FlinkSessionJob
   What I did: Added. The deployment test covers bothade and with 
restart.failed. The session job test
   covers both modes.
   
   Review point: Which of the two failures in the JIRA does the PR fix?
   What I did: The second one: the operator restoringfirst one comes from Flink 
losing the HA checkpoints
   (FLINK-34518), which the operator can't fix. This
   
   Review point: The docs still say periodic and manuckups
   What I did: Updated three pages, in both locales.
   
   Review point: The Blue/Green claim, the cut-off title, and the missing AI 
disclosure section
   What I did: The Blue/Green sentence is removed andck. The title you need to 
fix yourself (step 3).



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to