seanmuth commented on code in PR #69058:
URL: https://github.com/apache/airflow/pull/69058#discussion_r3500325596


##########
providers/cncf/kubernetes/src/airflow/providers/cncf/kubernetes/executors/kubernetes_executor.py:
##########
@@ -478,6 +498,37 @@ def _change_state(
             self.kube_scheduler.patch_pod_executor_done(pod_name=pod_name, 
namespace=namespace)
             self.log.info("Patched pod %s in namespace %s to mark it as done", 
key, namespace)
 
+        if state == TaskInstanceState.FAILED and 
self._is_pre_execution_failure(
+            state,
+            self._get_task_instance_state(key, session=session),
+            failure_details,
+            self.pod_launch_failure_excluded_container_reasons,
+        ):

Review Comment:
   Applied in `2352e1f453` — the `pod_launch_failure_max_retries != 0` guard 
now short-circuits before the task-instance state lookup, so the DB query is 
skipped entirely when requeues are disabled.
   
   ---
   Drafted-by: Claude Code (Opus 4.8); reviewed by @seanmuth before posting



##########
providers/cncf/kubernetes/src/airflow/providers/cncf/kubernetes/executors/kubernetes_executor.py:
##########
@@ -455,13 +471,17 @@ def _change_state(
         if state == ADOPTED:
             # When the task pod is adopted by another executor,
             # then remove the task from the current executor running queue.
+            self.last_known_jobs.pop(key, None)
             try:
                 self.running.remove(key)
             except KeyError:
                 self.log.debug("TI key not in running: %s", key)
             return
 
         if state == TaskInstanceState.RUNNING:
+            # The task process started, so any later failure is an execution 
failure that should
+            # not be requeued by the pre-execution path below.
+            self.last_known_jobs.pop(key, None)

Review Comment:
   Done in `2352e1f453` — the job spec, requeue count, and last-requeued pod 
are now wrapped in a single `_PodLaunchAttempt` per key 
(`pod_launch_attempts`), popped in one place across all exit paths. That also 
fixes the count leaking: the previous separate `pod_launch_failure_attempts` 
counter was never cleared on the ADOPTED path.
   
   ---
   Drafted-by: Claude Code (Opus 4.8); reviewed by @seanmuth before posting



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

Reply via email to