dongjoon-hyun commented on code in PR #875:
URL:
https://github.com/apache/spark-kubernetes-operator/pull/875#discussion_r4085106347
##########
spark-operator/src/main/java/org/apache/spark/k8s/operator/reconciler/reconcilesteps/ClusterReconcileStep.java:
##########
@@ -48,15 +48,19 @@ public abstract ReconcileProgress reconcile(
* @param statusRecorder The SparkClusterStatusRecorder for recording status
updates.
* @param updatedStatus The updated ClusterStatus.
* @param requeueAfter The duration after which to re-queue.
- * @return The ReconcileProgress indicating the re-queue.
+ * @return The ReconcileProgress indicating the re-queue, or the default
re-queue if the status
+ * is not persisted.
*/
protected ReconcileProgress updateStatusAndRequeueAfter(
SparkClusterContext context,
SparkClusterStatusRecorder statusRecorder,
ClusterStatus updatedStatus,
Duration requeueAfter) {
- statusRecorder.persistStatus(context, updatedStatus);
- return ReconcileProgress.completeAndRequeueAfter(requeueAfter);
+ // A rejected update, e.g. of a state unknown to an older CRD, would be
rejected again right
+ // away, so it is retried with the default interval, not immediately.
+ return statusRecorder.persistStatus(context, updatedStatus)
Review Comment:
Correction: this was already addressed by SPARK-53706 (e80dead). On `main`,
`AppReconcileStep.updateStatusAndRequeueAfter` goes through
`attemptStatusUpdate`, and `appendStateAndRequeueAfter` checks the result of
`appendNewStateAndPersist`, both returning `completeAndImmediateRequeue()` when
the status is not persisted. So `AppCleanUpStep` already does not report
progress on a rejected patch, and no new JIRA issue is needed. The only
difference is that the cluster side here retries with the default interval,
since a status rejected by an older CRD, e.g. for `Suspended`, would be
rejected again right away.
##########
spark-operator/src/main/java/org/apache/spark/k8s/operator/reconciler/reconcilesteps/ClusterReconcileStep.java:
##########
@@ -48,15 +48,19 @@ public abstract ReconcileProgress reconcile(
* @param statusRecorder The SparkClusterStatusRecorder for recording status
updates.
* @param updatedStatus The updated ClusterStatus.
* @param requeueAfter The duration after which to re-queue.
- * @return The ReconcileProgress indicating the re-queue.
+ * @return The ReconcileProgress indicating the re-queue, or the default
re-queue if the status
+ * is not persisted.
*/
protected ReconcileProgress updateStatusAndRequeueAfter(
SparkClusterContext context,
SparkClusterStatusRecorder statusRecorder,
ClusterStatus updatedStatus,
Duration requeueAfter) {
- statusRecorder.persistStatus(context, updatedStatus);
- return ReconcileProgress.completeAndRequeueAfter(requeueAfter);
+ // A rejected update, e.g. of a state unknown to an older CRD, would be
rejected again right
+ // away, so it is retried with the default interval, not immediately.
+ return statusRecorder.persistStatus(context, updatedStatus)
Review Comment:
I re-checked the code and realized that SPARK-53706 resolved it already.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
