pchoudhury22 commented on code in PR #968:
URL:
https://github.com/apache/flink-kubernetes-operator/pull/968#discussion_r2064042112
##########
flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/reconciler/deployment/AbstractJobReconciler.java:
##########
@@ -202,13 +203,38 @@ protected boolean reconcileSpecChange(
currentDeploySpec,
deployConfig,
// We decide to enforce HA based on how job was previously
suspended
- lastReconciledSpec.getJob().getUpgradeMode() ==
UpgradeMode.LAST_STATE);
+ lastReconciledSpec.getJob().getUpgradeMode() ==
UpgradeMode.LAST_STATE,
+ lastReconciledSpec.getJob().getUpgradeMode() ==
UpgradeMode.LAST_STATE
+ && wasUpgradeForScalingOnly(ctx));
ReconciliationUtils.updateStatusForDeployedSpec(resource,
deployConfig, clock);
}
return true;
}
+ /**
+ * Determines whether the last upgrade was performed for scaling.
+ *
+ * @param ctx Reconciliation context.
+ * @return {@code true} if the upgrade was for scaling, {@code false}
otherwise.
+ */
+ protected boolean wasUpgradeForScalingOnly(FlinkResourceContext<CR> ctx) {
+ var resource = ctx.getResource();
+
+ STATUS status = resource.getStatus();
+ SPEC lastReconciledSpec =
status.getReconciliationStatus().deserializeLastReconciledSpec();
+ SPEC lastStableSpec =
status.getReconciliationStatus().deserializeLastStableSpec();
+ if (lastStableSpec == null) {
+ return false;
+ }
+
lastReconciledSpec.getJob().setState(lastStableSpec.getJob().getState());
+ var specDiff =
+ new ReflectiveDiffBuilder<>(
+ ctx.getDeploymentMode(), lastReconciledSpec,
lastStableSpec)
+ .build();
+ return specDiff.getType() == DiffType.SCALE;
+ }
Review Comment:
I absolutely agree! honestly while working on this, I was able to come up
with two options,
1. use lastStableSpec to compare with lastReconciledSpec. which definitely
goes agains the current design. But due the 2step upgrade, the other option
was, as you mentioned we have to record some extra metadata somewhere. I was
considering ReconciliationState for the same. Maybe to have like SCALING state.
But again that does not sound very intuitive aswell. as Upgrading already does
encompass Scaling. so definitely some guidance would be very helpful! Thanks!
--
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]
