sunchao commented on code in PR #57332:
URL: https://github.com/apache/spark/pull/57332#discussion_r3627306440
##########
resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/BasicExecutorFeatureStep.scala:
##########
@@ -168,7 +168,11 @@ private[spark] class BasicExecutorFeatureStep(
ENV_DRIVER_URL -> driverUrl,
ENV_EXECUTOR_CORES -> {
if
(kubernetesConf.get(KUBERNETES_ALLOCATION_RECOVERY_MODE_ENABLED).getOrElse(false))
{
- kubernetesConf.get("spark.task.cpus", "1")
+ // `spark.task.cpus` may be fractional, so round up to an
integer (at least 1).
+ // When it is 0.5 or less, the single announced core fits more
than one task and
+ // the single-task-per-recovery-executor guarantee no longer
holds;
+ // ExecutorPodsAllocator logs a warning for that case.
+ math.max(1, kubernetesConf.get("spark.task.cpus",
"1.0").toDouble.ceil.toInt).toString
Review Comment:
[P2] I agree that the general recovery/profile mismatch is pre-existing and
that the full cross-profile fix can be handled separately. However, this PR
newly supports fractional stage-level CPU requests, so there is still a new
failure in the simpler dynamic-allocation case.
For example:
- Global `spark.task.cpus = 1`.
- A stage resource profile requests `1.5` CPUs per task.
- Dynamic allocation is enabled, so tasks can run only on executors with the
same resource-profile ID.
After an OOM, the recovery executor for that profile currently advertises
only one core because it uses the global setting. The scheduler therefore
cannot place any task requiring `1.5` CPUs on that executor, and the stage
stalls. Conversely, a `0.5`-CPU profile can admit two concurrent tasks without
the profile-specific recovery warning.
The case with dynamic allocation disabled is different: task-only profiles
may share default-profile executors, so the launched executor profile does not
identify every task it can run. I agree that broader, pre-existing problem can
be a follow-up.
Could we retain profile-derived recovery cores and warnings when dynamic
allocation is enabled, where profile IDs must match? If that path is
intentionally deferred as well, the PR should explicitly document that
fractional stage-level profiles are unsupported in recovery mode.
--
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]
