AbhinavBattu opened a new pull request, #58168:
URL: https://github.com/apache/spark/pull/58168
### What changes were proposed in this pull request?
`KubernetesClusterSchedulerBackend.doKillExecutors` reports `ExecutorKilled`
for every
executor it kills. This PR skips that report for executors killed with
`countFailures = true`, whose loss reason is reported by the caller instead.
`CoarseGrainedSchedulerBackend.killExecutors` records
`executorsPendingToRemove(id) = !countFailures`, so such a kill stores
`false`.
### Why are the changes needed?
On a heartbeat timeout, `HeartbeatReceiver.expireDeadHosts` does two things
in order:
1. `sc.killAndReplaceExecutor(id)`, which calls
`killExecutors(..., countFailures = true, force = true)`
2. sends `RemoveExecutor(id, ExecutorProcessLost("Executor heartbeat timed
out after N ms"))`
`doKillExecutors` runs synchronously inside step 1
(`adjustTargetNumExecutors = false`, so
the future resolves on `ThreadUtils.sameThread`), so its `RemoveExecutor(id,
ExecutorKilled)`
reaches the driver endpoint first and removes the executor from
`executorDataMap`. Step 2's
message then falls into the `case None` branch and has no effect. This is
deterministic
rather than a race.
The recorded reason is therefore `ExecutorKilled`, which `TaskSetManager`
maps to
`exitCausedByApp = false`, so the task failures do not count towards
`spark.task.maxFailures` and the job does not fast-fail. Kubernetes is the
only backend
whose `doKillExecutors` reports a loss reason, which is why YARN is
unaffected.
`countFailures = true` has a single caller,
`SparkContext.killAndReplaceExecutor`, itself
called only from `HeartbeatReceiver.expireDeadHosts`, so only the heartbeat
timeout path
changes.
### Does this PR introduce _any_ user-facing change?
Yes. On Kubernetes, tasks lost to a heartbeat timeout now count towards
`spark.task.maxFailures`. A job that repeatedly loses executors this way now
fails after the
retry limit instead of retrying indefinitely, matching YARN.
### How was this patch tested?
Added a test to `KubernetesClusterSchedulerBackendSuite` covering both
branches: an executor
killed with `countFailures = true` is not reported as `ExecutorKilled`,
while an ordinary
kill still is. The existing "Kill executors" test, which calls
`doKillExecutors` with no
pending-removal entry, is unchanged.
```
build/sbt -Pkubernetes 'kubernetes/testOnly
*KubernetesClusterSchedulerBackendSuite'
```
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Claude Opus 5)
--
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]
