Skip to content

fix: collapse duplicate recurring routine chains on register - #583

Closed
chubes4 wants to merge 1 commit into
Automattic:mainfrom
chubes4:fix/582-collapse-duplicate-routine-chains
Closed

chubes4 wants to merge 1 commit into
Automattic:mainfrom
chubes4:fix/582-collapse-duplicate-routine-chains

Conversation

@chubes4

@chubes4 chubes4 commented Oct 14, 2026

Copy link
Copy Markdown
Contributor

Fixes #582.

Problem

A registration race (two concurrent requests both see no pending action and both schedule) leaves two matching recurring chains for a routine. register()'s idempotent path returns at the first match and never cancels the others, so every duplicate chain fires once per tick forever. In Extra Chill production this showed up as paired wp_agent_routine_run_scheduled jobs created in the same second (e.g. jobs 1031852/1031853) for system-* routines, with two separate 300s chains a day apart (first_timestamp 1789233749 / 1789365037).

The fix

In register(), when one or more pending chains already match the routine's trigger, all matching chains are collapsed before returning true:

  • Keeper = highest action id. AS ids are a global increasing sequence and a watermark is always some chain's own first id, so the newest chain is the only choice guaranteed to sit at or above the routine's current watermark — a keeper below the watermark would be cancelled by the generation fence at its first execution (and keeping the lowest id would live-lock in the common race outcome where the watermark points at the higher chain). Existing duplicates self-heal on the next registration, i.e. the next plugin boot.
  • A collapse is not a schedule change: no new action is scheduled, no generation/watermark is minted — the unchanged-register path stays read-only apart from the duplicate cancels, and a follow-up register is a pure no-op again.
  • One-shot run_now enqueues survive: they are non-recurring, never match, and are never collapsed away.

Design calls

  1. No add_option lock around check-then-schedule. The collapse bounds the race's damage to a single scheduling interval, because routines re-register on every boot. A short lock would reintroduce the stuck-lock / cache-drift failure mode that fix: retry transient InnoDB deadlocks during option-store lock acquisition #580 just fixed for the reconcile lock (and which forced that lock down to raw $wpdb). Not worth it for a self-correcting condition.
  2. Watermark is left untouched on the collapse path. Worst corner (watermark equal to the lower chain's id and that chain claimed in-flight exactly as the collapse lands) lets the claimed duplicate fire one last time and spawn a successor; the next register() collapses again and it converges. Advancing the watermark instead would write an option on the "read-only" path and could fence a still-pending one-shot run_now enqueue. Not worth it.
  3. Only matching (recurring) actions are cancelled, not "every other pending action for the routine" — cancelling a pending run_now would silently drop a manual wake.

Tests

tests/routines-durability-smoke.php (all against the fake in-memory AS store):

  • two matching chains with the stale-low watermark from the observed production race → register() cancels exactly the lowest-id chain, schedules nothing, mints no generation, leaves exactly one pending chain, which runs unfenced; a follow-up register cancels nothing;
  • an in-flight claimed duplicate whose claim won the race against the cancel → fenced at before_execute (id below watermark), does not fire, spawns no successor; the keeper fires normally;
  • a pending one-shot run_now enqueue survives the collapse; no generation option is minted when none existed (pre-fence legacy chains).

Verified the new assertions fail on main (6 failures) and pass with the fix; full composer smoke and composer phpstan are green.

Once two matching chains existed (e.g. a registration race where two
concurrent requests both saw no pending action and both scheduled),
register()'s idempotent path returned at the first match and never
cancelled the others: every duplicate chain kept firing once per tick,
forever (observed in production as paired jobs created in the same
second for system-* routines).

When several pending chains now match the routine's trigger, register()
collapses them: it keeps the highest-id chain and cancels the rest, then
returns true as before. The highest id is the only keeper guaranteed to
sit at or above the routine's current watermark (Action Scheduler ids
are a global increasing sequence and the watermark is always some
chain's own first id), so the kept chain can never be killed by the
generation fence. A collapse is not a schedule change: no new action is
scheduled and no generation/watermark is minted, keeping the unchanged
register() path read-only apart from the duplicate cancels. One-shot
run_now enqueues are non-recurring and are never collapsed away.

Existing duplicate chains self-heal on the next registration (every
plugin boot re-registers), so no concurrent double-schedule lock is
added around check-then-schedule - the collapse already bounds the
race's damage to a single interval, without reintroducing the stuck-lock
failure mode fixed for the reconcile lock in Automattic#580.

Fixes Automattic#582.
@chubes4

chubes4 commented Oct 21, 2026

Copy link
Copy Markdown
Contributor Author

Superseded by #588 (merged), which collapses duplicate matching chains and also makes the check-then-schedule step single-flight and skips Action Scheduler I/O for unchanged declarations.

AI disclosure: Claude (via the Pi coding agent in a Roadie session) closed this at Chris's direction during the #582 / Extra-Chill/data-machine#3495 work.

@chubes4 chubes4 closed this Oct 21, 2026
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.

fix: register() never collapses duplicate matching routine chains — routines run twice per tick

1 participant