Skip to content

[Due for payment 2026-09-17] Count in-flight Pusher applies as applied when detecting update gaps #100313

Description

@MelvinBot

Problem

Pusher updates are applied one at a time through a shared promise chain, and the client's update watermark (lastUpdateIDAppliedToClient) only moves once an apply finishes. The next Pusher event says it follows the update that is still saving, so the gap check compares it against a watermark that has not caught up yet, sees a hole that does not exist, pauses the write queue, and fires GetMissingOnyxMessages to refetch a range the client is already writing.

The client already has the same problem for writes and already solves it: lastUpdateIDPendingFlush counts a WRITE update taken in but not yet flushed as applied, added in #98777. There is no equivalent for an update that is part-way through applying.

Measured on production logs for 2026-08-26 09:00 UTC: 1,794 of 7,316 gap detections (24.5%) were this false positive. Each one is a redundant round trip plus a write-queue pause, so the user's messages and expenses sit on the device while the client refetches data it already has.

Solution

Add a lastUpdateIDPendingApply marker, the apply-time counterpart to lastUpdateIDPendingFlush, so an update part-way through applying counts as applied during gap detection.

Only Pusher updates may read it. doesClientNeedToBeUpdated takes the transport the update arrived on, because treating an in-flight Pusher apply as applied is only safe for callers that are themselves serialized behind it on pusherEventsPromise. HTTPS applies run on an independent chain — were they to read the marker, they could advance the watermark past updates the held apply has not written yet, and a later rejection would leave no gap to recover from. Airship runs on a third chain and is likewise excluded.

The marker clears on every exit: a successful apply, any failed apply (not just a Pusher one, so an unrelated lower-ID failure is never masked), and sign-out. It stays out of getEffectiveLastUpdateID(), so a real gap still refetches from the persisted watermark rather than a shortened range.

No user-visible behaviour change on its own — the observable effect is fewer redundant fetches and fewer write-queue stalls under load.

Not in scope

#100174 (a single rejected Pusher apply poisons pusherEventsPromise for the rest of the session) was found while writing this change and is being handled separately. It is pre-existing on main. Note the ordering constraint recorded there: it must not be fixed before an explicit ID-continuity guard exists in advanceLastUpdateIDAfterApply, or this change turns into data loss.

PR

#99774

Issue OwnerCurrent Issue Owner: @mallenexpensify

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

Awaiting PaymentAuto-added when associated PR is deployed to productionDailyKSv2Task

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions