Reduce memory used when deleting a Dag with a large history - #71185
Open
ColtenOuO wants to merge 1 commit into
Open
Reduce memory used when deleting a Dag with a large history#71185ColtenOuO wants to merge 1 commit into
ColtenOuO wants to merge 1 commit into
Conversation
delete_dag forced SQLAlchemy's "fetch" synchronization strategy on every bulk delete it issues. That strategy reads the primary key of every deleted row back from the database so it can mark matching in-memory objects as deleted, but the session holds nothing beyond the Dag's own DagModel row — the keys were matched against an effectively empty identity map and discarded, once per table with a dag_id column. The cost scaled with the Dag's history rather than with the number of objects actually needing synchronization: roughly 211 bytes of transient Python heap per deleted row on PostgreSQL, or about 1 GiB in the API server for a Dag with five million task instances. The default strategy evaluates the criteria in Python against the objects already loaded, so synchronization still happens without a round-trip sized by the row count.
ColtenOuO
requested review from
ephraimbuddy,
jason810496,
pierrejeambrun and
rawwar
as code owners
August 5, 2026 14:46
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.
Summary
delete_dagforces SQLAlchemy's"fetch"synchronization strategy on every bulkdelete it issues. That strategy reads the primary key of every deleted row back from
the database so it can mark matching in-memory objects as deleted — via
RETURNINGon backends that support it, or a second full
SELECTbeforehand on those that don't.The session holds nothing but the Dag's own
DagModelrow, so there is no identitymap to synchronize. The keys are read back, matched against an effectively empty map,
and discarded — at a cost proportional to the Dag's history, once per table with a
dag_idcolumn.Removing the override restores SQLAlchemy's default
"auto"strategy, which evaluatesthe criteria in Python against the objects already loaded. Synchronization still
happens; it just no longer needs a round-trip whose size is set by the row count
instead of the identity map.
Before / after
200,000 rows in a table shaped like
task_instance(its eight declared indexes plusthe
dag_runFK), with one ORM object in the session:synchronize_session"fetch"(before)DELETE … RETURNING id)DELETE)"fetch"(before)SELECT id …+DELETE)DELETE)The memory saving is the point, and it scales with the Dag: ~211 bytes per deleted
row, so ~1 GiB of transient heap in the API server for a Dag with five million task
instances. The time saving is real on PostgreSQL only — InnoDB's own delete cost
dwarfs the extra
SELECT, so the MySQL times are noise in both directions.Benchmark script
The regression test asserts on the emitted SQL rather than the query count, because on
backends with
RETURNINGthe count is identical either way. It also pins a propertyworth keeping: the delete loop runs over every mapped model with a
dag_id, and"auto"silently falls back to"fetch"for criteria it cannot evaluate in Python, sothe assertions catch a future model that would quietly reintroduce the round-trip.
Was generative AI tooling used to co-author this PR?