peter-toth commented on code in PR #974:
URL: 
https://github.com/apache/spark-kubernetes-operator/pull/974#discussion_r4334687300


##########
spark-operator/src/main/java/org/apache/spark/k8s/operator/reconciler/reconcilesteps/AppValidateStep.java:
##########
@@ -59,10 +59,12 @@ public ReconcileProgress reconcile(
       log.warn("Spark application found with empty status. Resetting to 
initial state.");
       return attemptStatusUpdate(context, statusRecorder, new 
ApplicationStatus(), proceed());
     }
-    if (isClientMode(app)) {
-      return failWith(context, statusRecorder, "Client mode is not supported 
yet.");
-    }
+    // Only before the driver of an attempt is requested. Otherwise, an 
application which failed the
+    // validation would be failed again on every reconciliation, and never 
cleaned up or deleted.

Review Comment:
   **Finding 1.** This stops new loops. An app which an operator from 0.7.0 to 
1.0.x already left in the loop may not recover on the upgrade, though.
   
   - Each loop write appends a `Failed` entry of about 137 bytes of status 
JSON. I measured it by serializing 10,000 of them.
   - At about 20 writes per minute, the object reaches the etcd size limit (1.5 
MiB by default) in under 10 hours.
   - From then on, a write which appends another entry of that size fails. 
`persistStatus` returns `false`, which requeues with no state change.
   - The visible symptom of the bug is a delete that hangs, so such an app most 
likely already has a `deletionTimestamp`. Then only `cleanup()` runs after the 
upgrade.
   - `cleanup()` has to append `appCancelled()`. Its message is longer than the 
`Failed` one, so that write fails too. The finalizer is then never removed.
   
   This is derived from reading, apart from the entry size. To settle it, give 
a `ClientMode` app a status of about 11k `Failed` entries on a kind cluster, 
delete it, and run this head.
   
   A line under `SparkApplication` in `docs/migration_guide.md` could be:
   
   ```markdown
   - Since 1.1.0, a `ClientMode` application fails once and then terminates. 
1.0 failed it again on every reconciliation, so its status may have grown too 
large to update. If such an application cannot be deleted, remove its finalizer 
by hand, e.g. `kubectl patch sparkapp <name> --type=merge -p 
'{"metadata":{"finalizers":null}}'`.
   ```
   



-- 
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]

Reply via email to