tldr: The new implementation of savepoints and SELECT FOR UPDATE (row locks) are currently behaving in a way incompatible with postgres. This is in direct violation to our commitment to pg compatibility, and introduces the risk of application-level deadlocks.

This issue is to ensure ROLLBACK TO SAVEPOINT either release locks, like in pg, or fail with an "unimplemented error".

This has been discussed with @tbg

Details

• pg says "ROLLBACK TO SAVEPOINT" must release locks placed "under" the savepoint
• crdb currently does not do this and keeps the locks active.

There are two problems with this:

• this is different from pg (violation to our commitment to pg compat)
• it creates a risk of application-level deadlock. I recall at least one occurrence of a test that demonstrates this in the Hibernate test suite. The application takes a lock, then starts a separate thread that waits on the lock, then in the first thread releases the lock with ROLLBACK TO SAVEPOINT, then expects the separate threat to resume execution. Today this would not work in crdb.

Nuance: rollback after a serialization error ("retry error")

The current crdb behavior is motivated as follows: after a retry error, keeping the locks active during ROLLBACK TO SAVEPOINT increases the chances that the new txn attempt will succeed.

However the current implementation keeps the locks unconditionally, causing the pg compat problem described above.

What we can do instead is the following:

• After a retry error keep the locks and allow ROLLBACK TO SAVEPOINT
• After other types of error either release the locks, like in pg, or report an error to the client that ROLLBACK TO SAVEPOINT is not supported after a FOR UPDATE query

Nuance: implicit locks under UPDATE statements

The implementation places locks for UPDATE statements. Naively preventing ROLLBACK TO SAVEPOINT after locks have been placed would block the ability to do ROLLBACK TO SAVEPOINT after an UPDATE statement. This would strongly limit the savepoint feature.

However, looking at the detail: the locks laid by an UPDATE statement are subsequently converted to write intents. If our understanding (knz+@tbg) is correct, there is no lock left over after the UPDATE statement completes (assuming no txn conflict yielding a retry error). Intents can be rolled back without issue with ROLLBACK TO SAVEPOINT.