[PR] [SPARK-58908][K8S] Do not report ExecutorKilled for heartbeat-timeout executor kills [spark]

["AbhinavBattu (via GitHub)" <gi...@apache.org>]

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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

Re: [PR] [SPARK-58908][K8S] Do not report ExecutorKilled for heartbeat-timeout executor kills [spark]

["uros-b (via GitHub)" <gi...@apache.org>]

uros-b commented on PR #58168:
URL: https://github.com/apache/spark/pull/58168#issuecomment-5363135249

   Thank you @AbhinavBattu!


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org