Fix permanent badge inflation from read receipts before first rotation#19785
Open
stefanceriu wants to merge 1 commit into
Open
Fix permanent badge inflation from read receipts before first rotation#19785stefanceriu wants to merge 1 commit into
stefanceriu wants to merge 1 commit into
Conversation
_handle_new_receipts_for_notifs_txn used simple_update_txn to record last_receipt_stream_ordering, which silently no-ops if no summary row exists yet. _rotate_notifs_before_txn then INSERTed the row with last_receipt_stream_ordering=NULL. The badge query treats NULL as "trust the counts", and since highlights survive the receipt DELETE, they get re-counted on every rotation, growing the badge unboundedly. Pre-populate event_push_summary rows with the receipt position for every thread with pending push actions, and switch the threaded path to upsert. A SQL migration backfills existing NULL rows from receipts_linearized.
stefanceriu
force-pushed
the
stefan/fixBadgeInflation
branch
from
d30a049 to
edbba7f
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
We've had problems with the iOS badge numbers for the longest time, with element-hq/element-x-ios#3151 being our most commented on issue. We were never quite sure what the root cause was nor had time to look into it but so I decided to let Claude have a go. I'm too far away from Synapse to have enough context here but the problem it found (and subsequent regression tests) do make a lot of sense.
If a user reads a room before its first
_rotate_notifscycle, the receipt position is dropped:simple_update_txnno-ops because noevent_push_summaryrow exists yet, then rotation INSERTs the row withlast_receipt_stream_ordering=NULL. The badge query treats NULL rows as up-to-date, so already-read highlights (which survive the receipt DELETE) get re-counted, and inflation accumulates on every rotation._handle_new_receipts_for_notifs_txnusedsimple_update_txnto recordlast_receipt_stream_ordering, which silently no-ops if no summary row exists yet._rotate_notifs_before_txnthenINSERTed the row withlast_receipt_stream_ordering=NULL. The badge query treatsNULLas "trust the counts", and since highlights survive the receiptDELETE, they get re-counted on every rotation, growing the badge unboundedly.Pre-populate
event_push_summaryrows with the receipt position for every thread with pending push actions, and switch the threaded path to upsert. A SQL migration backfills existingNULLrows fromreceipts_linearized.