Fix remote log handlers crashing triggerer logging thread on concurrent close - #70479
Draft
ZhaoMJ wants to merge 2 commits into
Draft
Fix remote log handlers crashing triggerer logging thread on concurrent close#70479ZhaoMJ wants to merge 2 commits into
ZhaoMJ wants to merge 2 commits into
Conversation
…nt close Remote task log handlers (S3, GCS, WASB, OSS, Elasticsearch, HDFS) reclaimed disk on upload by calling ``shutil.rmtree(os.path.dirname(local_loc))`` when ``delete_local_copy`` is enabled -- removing the log file's parent directory rather than the file itself. The triggerer runs many trigger log handlers concurrently in a single process, and handler close is not atomic (the main thread's ``logging.shutdown()`` and the ``QueueListener`` monitor thread draining a ``trigger_end`` record can both call ``close()`` on the same handler). Two ``rmtree`` calls then run on the same directory: the first removes the tree, the second walks a path that just vanished and raises ``FileNotFoundError``. Raised on the logging monitor thread, this is unhandled and kills the listener, silently stopping all trigger log delivery for the rest of the process's life. Delete only the uploaded file (``unlink(missing_ok=True)``) and then prune now-empty parent directories, matching the pattern already used by the OpenSearch handler. This reclaims disk without deleting a directory a concurrent sibling still uses, and is idempotent under concurrent/double close.
potiuk
marked this pull request as draft
July 28, 2026 16:11
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Remote task log handlers (S3, GCS, WASB, OSS, Elasticsearch, HDFS) reclaim disk on upload by calling
shutil.rmtree(os.path.dirname(local_loc))whendelete_local_copyis enabled — removing the log file's parent directory rather than the file itself.The triggerer runs many trigger log handlers concurrently in a single process, and handler close is not atomic: the main thread's
logging.shutdown()and theQueueListenermonitor thread draining atrigger_endrecord can both callclose()→upload()on the same handler. Twormtreecalls then run against the same directory — the first removes the tree, the second walks a path that just vanished and raisesFileNotFoundError. Because it is raised on the logging monitor thread, it is unhandled and kills the listener thread, silently stopping all trigger log delivery for the remainder of the process's life.Observed traceback (provider
amazon9.2.0, Airflow 2.11.0):Fix
Delete only the uploaded file (
unlink(missing_ok=True)) and then prune now-empty parent directories, stopping at the first non-empty parent and atbase_log_folder. This is the same pattern the OpenSearch handler already uses (added in #64364). It:missing_ok=True+contextlib.suppress(OSError)),if any(parent.iterdir()): break),Applied to all six affected handlers (
opensearchalready had it).Tests
Added a regression test for the S3 handler asserting the delete is idempotent and that a shared parent directory with a sibling log is preserved.
Was generative AI tooling used to co-author this PR?
Generated-by: Claude Code
Important
🛠️ Maintainer triage note for @ZhaoMJ · by
@potiuk· 2026-07-28 16:11 UTCHelpful heads-up from the maintainers — please address before this PR can be reviewed:
The ball is in your court — you've been assigned to this PR. Fix the above, then mark it Ready for review.
See the Pull Request quality criteria for how to fix each item. There is no rush.
Automated triage — may be imperfect; a maintainer takes the next look. We use this two-stage triage process so maintainers' limited time goes to the conversation with you.