Stop airflow db clean deleting triggers that are still in use - #71339
Open
steveahnahn wants to merge 2 commits into
Open
Stop airflow db clean deleting triggers that are still in use#71339steveahnahn wants to merge 2 commits into
steveahnahn wants to merge 2 commits into
Conversation
The triggerer already removes every unreferenced trigger on each loop, so an old trigger row that survives is almost always still doing work. Cleaning that row by age took a running deferred task or an event-driven asset watcher with it through ON DELETE CASCADE, and neither reaches an archive table, so db export-archived cannot recover them. A trigger held by a callback instead failed the foreign key and stopped the whole command. trigger is also a reserved word on MySQL, and the archive DDL interpolated the table name unquoted, so the table could never be cleaned there at all.
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
1 participant
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.
Why
airflow db cleanselectstriggerrows by age alone, with no check for rows still in use.The triggerer already removes every unreferenced trigger on each loop, so an old trigger row that survives is almost always still doing work. Three foreign keys point at
trigger, and cleaning by age hits all three:task_instanceCASCADEasset_watcherCASCADEcallbackOnly the
triggerrows are copied to the archive table, so the cascade-deleted task instance reaches no archive anddb export-archivedcannot recover it. Asset watchers are created once and reused, so theircreated_dateages past any retention window and every clean removes them until the next Dag parse recreates them.Verified on Postgres. Four old triggers, three of them in use, and the cleanup reports
Found 4 rows meeting deletion criteria:task_instancerowsasset_watcherrowsdag_versionalready solves this withskip_if_referenced(#68339). This applies the same guard totrigger, so only genuinely orphaned rows are purged.MySQL
triggeris a reserved word on MySQL, and the archive step interpolated the table name intoCREATE TABLE ... LIKEunquoted. That is a syntax error, so thetriggertable could never be cleaned on MySQL at all.ProgrammingErroris suppressed per table, so it surfaced only as a warning.The two changes ship together because they share a code path: the archive table is always created, so the reference guard alone would still fail on MySQL.
Tests
Both new tests fail without the change and pass with it. Full file: 74 pass on Postgres, 73 on MySQL, 72 on SQLite.
related: #56192
Was generative AI tooling used to co-author this PR?
Generated-by: Claude Code (Opus 5) following the guidelines