-
Notifications
You must be signed in to change notification settings - Fork 4.1k
kv: reads rolled back by savepoints are not refreshed #111228
Description
Describe the problem
When a read occurs between a SAVEPOINT x and a ROLLBACK TO SAVEPOINT x, the read is first added to the set of refresh spans for the txn, and then removed from that set when the savepoint is rolled back. If the txn's read and write timestamps diverge, this read will not be refreshed, which can be seen as a serializability violation.
Going back to the commit that introduced this change, it seems like there was a reason for having this behavior. In short, if the client encounters a serializability error after creating a savepoint, they should be able to roll back to it and retry only a small amount of work, instead of retrying the entire txn. Currently, the only errors that can be recovered from by a savepoint rollback are ConditionFailedError and LockConflictError; in particular, recovering from a serializability error is not allowed. So the vision of the initial implementation didn't really come to life.
Moreover, the current behavior doesn't match the corresponding postgres behavior, which ensures the rolled back reads can be serialized.
To Reproduce
A simple write skew example with and without a savepoint can illustrate the issue.
create table kv (k int primary key, v int);
insert into kv values (1, 1);
insert into kv values (2, 2);
-- session 1
begin;
select * from kv where k = 1;
-- session 2
begin;
-- savepoint a;
select * from kv where k = 2;
-- rollback to savepoint a;
-- session 1
update kv set v = v+1 where k = 2;
-- session 2
update kv set v = v+1 where k = 1;
-- session 1
commit;
-- session 2
commit;
ERROR: restart transaction: TransactionRetryWithProtoRefreshError: TransactionRetryError: retry txn (RETRY_SERIALIZABLE - failed preemptive refresh due to encountered recently written committed value /Tenant/2/Table/112/1/2/0Without the two savepoint lines in session 2, committing the second transaction results in a serializability error. But with the savepoint and rollback added, both txns commit successfully.
In postgres, this example returns a serializability error with or without savepoints.
Additional context
This issue came out of the work to add savepoint support in kvnemesis. With sufficiently high probabilities for savepoint operations, kvnemesis encounters cases of non-serializable reads within a few hundred runs.
It's not clear if this has any impact on customer workloads, or if it's represented in any of the test workloads we run.
Jira issue: CRDB-31829