Skip to content

Fix failed SQLite payload commit cleanup; document remaining OOM recovery - #65

Draft
marcobambini wants to merge 4 commits into
pg-fixes11092026from
codex/sqlite-apply-transaction-cleanup
Draft

marcobambini wants to merge 4 commits into
pg-fixes11092026from
codex/sqlite-apply-transaction-cleanup

Conversation

@marcobambini

@marcobambini marcobambini commented Sep 19, 2026

Copy link
Copy Markdown
Member

Payload apply can return a deferred foreign-key or busy-commit error while leaving its internally opened SQLite transaction active. Uncommitted rows remain visible and a later BEGIN fails.

This change centralizes error cleanup, rolls back a failed transaction only when apply started it from autocommit mode, preserves the original error and committed prefixes, and excludes rolled-back rows from applied statistics. It also rejects a failed group-savepoint open and fixes a primary-key-name allocation leak found by fault injection. Caller-owned transactions remain under the caller's control.

Validation

  • 100 deferred-constraint failure/retry cycles, covering final and intermediate source-version commit boundaries.
  • 30 repeated reader-blocked commits, followed by successful retry.
  • Caller transaction and savepoint preservation, unchanged checkpoints, data/metadata rollback and connection reuse.
  • Core and audit suites pass under ASan/UBSan, including the SQLite amalgamation; zero outstanding SQLite memory.
  • Independent PostgreSQL 15.19 compatibility suite: 521 checks pass.
  • Negative control: restoring the old apply implementation fails 400 assertions in the initial deferred-constraint tests.

Why this is a draft

The associated engine-level OOM issue from the report is not fully fixed. The allocation sweep covers indices 0–347 and now reports zero leaks and zero failed retries after explicit recovery, but 126 attempts still leave a transaction open. SQLite rejects reentrant cleanup SQL while its outer SQL function is executing with a malloc-failed/interrupted connection. A complete solution needs host-side recovery or a change to the SQL apply execution model. The ASan/UBSan sweep reaches index 351 with 128 open transactions, zero leaks, zero failed retries after recovery and no sanitizer diagnostics; allocation positions depend on the build. The diagnostic test/stress/payload_oom.c deliberately exits nonzero for those remaining failures; it is not represented as a passing test.

docs/internal/apply-transaction-cleanup.md documents the fix, reproduction command, results and remaining recovery requirement.

This is one of three independent follow-ups to #64, based on pg-fixes11092026 at 9d0abb3. Retarget to main after #64 merges. No live cloud endpoint was used or modified.

Cloud integration reliability

The shared chunked-tenant job is serialized across branches with a queued concurrency group. This prevents a different PR from writing during the negative-cache test's required idle phase; all idle assertions remain intact, and other platforms still run in parallel.

Fresh receivers now wait for actual received rows and expected fixture data rather than requiring the first poll to contain rows. Polling is bounded and SQL/protocol failures abort immediately. A materialized result ensures one network call per attempt. The offline integration_bootstrap test runs as part of make unittest, covering 312 scenarios including delayed/partial delivery, timeout, missing data, protocol errors and malformed responses. Local ordinary and ASan/UBSan runs pass with zero outstanding SQLite memory.

Latest CI verification

Workflow run 35443339032 completed successfully on a35c0eb: 37 jobs passed, with only the release job skipped. The real-cloud negative-cache test ran and passed on Linux x86_64.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant