Conversation
In large groups a cohort of members whose pairwise sessions never establish (dead registrations, exhausted prekeys, re-registered LIDs) sends a retry receipt for every single group message. Each receipt pays markForgetSenderKey (a DB write plus a whole-group sender-key cache invalidation), bundle processing and possibly a resend - all upstream of the per-chat resend cap (oxidezap#871), which only bounds the resend itself. Observed in production: ~58k "Marked ... for fresh SKDM" in 3.5 days from 468 distinct members of a single 1012-participant group (~99.7% of all marks), with the per-chat cap blocking ~75% of the resends but none of the repair work. This bounds the whole repair path per member with a token bucket keyed by (chat user, requester user): burst 2, refill 2/day. One mark is enough to repair a healthy member (the next send carries the SKDM), so past the burst further receipts from the same pair are dropped before any work happens; the refill keeps genuine recovery possible. Device is excluded from the key on purpose - WA Web re-targets the whole user when the primary goes cold, so all devices of a broken account share one budget. - RetryMarkQuarantine reuses the existing TokenBucket/Cache machinery from the resend limiter (lazy monotonic refill, capacity-only cache) - burst 0 disables; live-tunable via Client::set_retry_mark_quarantine - observability: StatsSnapshot.retry_receipts_quarantined counter and retry_mark_quarantine_pairs in MemoryDiagnostics
- cache_config.retry_mark_quarantine_capacity (default 32768): the quarantine keyspace is O(groups x broken members), not O(chats), and evicting an ACTIVE pair refunds its burst - sharing the resend limiter's per-chat capacity (4096) undersized it (greptile P1) - gate moved before the group-info fetch and the rotateKey path, so a quarantined pair no longer pays query_info nor the unknown-sender rotateKey (own sender-key deletion + whole-group invalidation) at full rate; still after the message-cache lookup so expired-message no-ops don't burn the repair budget (greptile P2) - documented the owned-key allocation tradeoff in try_acquire
Mirrors the ResendRateLimiter contention test: 40 concurrent receipts for the same (chat, requester) pair, asserting exactly burst pass and the rest are throttled — proving burst enforcement holds under the concurrent-receipt storm the quarantine guards (coderabbit/cubic).
is_peer (our own account's secondary devices) was subject to the quarantine gate: a companion that was offline or rotated keys can legitimately need many group-session repair cycles, and dropping those after the burst would block group decryption on our own device. The storm this guards is third-party members, never our own. (greptile)
Owner
Author
|
Replaced by new review run |
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
2 participants
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.
FriendlyReviewer review of oxidezap/whatsapp-rust#982.