dongjoon-hyun commented on code in PR #895:
URL:
https://github.com/apache/spark-kubernetes-operator/pull/895#discussion_r4123541224
##########
spark-operator/src/main/java/org/apache/spark/k8s/operator/reconciler/reconcilesteps/AppInitStep.java:
##########
@@ -236,15 +250,28 @@ private boolean isDriverRequested(SparkAppContext
context) {
if (context.getCurrentAttemptDriverPod().isPresent()) {
return true;
}
+ SparkApplication app = context.getResource();
+ boolean suspended = app.getSpec().isSuspend();
Optional<Workload> workload = context.getCachedKueueWorkload();
if (workload.isEmpty()
|| !KueueWorkloadUtils.isAdmitted(workload.get())
- || (KueueWorkloadUtils.findEviction(workload.get()).isEmpty()
+ || (!suspended
+ && KueueWorkloadUtils.findEviction(workload.get()).isEmpty()
&& !KueueWorkloadUtils.isDeactivated(workload.get()))) {
return false;
}
- Pod driverPod =
context.getClient().resource(context.getDriverPodSpec()).get();
- return driverPod != null && driverPod.getMetadata().getDeletionTimestamp()
== null;
+ Pod driverPodSpec;
+ try {
+ driverPodSpec = context.getDriverPodSpec();
+ } catch (Exception e) {
Review Comment:
Thank you, @peter-toth. Addressed in 46ef4117 as you suggested. A spec-build
failure now propagates unless the API server lists no live driver pod of the
application, and the list runs only on that failure path. The new
`suspendedAppWithUncachedDriverAndUnbuildableSpecKeepsKueueWorkload` covers
your rows 2 and 3: the `SparkException` propagates and the
`KubernetesClientException` is retried, both keeping the `Workload`.
`suspendedAppWithUnbuildableDriverSpecReleasesKueueWorkload` now also covers a
driver pod which is being deleted, for which the `Workload` is still released.
The follow-up JIRA will remove the spec build from this lookup altogether.
##########
spark-operator/src/main/java/org/apache/spark/k8s/operator/reconciler/reconcilesteps/AppInitStep.java:
##########
@@ -223,9 +228,18 @@ private Optional<ReconcileProgress> holdForKueueAdmission(
* driver. See {@link SparkAppContext#getCurrentAttemptDriverPod()} for how
a pod left
* from a previous attempt is told apart. The informer cache may not have
seen a driver created
* in the previous reconcile yet, so a driver missing from it is looked up
on the API server as
- * well if the Kueue Workload is evicted or deactivated but still admitted,
since that Workload
- * would be released or held under the driver, which is requested only after
the admission.
- * Otherwise, the driver spec is not built for the lookup.
+ * well if the Kueue Workload is still admitted and either the application
is suspended or the
+ * Workload is evicted or deactivated, since that Workload would be released
or held under the
+ * driver, which is requested only after the admission. Otherwise, the
driver spec is not built
+ * for the lookup. Like a cached pod, the one found there counts only if it
is a live driver pod,
+ * see {@link SparkAppContext#isLiveDriverPod}, so that a pod of another
application with the
+ * same name is not taken for the driver.
+ *
+ * <p>A driver spec which cannot be built for that lookup fails the
reconcile, except for a
+ * suspended application, which is held as it was before the lookup,
releasing the Workload
+ * rather than failing every reconcile while it keeps the quota. The spec
built for a cached
+ * driver, see {@link SparkAppContext#getCurrentAttemptDriverPod()}, always
fails the reconcile,
Review Comment:
Addressed in 46ef4117. The Javadoc now names both outcomes: a suspended
application throws out of the reconcile, which JOSDK retries, while any other
application ends in `SchedulingFailure`, apart from a
`KubernetesClientException`, which the callers retry.
--
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]
