fix: delete expired presence rows by key in bounded passes - #258
fix: delete expired presence rows by key in bounded passes#258aldorizona10-glitch wants to merge 2 commits into
Conversation
wp_delete_expired_presence_data() looped over a MySQL-only `DELETE ... LIMIT` with no iteration ceiling. Two problems (WordPress#206): 1. `DELETE ... LIMIT` is MySQL-specific and appears nowhere in wp-includes, so it is a portability hazard for non-MySQL backends, including the SQLite integration Playground uses for the demo blueprints. 2. The loop had no upper bound. A site returning from a cron outage with a large backlog would keep looping until max_execution_time, having committed an unpredictable fraction of the work. Follow core's batching convention: select a bounded page of primary keys older than the cutoff and delete them by key (portable `IN (...)`), for a fixed number of passes per invocation. The remainder is left for the next cron run (the event fires every minute), so a single request cannot run unbounded. Batch size (default 1000) and passes per run (default 10) are filterable via wp_presence_cleanup_batch_size / wp_presence_cleanup_max_passes. Adds tests covering the per-invocation bound (remainder carried to the next run) and that fresh entries are never removed. Fixes WordPress#206 ## Use of AI Tools AI assistance: Yes Tool(s): Claude Code Model(s): Claude Opus 4.8 Used for: Investigating core's batching convention, implementing the fix, and writing tests
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
▶ Preview in WordPress PlaygroundBoots a fresh WordPress with this PR's presence-api build, seeds 5 demo users, and drops you on the dashboard. Stress-test variant: 40 demo users · Built from |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #258 +/- ##
============================================
+ Coverage 72.51% 72.63% +0.12%
Complexity 175 175
============================================
Files 14 14
Lines 1819 1831 +12
============================================
+ Hits 1319 1330 +11
- Misses 500 501 +1
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
The interpolated %d placeholder list trips WordPress.DB.PreparedSQLPlaceholders.UnfinishedPrepare, not ReplacementsWrongNumber; suppress the correct code.
What & why
Resolves #206.
wp_delete_expired_presence_data()(includes/functions.php) batched deletions with a MySQL-onlyDELETE ... LIMITinside a loop that had no iteration ceiling:Two divergences from core, both from #206:
DELETE ... LIMITis MySQL-specific and appears nowhere inwp-includes, so it breaks on non-MySQL backends, including the SQLite integration Playground uses to run the demo blueprints.max_execution_time, committing an unpredictable fraction of the work.Change
Follows core's batching convention — select a bounded set of primary keys, delete by key, a fixed number of passes per invocation, remainder to the next run:
SELECTs a page ofids older than the cutoff (LIMIT %d) and deletes them with a portableWHERE id IN ( ... )built from%dplaceholders passed throughprepare().1000) and passes per run (default10) are filterable viawp_presence_cleanup_batch_sizeandwp_presence_cleanup_max_passes.No schema change; behavior on a normally-drained table is unchanged.
Tests
Added to
tests/test-functions.php(@covers ::wp_delete_expired_presence_data):test_cleanup_is_bounded_per_invocation— withbatch_sizeandmax_passesfiltered to2, seeds 5 expired rows and asserts one invocation deletes at most2 × 2 = 4, leaving the remainder; a second invocation clears it. This locks in the "remainder to the next run" guarantee.test_cleanup_leaves_fresh_entries_untouched— fresh rows survive cleanup even with a batch size of one.The existing
test_cleanup_removes_expired_entriescontinues to pass.Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 4.8
Used for: Investigating core's batching convention, implementing the fix, and writing tests