hkc-8010 opened a new pull request, #76514: URL: https://github.com/apache/airflow/pull/76514
When the triggerer stops (a deploy, a pod replacement, any SIGTERM), every trigger that is still running loses what it logged on that triggerer. A trigger's log is uploaded to remote storage only when the runner reports the trigger finished, and a trigger that is running at shutdown never reports that. Its file stays on the local disk of a pod that is going away. The trigger then resumes on another triggerer, which writes its own `.trigger.<job_id>.log`, so the task log in the UI jumps from "Pausing task as DEFERRED" straight to the new triggerer's "trigger ... starting", with nothing in between. We hit this on a production deployment where an image deploy restarted the triggerer while 32 triggers were running, and a data-wait trigger lost three hours of log. Airflow 2 did not have this gap. `TriggererHandlerWrapper.close()` closed every open per-trigger handler at shutdown, and the object-store task handlers upload on close (see #37206). That path went away when the triggerer moved to `TriggerRunnerSupervisor`. ### Change After `kill()` in `TriggererJobRunner._execute`, the supervisor now: 1. drains the runner's sockets with the existing `cleanup_sockets_after_kill()`. `kill()` can return with log lines still unread, for example the "trigger completed" lines the runner writes as it cancels its triggers. 2. uploads and closes every entry still in `logger_cache`, through the same upload/close helper the finished-trigger path uses, so one failed upload does not stop the rest and the file handle is always closed (as in #66675). ### Testing - `test_execute_uploads_remaining_trigger_logs_after_runner_exits`: `_execute` calls `kill`, then `cleanup_sockets_after_kill`, then `upload_remaining_trigger_logs`. - `test_upload_remaining_trigger_logs_uploads_and_closes_every_trigger`: a failing upload in the middle does not stop the triggers before or after it, and every factory is closed. - `test_trigger_log_uploaded_when_triggerer_stops_before_trigger_finishes`: starts the real runner subprocess with a 1 hour `TimeDeltaTrigger`, shuts it down, and checks the uploaded file has both the "starting" line and the "trigger completed" line written during shutdown. It patches `load_remote_log_handler`, so the real path resolution runs. - I reverted each part of the change in turn (no upload call, upload before `kill()`, no drain, no-op flush, no per-trigger isolation, no close) and each one fails at least one of these tests. - End to end: a real `airflow triggerer` process with a deferred task on a 1 hour trigger, a remote log handler that copies to a local directory, then SIGTERM. On `main` nothing reaches remote storage. On this branch the trigger's `attempt=0.log.trigger.1.log` is uploaded. - `test_triggerer_job.py` passes in full on Python 3.11. ### Trade-off The uploads run one after another and before the job is marked ended. Other triggerers treat this job as alive while its heartbeat is fresh, so handing its triggers over can be delayed by up to `[triggerer] triggerer_health_check_threshold` (30s by default) when there are many triggers to upload. If the pod is killed partway through the loop, the remaining logs are not uploaded, which is the same as today. I kept it simple rather than adding a deadline or a thread pool. Happy to add either if you think it is worth it. No newsfragment, since this is a bugfix. --- ##### Was generative AI tooling used to co-author this PR? - [X] Yes (please specify the tool below) Generated-by: Claude Code following [the guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions) -- 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]
