Skip to content

Fix N+1 query in bulk task instance delete endpoint#67304

Open
ColtenOuO wants to merge 2 commits into
apache:mainfrom
ColtenOuO:fix-bulk-ti-delete-n-plus-one
Open

Fix N+1 query in bulk task instance delete endpoint#67304
ColtenOuO wants to merge 2 commits into
apache:mainfrom
ColtenOuO:fix-bulk-ti-delete-n-plus-one

Conversation

@ColtenOuO
Copy link
Copy Markdown
Contributor

BulkTaskInstanceService.handle_bulk_delete re-queried the database once per task instance inside the delete loop, even though _categorize_task_instances had already loaded every matched task instance into task_instances_map.

The "all map index" branch of the same method already batches its lookup correctly — only the "specific (task_id, map_index)" branch had the N+1.

Database round trips (N = task instances deleted)

Each redundant SELECT is one extra DB round trip. The fix reuses the in-memory task_instances_map, so those N re-fetch round trips are removed:

N (task instances) Round trips before Round trips after
5 15 10
10 25 15
20 45 25

Before: 5 + 2N queries — N of them redundant re-fetches.
After: 5 + N queries — exactly N round trips eliminated per bulk delete.

Fix

Capture the task_instances_map returned by _categorize_task_instances (previously discarded with _) and look each task instance up in it instead of issuing a fresh SELECT in the loop. Every key in matched_task_keys is a key of that map, so the lookup is guaranteed to hit.

Validation plan

  1. Added test_bulk_delete_does_not_requery_each_task_instance: deletes two
    differently-sized batches (5 and 15) and asserts each extra task instance
    adds exactly one query — the removed re-query would roughly double the delta.
  2. Confirmed the test fails on the pre-fix code (query delta 20 instead of 10, i.e. doubled) and passes with the fix, so it genuinely guards the regression.
  3. Ran the full TestBulkTaskInstances suite (27 tests) — all pass; bulk delete / update / wildcard / authorization behaviour is unchanged.

Was generative AI tooling used to co-author this PR?
  • Yes — Claude Code (Opus 4.7)

@boring-cyborg boring-cyborg Bot added the area:API Airflow's REST/HTTP API label May 21, 2026
@ColtenOuO ColtenOuO changed the title Fix bulk ti delete n plus one Fix N+1 query in bulk task instance delete endpoint May 21, 2026
Copy link
Copy Markdown
Member

@jason810496 jason810496 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM overall, thanks.

}
]

def test_bulk_delete_does_not_requery_each_task_instance(self, test_client, session):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to follow the same test pattern in #66222 .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:API Airflow's REST/HTTP API

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants