salvalcantara commented on code in PR #987:
URL:
https://github.com/apache/flink-kubernetes-operator/pull/987#discussion_r2181918327
##########
flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/reconciler/deployment/AbstractJobReconciler.java:
##########
@@ -409,25 +409,30 @@ protected void setUpgradeSavepointPath(
conf.get(KubernetesOperatorConfigOptions.OPERATOR_SAVEPOINT_FORMAT_TYPE)
.name());
- FlinkStateSnapshotUtils.createUpgradeSnapshotResource(
- conf,
- ctx.getOperatorConfig(),
- ctx.getKubernetesClient(),
- ctx.getResource(),
- savepointFormatType,
- savepointLocation);
+ var snapshotCrOpt =
+ FlinkStateSnapshotUtils.createUpgradeSnapshotResource(
+ conf,
+ ctx.getOperatorConfig(),
+ ctx.getKubernetesClient(),
+ ctx.getResource(),
+ savepointFormatType,
+ savepointLocation);
var jobStatus = ctx.getResource().getStatus().getJobStatus();
jobStatus.setUpgradeSavepointPath(savepointLocation);
- // Register created savepoint in the now deprecated savepoint info and
history
- var savepoint =
- new Savepoint(
- cancelTs.toEpochMilli(),
- savepointLocation,
- SnapshotTriggerType.UPGRADE,
- savepointFormatType,
- null);
- jobStatus.getSavepointInfo().updateLastSavepoint(savepoint);
+ if (snapshotCrOpt.isEmpty()) {
+ // Register created savepoint in the now deprecated savepoint info
and history
+ // only if snapshot CR was not created, otherwise it would be
double recorded
+ // and disposed immediately
+ var savepoint =
+ new Savepoint(
+ cancelTs.toEpochMilli(),
+ savepointLocation,
+ SnapshotTriggerType.UPGRADE,
+ savepointFormatType,
+ null);
+ jobStatus.getSavepointInfo().updateLastSavepoint(savepoint);
+ }
Review Comment:
:+1: Maybe you could also use `isSnapshotResourceEnabled` instead? Looks
more explicit / less indirect to me.
--
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]
