Skip to content

Check vanished jobs in refresh() without nesting key_source in NOT IN - #1674

Open
dimitri-yatsenko wants to merge 3 commits into
masterfrom
fix/refresh-vanished-keys
Open

dimitri-yatsenko wants to merge 3 commits into
masterfrom
fix/refresh-vanished-keys

Conversation

@dimitri-yatsenko

@dimitri-yatsenko dimitri-yatsenko commented Nov 10, 2026 •

Copy link
Copy Markdown
Member

Fixes #1673

Why

Job.refresh() removed jobs whose keys had left key_source with DELETE ... WHERE (pk) NOT IN (key_source). When key_source is restricted by part tables (for example A * B & A.Item & B.Item), MySQL flattens the part restrictions inside that subquery into joins and deduplicates only at the end. It then builds the full join of the part tables before collapsing it to the few keys in key_source. In the case described in the issue, 2 keys in key_source cost about 270 million intermediate rows and 140 s per refresh(), against about 1 ms for key_source on its own. Users with a key_source restricted by part tables see refresh() take minutes, and a loop that refreshes before each populate() pays that on every pass.

What

  • Steps 3a and 3b of refresh() now go through Job._delete_vanished(candidates), which never puts key_source inside NOT IN:
    1. Count the candidate jobs (3a: success/error jobs not in the target; 3b: non-ignore jobs older than stale_timeout). If there are none, stop.
    2. When the jobs table and key_source share the same primary key, count the key_source keys restricted to the candidates, with key_source at the top level of the query. If the counts are equal, nothing has vanished. This is the common case and costs two COUNT queries.
    3. Otherwise, read the candidates in pages of VANISHED_DELETE_BATCH (1000) keys ordered by key. For each page, fetch the key_source keys restricted to that page and delete the page's other jobs in one DELETE. Memory is bounded by the page size, even with keep_completed=True, where 3b's candidates are all old success jobs.
  • The set of jobs removed and kept is unchanged. Existing tests in test_jobs_refresh.py pass unchanged, except that test_refresh_cleanup_is_set_based now uses 50 vanished keys instead of 20, because the cleanup issues a few more (still a fixed number of) statements.

Semantics to review:

  • The cleanup is no longer a single DELETE per step: it checks first, then deletes. Each page's DELETE is restricted again by the candidate condition, so a job that has left the candidates in between is kept. For 3a that means a job whose status is no longer success or error; for 3b the condition is only status != 'ignore' and the age, so a job that moved from pending to reserved is still removed, as the old single DELETE also did. A key that re-enters key_source between the check and the DELETE can still lose its job record. The next refresh() then adds it back as pending. Inside an enclosing transaction, all of these statements run in that transaction.
  • Keys are compared through their fetched values. A key that does not match its own value as a restriction, such as an inexact float32 key, is neither found present nor deleted: the cursor moves past it, so refresh() still finishes, but such a job is not removed when its key vanishes (the old NOT IN compared columns and did remove it).
  • When the jobs table and key_source have different primary keys (possible with a custom key_source), the count shortcut does not apply: any candidates are compared page by page, on the attributes the two keys share.

How tested

New tests in tests/integration/test_jobs_refresh_part_restricted.py, run against both MySQL and PostgreSQL through testcontainers (pixi run -e test pytest tests/integration/test_jobs_refresh_part_restricted.py):

  • test_refresh_master_with_two_parts: a master with two part tables and key_source = Master & Master.PartA & Master.PartB, with four success and two error jobs. After a master is deleted and a part of each of two others, refresh() removes the vanished success and error records at once. The vanished key whose target row remains is removed after stale_timeout. All other jobs are kept.
  • test_refresh_never_negates_key_source: records every statement refresh() issues and asserts that no NOT IN (...) subquery mentions the part tables. It fails on master on both backends and passes with this change.
  • test_refresh_moves_past_keys_that_do_not_match_themselves: float32 keys with a page size of 3, where the first page cannot be matched by value; refresh() moves on and removes the vanished key on the next page. It fails (reads the same page forever) without the cursor fix.
  • test_refresh_pages_through_candidates: with a page size of 2, vanished keys at page boundaries and inside pages are all removed and the rest kept.
  • test_refresh_removes_jobs_of_vanished_keys: the same checks for key_source = A * B & A.Item & B.Item.
  • test_refresh_does_not_join_the_parts (MySQL only): two masters with 3,000 parts each and one master with 2,000 parts. MySQL's materialization strategy is forced through optimizer_switch, since at this size it may otherwise choose FirstMatch. On master refresh() takes about 10 s and the test fails (limit 2 s). With this change it passes.

The full job suites (test_jobs.py, test_jobs_refresh.py, test_jobs_refresh_part_restricted.py, test_hidden_job_metadata.py, test_autopopulate.py) pass on both backends: 67 passed, 3 skipped (MySQL-only tests on PostgreSQL). On the dataset from the issue (2 keys, part tables of about 42,000 and 6,400 rows), refresh() went from 140 s to 0.01 s with the same result.

refresh() removed jobs whose keys had left key_source with
DELETE ... WHERE key NOT IN (key_source). MySQL materializes that subquery,
and when key_source is restricted by two part tables (A * B & A.Part & B.Part)
it builds the materialized key_source as the join of both parts: the product
of their rows. With 21,087 and 6,407 parts per master and two masters, each
refresh() took about 110 s on MySQL 8.0, so a loop that refreshes before
every populate() spent minutes per cycle on a key_source of two keys.

The cleanup now counts the candidate jobs, then counts the key_source keys
restricted to them, with key_source at the top level of the query. Equal
counts mean nothing vanished. Otherwise the keys are compared and the
vanished jobs deleted by key, in batches. Same reproduction: 0.02 s.

test_refresh_cleanup_is_set_based now uses 50 vanished keys, since the
cleanup issues a few more statements (still a fixed number).
When the counts show that some candidate jobs have left key_source,
_delete_vanished() fetched every candidate key and every present
key_source key at once. With keep_completed=True the stale-job step's
candidates are all old success jobs, so a single vanished key loaded the
whole jobs table into memory. The comparison now reads the candidates
one page of VANISHED_DELETE_BATCH keys at a time, ordered by key, and
deletes each page's vanished jobs in one statement. Memory is bounded by
the page size; the common case (nothing vanished) is still two counts.

New tests use a master with two part tables and a key_source that
requires rows in both (Master & Master.PartA & Master.PartB):
- vanished success and error records are removed at once, a vanished
  key that still has a target row goes after stale_timeout, and the
  other jobs are kept;
- no statement issued by refresh() nests the part tables inside a
  NOT IN subquery (fails before the fix on both backends);
- with a page size of 2, vanished keys on and inside page boundaries
  are all removed.
@dimitri-yatsenko dimitri-yatsenko added this to the v2.3.7 milestone Nov 10, 2026
A page of keys that do not match their own values as a restriction (an
inexact float32 key, for example) was neither found present nor deleted,
and the cursor did not move, so refresh() read the same page forever. The
cursor now advances by the rows each page leaves in place, and a test
covers a float32 key on both backends.

Also drop the test schemas with the adapter's DROP statement, which is
DROP SCHEMA ... CASCADE on PostgreSQL, and state the re-restriction
guarantee in the docstring precisely.
@dimitri-yatsenko
dimitri-yatsenko marked this pull request as ready for review November 10, 2026 17:38

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: Job.refresh() takes minutes when key_source is restricted by part tables (MySQL)

1 participant