sql: simplify connection state machine - stop tracking retry intent

[andreimatei]

Before this patch, the SQL connection state machine had an optimization:
if a transaction that hadn't used "SAVEPOINT cockroach_restart"
encountered a retriable error that we can't auto-retry, then we'd
release the txn's locks eagerly and enter the Aborted state. As opposed
to transactions that had used the "SAVEPOINT cockroach_restart", which
go to RestartWait.
This optimization is a significant complication for the state machine,
so this patch is removing it. All transactions now go to RestartWait,
and wait for a ROLLBACK to release the locks.

On the flip side, doing "RELEASE SAVEPOINT cockroach_restart" and
"ROLLBACK SAVEPOINT cockroach_restart" now works even for transactions
that haven't explicitly declared that savepoint, which is nice. Although
I don't promise I'll keep it working.

Release note: None

[cockroach-teamcity]

This change is 

[andreimatei]

cc @knz

before:

after:

[nvanbenschoten]

then we'd release the txn's locks eagerly and enter the Aborted state.

I did some testing with PG and it's actually pretty smart. It seems to eagerly release all locks acquired in the current savepoint scope when an error is hit, but none of the locks that were acquired in other scopes. For instance, the following would allow concurrent transactions that want to write to "b" to proceed but not those that want to write to "a":

BEGIN;
SAVEPOINT x;
INSERT INTO kv VALUES ('a', 1);
SAVEPOINT y;
INSERT INTO kv VALUES ('b', 2);
INSYNTAXERROR;

Is this something you'd like to support? If so then we'd actually want something like what we had before, right?

Other than this question though, the code changes here LGTM.

Reviewed 9 of 9 files at r1.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @jordanlewis)

[andreimatei]

Is this something you'd like to support?

If anything, the case that I'd be most interested in improving is the case in which no savepoint has been used. But even there I don't care too much; it's optimizing for errors after all. The code is definitely simpler without caring about this. But, relatedly, allowing for partial retries after a serializability failure - I think there's money in that.

bors r+

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @jordanlewis and @nvanbenschoten)

[craig]

Build failed (retrying...)

• GitHub CI (Cockroach)

[craig]

Build succeeded

• GitHub CI (Cockroach)