hyeonkimmm commented on code in PR #1216:
URL:
https://github.com/apache/flink-kubernetes-operator/pull/1216#discussion_r4191668284
##########
flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/reconciler/deployment/AbstractJobReconciler.java:
##########
@@ -189,8 +189,11 @@ protected boolean reconcileSpecChange(
}
if (currentJobState == JobState.SUSPENDED && desiredJobState ==
JobState.RUNNING) {
- // We inherit the upgrade mode unless stateless upgrade requested
- if (currentDeploySpec.getJob().getUpgradeMode() !=
UpgradeMode.STATELESS) {
+ // We inherit the upgrade mode unless stateless upgrade requested.
A savepoint the user
+ // explicitly requested through initialSavepointPath is always
restored, regardless of
+ // the upgrade mode of the spec.
+ if (currentDeploySpec.getJob().getUpgradeMode() !=
UpgradeMode.STATELESS
+ || restoringFromInitialSavepoint(resource,
lastReconciledSpec)) {
Review Comment:
Fixed in 71803f2d. The resume path now marks the reconciled spec stable
after restoring from the explicitly requested savepoint, as
`redeployWithSavepoint` does. Added `testSavepointRedeployRetryIsNotRolledBack`
(rollback enabled, readiness timeout exceeded after the retry) and added an
`isLastReconciledSpecStable()` assertion to the existing retry tests.
##########
flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/reconciler/deployment/AbstractJobReconciler.java:
##########
@@ -211,6 +214,21 @@ protected boolean reconcileSpecChange(
return true;
}
+ /**
+ * Checks whether the suspended job is to be restored from the savepoint
the user explicitly
+ * requested through initialSavepointPath, recorded as the upgrade
savepoint by a savepoint
+ * redeploy or the first deployment. Such a savepoint is honoured even for
stateless specs,
+ * otherwise a savepoint redeploy requested while suspended, or one whose
deployment attempt
+ * failed, would be replaced by an empty state restore.
+ */
+ private boolean restoringFromInitialSavepoint(CR resource, SPEC
lastReconciledSpec) {
+ var initialSavepointPath =
resource.getSpec().getJob().getInitialSavepointPath();
Review Comment:
Fixed in 71803f2d. `restoringFromInitialSavepoint` now reads
`initialSavepointPath` from the last reconciled spec. Added
`testResumeIgnoresInitialSavepointPathEditAfterRedeployWhileSuspended`, which
clears the path before resuming a stateless job and asserts that the job still
restores from the recorded savepoint.
##########
docs/content/docs/managing/job-management.md:
##########
@@ -68,7 +68,7 @@ job:
state: suspended
```
-Setting the value to `suspended` stops the job while keeping its state
information, and setting it back to `running` resumes the job from where it
stopped. Any other spec change while the job is running triggers an
[upgrade](#upgrades), and changes made while suspended are recorded and take
effect once the job is resumed. In every case, how state survives the stop and
restore is decided by the upgrade mode.
+Setting the value to `suspended` stops the job while keeping its state
information, and setting it back to `running` resumes the job from where it
stopped. Any other spec change while the job is running triggers an
[upgrade](#upgrades), and changes made while suspended are recorded and take
effect once the job is resumed. In every case, how state survives the stop and
restore is decided by the upgrade mode. A [savepoint
redeploy](#redeploying-from-a-savepoint) requested while suspended is the
exception: resuming starts from its `initialSavepointPath` regardless of the
upgrade mode.
Review Comment:
Moved it to the "Redeploying from a Savepoint" section in 71803f2d and
restored the previous wording of the "Suspending and Resuming" paragraph.
##########
docs/content/docs/managing/job-management.md:
##########
@@ -68,7 +68,7 @@ job:
state: suspended
```
-Setting the value to `suspended` stops the job while keeping its state
information, and setting it back to `running` resumes the job from where it
stopped. Any other spec change while the job is running triggers an
[upgrade](#upgrades), and changes made while suspended are recorded and take
effect once the job is resumed. In every case, how state survives the stop and
restore is decided by the upgrade mode.
+Setting the value to `suspended` stops the job while keeping its state
information, and setting it back to `running` resumes the job from where it
stopped. Any other spec change while the job is running triggers an
[upgrade](#upgrades), and changes made while suspended are recorded and take
effect once the job is resumed. In every case, how state survives the stop and
restore is decided by the upgrade mode. A [savepoint
redeploy](#redeploying-from-a-savepoint) requested while suspended is the
exception: resuming starts from its `initialSavepointPath` regardless of the
upgrade mode.
Review Comment:
Reworded. It now reads: "A savepoint redeploy requested while the job is
suspended is applied when the job is resumed. The job then starts from
`initialSavepointPath` regardless of the upgrade mode."
--
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]
