Skip to content

Persist pending db_version in transaction guard vtab xSync - #29

Merged
gorbak25 merged 2 commits into
mainfrom
gorbak/db-version-commit-guard
Oct 13, 2026
Merged

gorbak25 merged 2 commits into
mainfrom
gorbak/db-version-commit-guard

Conversation

@gorbak25

Copy link
Copy Markdown

A statement can be rolled back via the statement journal without rolling back the enclosing transaction:

CREATE TABLE foo(id INTEGER PRIMARY KEY);
BEGIN;
-- The first row inserts and fires all AFTER INSERT triggers;
-- the second row fails and the whole statement is rolled back.
INSERT INTO foo VALUES (1), (1);
-- The transaction is still open, so this succeeds.
INSERT INTO foo VALUES (2);
COMMIT;

We assign a new db_version inside triggers. If the statement is rolled back, the write of that db_version is undone too, but our in-memory state still says it was persisted, so we never write it again. After commit, a new connection sees the old db_version and reuses it. Rolled-back savepoints hit the same problem. #26 added more savepoints, which makes this the likely root cause of https://status.flyio.net/incidents/s9dzwg3d4bs7.

This PR moves db_version handling into a wrapper vtab that persists the pending db_version in its xSync callback, so it is always written at commit regardless of statement or savepoint rollbacks.

The trade-off is that transactions like this can leave gaps in db_version:

BEGIN;
SAVEPOINT test;
INSERT INTO foo VALUES (2);
ROLLBACK TO test;
COMMIT;

Gaps are safe, unlike reuse. If they become a problem, we can fix them by tracking savepoints properly.

@gorbak25
gorbak25 added this pull request to stack #30 October 11, 2026 22:14
@gorbak25
gorbak25 requested a review from somtochiama October 11, 2026 22:15
@gorbak25
gorbak25 force-pushed the gorbak/db-version-commit-guard branch from 2f36bde to d1e1900 Compare October 12, 2026 10:54

@somtochiama somtochiama left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a few nits but LGTM!

Comment thread core/src/version-guard.c Outdated
Comment thread core/src/version-guard.c Outdated
Base automatically changed from gorbak/reload-config-when-db-version-changes to main October 12, 2026 15:48
@gorbak25
gorbak25 force-pushed the gorbak/db-version-commit-guard branch from d1e1900 to 43f698d Compare October 12, 2026 15:48
The first local write of a transaction enlists the crsql_version_guard
virtual table; its xSync upserts the pending db_version into
crsql_db_versions atomically before commit. A statement or savepoint
rollback can no longer leave clocks committed under a db_version whose
version marker was rolled back and later reused.

A committed transaction may spend a version even when every write was
rolled back to a savepoint; such gaps are harmless while reuse of a
committed db_version is not.

(cherry picked from commit f5db9a80, ported to V1 clock tables)

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@gorbak25
gorbak25 force-pushed the gorbak/db-version-commit-guard branch 6 times, most recently from 3ab45d3 to 36c245e Compare October 13, 2026 10:46
Once crsql_version_guard is enlisted, the transaction's db_version is
fixed; next_db_version and peek_next_db_version now return
pendingDbVersion directly instead of re-running the max() allocation
math. The un-enlisted path keeps a single fill_db_version_if_needed
refresh before allocating dbVersion + 1 and enlisting.

Also document that the dbVersion != -1 check in
refresh_connection_state_if_needed is the first-load guard for the
lazily-initialized version cache, not a data_version condition.

Generated with [Devin](https://devin.ai)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@gorbak25
gorbak25 force-pushed the gorbak/db-version-commit-guard branch from 36c245e to 1b7123f Compare October 13, 2026 14:38
@gorbak25
gorbak25 merged commit 8e32195 into main Oct 13, 2026
9 checks passed
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.

2 participants