fat-catTW commented on code in PR #74497:
URL: https://github.com/apache/airflow/pull/74497#discussion_r4244584819
##########
providers/cncf/kubernetes/src/airflow/providers/cncf/kubernetes/operators/job.py:
##########
@@ -287,6 +314,15 @@ def execute_deferrable(self):
)
def execute_complete(self, context: Context, event: dict, **kwargs):
+ if event.get("status") == "readmitted":
+ self.log.info("Job preempted but now readmitted. Re-deferring to
restart pod discovery.")
+ self.job = self.hook.get_job_status(job_name=self.name,
namespace=self.namespace)
Review Comment:
Could we use the Job identity from the trigger event here?
In deferrable mode, the trigger event already contains the concrete
Kubernetes Job `name` / `namespace`. Using `self.name` / `self.namespace` on
resume can be wrong when the Job name was generated with the `job-`
prefix/random suffix, or when the namespace was resolved during job creation.
```python
self.job = self.hook.get_job_status(
job_name=event["name"],
namespace=event["namespace"],
)
##########
providers/cncf/kubernetes/src/airflow/providers/cncf/kubernetes/triggers/job.py:
##########
Review Comment:
I think this XCom path also needs to participate in the new readmission flow.
Right now the trigger reads XCom from `self.pod_names` before it waits for
the Job with `return_on_suspension=True`. If a Kueue-managed Job is preempted
and those initially discovered pods are deleted, this can fail or hang on the
old pod before the trigger gets a chance to detect suspension, wait for
readmission, and rediscover the replacement pod.
Could we move XCom extraction after the readmission/job-completion loop,
using the latest pod names instead? It would also be good to cover this with a
`do_xcom_push=True` preemption test.
--
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]
