fix: wrap simpleUpdateRecords with transaction, timeout, and retry#2677
Open
dkindlund wants to merge 1 commit intoteableio:developfrom
Open
fix: wrap simpleUpdateRecords with transaction, timeout, and retry#2677dkindlund wants to merge 1 commit intoteableio:developfrom
dkindlund wants to merge 1 commit intoteableio:developfrom
Conversation
simpleUpdateRecords calls the full computation pipeline (computed orchestrator, linkService.planDerivateByLink, commitForeignKeyChanges, batchService.updateRecords) without any transaction wrapping, timeout, or deadlock retry. If any step fails partway through, junction table changes can be committed while JSONB updates are not, computed field evaluations can be partially applied, and __version can be corrupted due to a read-then-write race without FOR UPDATE locking. This is inconsistent with updateRecords which wraps in $tx with bigTransactionTimeout and @retryOnDeadlock. This fix adds: - @retryOnDeadlock() decorator for transient deadlock recovery - $tx wrapper with bigTransactionTimeout for atomicity and timeout protection Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
dkindlund
added a commit
to dkindlund/teable
that referenced
this pull request
Mar 9, 2026
## Problem The /link-fix endpoint's InvalidLinkReference repair has a race condition under concurrent write load. The current two-step approach: 1. checkLinks() — SELECT to detect desynced record IDs 2. fixLinks(recordIds) — UPDATE only those specific records Between steps 1 and 2, concurrent writes can: - Create NEW desyncs not in the recordIds list (missed until next run) - Worsen existing desyncs (JSONB changes between detection and fix) - Overwrite the fix immediately after it's applied In production under sustained concurrent API load (200-800 writes/hr), we've observed full JSONB array wipes (e.g., 9,781 entries reduced to 0) caused by the read-modify-write race in Teable's link update path. The integrity fix endpoint, meant to repair these desyncs, is vulnerable to the same concurrent writes because of this detection-fix gap. ## Solution Add atomicFixLinks() to IntegrityQueryPostgres that combines detection and fix into a single UPDATE ... WHERE __id IN (SELECT ...) statement. Detection and fix execute under the same MVCC snapshot, eliminating the application-level gap entirely. The service (LinkFieldIntegrityService.checkAndFix) tries the atomic method first. If the database engine doesn't support it (e.g., SQLite), it falls back to the existing two-step approach. This is a safe, backwards-compatible change — SQLite behavior is completely unchanged. ## What the /link-fix endpoint covers For context, the endpoint handles 10 integrity issue types. This PR improves only InvalidLinkReference. Here is the full list: | Issue Type | What it detects | Fix applied | |---|---|---| | InvalidLinkReference | JSONB link column diverged from junction/FK source of truth | Rebuilds JSONB from junction/FK data (THIS PR) | | MissingRecordReference | Junction table has rows pointing to deleted records | Deletes orphaned junction rows | | ForeignTableNotFound | Link field references a deleted table | No auto-fix (requires manual intervention) | | ForeignKeyHostTableNotFound | Junction table is missing | No auto-fix | | ForeignKeyNotFound | Missing FK columns in junction table | Recreates columns, backfills from JSONB | | SelfKeyNotFound | Missing self-reference key in junction | No auto-fix | | SymmetricFieldNotFound | Bidirectional link missing its counterpart | Converts to one-way link | | ReferenceFieldNotFound | Referenced record was deleted | Deletes orphaned reference | | UniqueIndexNotFound | Missing unique constraint for OneOne links | Creates the index | | EmptyString | Text fields have empty strings instead of NULL | Converts to NULL | ## Relationship types handled The atomic fix handles all four relationship types: - ManyMany (isMultiValue=true): Rebuilds JSONB array from junction table - OneMany (isMultiValue=true): Same as ManyMany - ManyOne (isMultiValue=false, FK in same table): Rebuilds single JSONB object from FK column - OneOne (isMultiValue=false, FK in host table): Rebuilds via cross-table join ## Files changed - abstract.ts: Added atomicFixLinks() with default null return - integrity-query.postgres.ts: PostgreSQL implementation of atomicFixLinks() - link-field.service.ts: checkAndFix() tries atomic first, falls back to two-step ## Related issues - teableio#2680 (DataLoader cache invalidation under concurrency) - teableio#2676 (Sort record IDs in lockForeignRecords) - teableio#2677 (Wrap simpleUpdateRecords with transaction/timeout/retry) - teableio#2679 (Add foreign record locking to ManyMany, OneMany, OneOne) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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.
Bug Description
simpleUpdateRecordsinRecordOpenApiServicecalls the full computation pipeline (computed orchestrator, linkService.planDerivateByLink, commitForeignKeyChanges, batchService.updateRecords) without any transaction wrapping, timeout, or deadlock retry.If any step fails partway through, junction table changes can be committed while JSONB updates are not, computed field evaluations can be partially applied, and
__versioncan be corrupted due to a read-then-write race without FOR UPDATE locking.This is inconsistent with
updateRecordswhich wraps in$txwithbigTransactionTimeoutand@retryOnDeadlock.What This Fix Does
@retryOnDeadlock()decorator for transient deadlock recovery$txwithbigTransactionTimeoutfor atomicity and timeout protectionMatches the pattern already used by
updateRecordsandmultipleCreateRecords.Client Information
Platform
Docker standalone (Google Cloud Run)