Skip to content

Run queries on a connection one at a time so transactions stay on one session - #223

Open
just-some-random-pal wants to merge 1 commit into
planetscale:mainfrom
just-some-random-pal:fix/serialize-connection-queries
Open

just-some-random-pal wants to merge 1 commit into
planetscale:mainfrom
just-some-random-pal:fix/serialize-connection-queries

Conversation

@just-some-random-pal

Copy link
Copy Markdown

Fixes #191.

Every request carries the session the previous response returned, and Connection.execute only stores the new session after the response arrives. Two requests in flight on one connection both send the stale session, and the server treats the second as a new session or rejects it with "transaction ... in use". Inside transaction() that means a query runs outside the transaction, or COMMIT/ROLLBACK runs on a session with no transaction while the real one stays open until it times out, blocking every writer on the table. The failure path in #191 is the common one: the callback throws while a query it started is still running, ROLLBACK is sent immediately with the pre-query session, and the update's transaction is never rolled back.

This serializes requests per Connection through a FIFO chain, the same approach mysql2 takes per connection and the one discussed on #139. Each request now runs after the previous one has settled, so it always carries that response's session. A failed request does not block the ones behind it, so ROLLBACK still runs after a failed query. COMMIT and ROLLBACK therefore wait for every query the callback started, awaited or not, and run on the session that query produced. refresh() goes through the same chain.

What changes for callers: queries on one Connection no longer overlap. Client.execute and Client.transaction already create a connection per call, and Client.connection() hands out new ones, so parallel work across connections is unaffected. Promise.all over one connection still works, it just runs the queries in order, which is the only order the session protocol can honor. The README's transaction section now says so.

Tests, with a fetch that returns a new session per response and can hold a response open: a second query is not sent while the first is in flight and carries its session; a failed query does not block the next; a callback that throws while its query is running sends ROLLBACK only after that query completes and with its session; un-awaited inserts run before COMMIT. All four fail on main and pass with the change. The full suite is 46/46 and npm run lint is clean.

… session

Every request carries the session the previous response returned, and
Connection.execute only stores the new session after the response
arrives. Two requests in flight on one connection therefore both send
the stale session, and the server treats the second as a new, unrelated
session. Inside transaction() that meant a query ran outside the
transaction, or COMMIT/ROLLBACK ran on a session with no transaction
while the real one stayed open until it timed out, blocking writers.
A callback that throws while a query it started is still running hit
this on every failure path.

Serialize requests per Connection through a FIFO chain. A failed request
does not block the ones behind it, so ROLLBACK still runs after a failed
query, and COMMIT/ROLLBACK now wait for every query the callback
started, awaited or not, and run on the session that query produced.
Parallelism across connections is unchanged.

Fixes planetscale#191
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.

transaction rollback problems

1 participant