Queue user migrations in batches to avoid timeouts on large sites#9
Queue user migrations in batches to avoid timeouts on large sites#9dparker1005 wants to merge 1 commit into
Conversation
pmprompmt_queue_user_migrations ran as a single Action Scheduler task that looped over every user on the site, with a duplicate-check query per maybe_add_task() call. On large sites this one task could easily exceed PHP or queue-runner time limits, leaving the migration queue partially populated. Queue 250 users at a time and have the task re-queue itself with an updated offset until all users are queued. The re-queued task has distinct args, so PMPro_Action_Scheduler's duplicate detection does not block it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
flintfromthebasement
left a comment
There was a problem hiding this comment.
PR: #9 — Queue user migrations in batches to avoid timeouts on large sites
dparker1005 → dev | 1 file, +28 -8
#9
Summary
Clean fix. The batch-and-self-requeue pattern is correct: each run stays under the time limit, maybe_add_task() deduplicates per-user tasks on retry, and the self-re-queue args preserve correct positional mapping to the updated function signature. One minor risk introduced by making halt() unconditional.
Issues
-
Minor
pmpro-memberpress-migration-toolkit.php:109–134—halt()is now called unconditionally on every batch run, butresume()has no guarantee of executing ifmaybe_add_task()fatals mid-loop. The original code calledhalt()conditionally, so the exposure was limited; now every batch run holds a live halt. A fatal in theforeachor the self-re-queue block leavespmpro_as_halted = truein the DB until someone manually clears it — which stops all PMPro AS tasks site-wide, not just migration tasks.Wrap the work in
try/finally:PMPro_Action_Scheduler::instance()->halt(); try { foreach ( $user_ids as $user_id ) { ... } if ( count( $user_ids ) === $batch_size ) { ... } } finally { PMPro_Action_Scheduler::instance()->resume(); }
Looks Good
- The empty-batch early return (line 105) bails out before
halt()is called — no unnecessary halt/resume churn on the final iteration. That ordering is correct. intval( $offset )on line 99 correctly sanitizes the AS-deserialized arg before it hits the SQLLIMIT/OFFSET.- Removing the old
count > 250 → halt()heuristic was the right call — it was inconsistently placed and leftresume()always running regardless. add_actionarity bump from10, 1to10, 2is correctly matched to the new signature.
Questions
- Is there a cap on how many batches this chain can run? On a site with 500k users, that's 2,000 sequential AS tasks just to queue the actual migrations. Intentional, or worth adding a sanity-limit log at, say, every 10th batch?
Problem
pmprompmt_queue_user_migrations()runs as a single Action Scheduler task that loops over every user on the site, andPMPro_Action_Scheduler::maybe_add_task()performs a duplicate-check query for each call. On a site with tens of thousands of users, that single action can exceed PHP / queue-runner time limits and die partway, leaving the migration queue partially populated with no indication anything went wrong.Fix
Queue users in batches of 250 (ordered by ID with an offset) and have the task re-queue itself with
offset + 250whenever the batch came back full. Each run stays short, and a killed run can be safely retried sincemaybe_add_task()deduplicates per-user tasks.This also replaces the previous
count > 250 → halt()heuristic: the Action Scheduler is now paused for the duration of each (short) batch and resumed afterward.cc @flintfromthebasement
🤖 Generated with Claude Code