sql: restore cursor state during automatic retries - #174676
Conversation
Snapshot cursor membership and iterator positions alongside other rewindable session state. Retain snapshotted cursor resources so replayed CLOSE operations can be undone, and make lazy cursors rewindable before retried FETCH or MOVE operations consume them. Release note (bug fix): Automatic transaction and READ COMMITTED statement retries no longer advance or close SQL cursors more than once.
|
Thank you for contributing to CockroachDB. Please ensure you have followed the guidelines for creating a PR. My owl senses detect your PR is good for review. Please keep an eye out for any test failures in CI. I have added a few people who may be able to assist in reviewing: 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
Metamorphic validation and design notesI validated the bug with paired executions in which the cursor query and query plan are identical; the only transformed input is the retry trigger (
The first execution-path divergence occurs after a retryable error. The executor truncates the failed attempt's buffered results and restores its transaction/KV checkpoint, but the SQL cursor map and iterator stay mutated. Each replay therefore begins from the state left by the previous failed attempt. A smaller-looking fix that saves only Ordinary cursors add another constraint: Validation completed locally:
|
Fixes #173505.
Problem
SQL cursors are mutable session state, but their membership and iterator position were not part of either automatic-retry checkpoint:
sqlCursors;As a result, replaying
FETCHorMOVEconsumed rows twice, while replayingCLOSEfailed because the first attempt had already removed and destroyed the cursor.Solution
Snapshot cursor-map membership and the logical/persisted iterator position together with the existing rewindable state. Cursor resources are retained while a snapshot owns them, so a replayed
CLOSEcan be undone safely. Whole-transaction rewinds restore the transaction snapshot, and READ COMMITTED retries restore a statement-local snapshot after rolling back the KV savepoint.Lazy cursors remain lazy at
DECLARE. Immediately before a protectedFETCHorMOVE, the cursor iterator is made seekable so its exact position can be restored. If that conversion fails before the cursor becomes rewindable, CockroachDB now returns the retry error instead of transparently replaying from corrupted cursor state.Testing
Added regression coverage for:
FETCHwith no retry versus three transaction retries;CLOSEon a held cursor;FETCHwith no retry versus exactly three statement retries.The full sharded
//pkg/sql:sql_testtarget and the cursor-related local and local-read-committed logic tests pass.Release note (bug fix): Automatic transaction and READ COMMITTED statement retries no longer advance or close SQL cursors more than once.