mirror of
https://github.com/element-hq/synapse.git
synced 2026-06-20 13:13:02 +00:00
Erik Johnston
d7e9a3ff83
The sytest `After /purge_history users still get pushed for new messages` is flaky. The flakiness exposes a real bug rather than a test-timing issue. Notification counts are stored in two places: `event_push_actions` (one row per unread event) and `event_push_summary` (aggregate counts populated periodically by `_rotate_notifs`, which runs on a 30-second timer). `_purge_history_txn` deletes the purged events' rows from `event_push_actions` but never adjusts `event_push_summary` (only the full-room `purge_room` drops that table). So the result depends on a race: if rotation hasn't fired, counts come live from `event_push_actions`, the purge removes the right rows, and the count is correct. If rotation fires before the purge — more likely under the slower multi-postgres/workers/asyncio CI config — the events get folded into `event_push_summary`, the purge then deletes the underlying `event_push_actions` rows but leaves the summary untouched, and the count comes out inflated. ### Fix Before deleting the rotated rows from `event_push_actions`, decrement `event_push_summary` by the amount attributable to the events being deleted. The decrement mirrors the counting logic in `_rotate_notifs_before_txn`: only rows that were already rotated (`stream_ordering <= event_push_summary_stream_ordering`) and that fall after the summary's receipt are subtracted, so it stays correct in the presence of read receipts and unread/highlight rows. The SQL avoids `UPDATE ... FROM` and CTEs so it works on both SQLite and Postgres. End-of-purge cache invalidation already covers `get_unread_event_push_actions_by_room_for_user`. ### Tests Adds `test_count_aggregation_after_purge`, which forces a rotation before purging and asserts the aggregate count reflects only the surviving events, covering read receipts and a subsequent re-rotation. It fails (`3 != 1`) without the fix. --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>