fix(module-postgres-storage): only resnapshot rows with missing TOAST values - #788
Open
henriquekraemer wants to merge 1 commit into
Open
Conversation
… values saveOperation() marked every UPDATE without a current_data entry for resnapshot, even when the row arrived complete from the WAL. The complete row is evaluated and written to current_data on the same pass, so the resnapshot re-read a row the service already had, on a separate snapshot connection, followed by a keepalive and a wait for the next consistent checkpoint. A backfill touching rows not yet snapshotted queued one resnapshot per row (about a million rows on one deployment, close to an hour before confirmed_flush_lsn moved again). Gate the resnapshot on isCompleteRow() instead, matching the MongoDB storage since powersync-ja#655. Rows with missing TOAST values still go through resnapshot; complete rows take the normal path.
🦋 Changeset detectedLatest commit: 5f2df1d The changes in this PR will be included in the next version bump. This PR includes changesets to release 12 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
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.
Problem
PostgresBucketBatch.saveOperation()handles a streamed UPDATE for a row with nocurrent_dataentry by callingmarkRecordUnavailable()wheneverstoreCurrentDatais true, which queues a targeted resnapshot of that row. It does this even when the row arrived complete from the WAL. The complete row is evaluated and written tocurrent_dataon the same pass (theisCompleteRowgate before evaluation, then the unconditionalupsertCurrentData), so the resnapshot re-reads a row the service already has: a query on a separate snapshot connection, a keepalive, and a wait for the next consistent checkpoint beforeconfirmed_flush_lsnmoves again.The MongoDB storage only resnapshots when the row is incomplete (
!isCompleteRow(storeCurrentData, after)inMongoBucketBatch). That guard came with #655, which also touchedPostgresBucketBatchbut left this branch on the old condition. The same reasoning appears in the #655 review: a row that arrives complete needs no targeted resnapshot (#655 (comment)).We hit this with a backfill that updated about a million rows on tables whose snapshot had not reached them yet. Every one of those rows queued a resnapshot; the queue drained in batches over close to an hour, and the slot's
confirmed_flush_lsnstayed put until it did. The cost is paid once per row (the first UPDATE while the row is missing from storage), so it self-limits, but it repeats on every large backfill or onboarding.Fix
Gate the resnapshot on
isCompleteRow(storeCurrentData, after), matching the MongoDB storage. Rows with missing TOAST values still go through resnapshot as before; complete rows take the normal path and are stored directly. WithstoreCurrentDatafalse the guard is never entered, same as today.Tests
New test in the shared storage suite (
register-data-storage-data-tests.ts), so it runs against both storages: an UPDATE with a complete row for a record not in storage must not callmarkRecordUnavailable, a following partial UPDATE on the same record must be filled from the stored copy, and a partial UPDATE for another missing record must be marked. It fails on Postgres storage v1 and v2 without the fix and passes with it; it passes unchanged on the MongoDB storage suite. The module-postgres-storage suite passes against real Postgres (174 tests). Changeset included (patch).