Skip to content

sql,kv: permit txn rollbacks across LockNotAvailable errors#67514

Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom
nvb:nvanbenschoten/writeIntentErr
Jul 14, 2021
Merged

sql,kv: permit txn rollbacks across LockNotAvailable errors#67514
craig[bot] merged 1 commit intocockroachdb:masterfrom
nvb:nvanbenschoten/writeIntentErr

Conversation

@nvb
Copy link
Copy Markdown
Contributor

@nvb nvb commented Jul 12, 2021

This commit adds support for rolling a transaction back across a LockNotAvailable (pgcode 55P03) error. LockNotAvailable errors are returned in two cases:

  1. when a locking SELECT is run with a NOWAIT wait policy and conflicts with an active lock
  2. when a statement is run with a lock_timeout and this timeout is exceeded (unsupported, see sql/kv: implement lock_timeout session variable #67513)

The following test case from pkg/sql/testdata/savepoints demonstrates this new capability:

# txn1
BEGIN
INSERT INTO t VALUES (1)
----
1: BEGIN -- 0 rows
-- NoTxn       -> Open        #.  (none)
2: INSERT INTO t VALUES (1) -- 1 row
-- Open        -> Open        ##  (none)

# txn2
BEGIN
SAVEPOINT foo
SELECT * FROM t WHERE x = 1 FOR UPDATE NOWAIT
ROLLBACK TO SAVEPOINT foo
SELECT * FROM t WHERE x = 2 FOR UPDATE NOWAIT
COMMIT
----
1: BEGIN -- 0 rows
-- NoTxn       -> Open        #.....  (none)
2: SAVEPOINT foo -- 0 rows
-- Open        -> Open        ##....  foo
3: SELECT * FROM t WHERE x = 1 FOR UPDATE NOWAIT -- pq: could not obtain lock on row (x)=(1) in t@primary
-- Open        -> Aborted     XXXXXX  foo
4: ROLLBACK TO SAVEPOINT foo -- 0 rows
-- Aborted     -> Open        ##....  foo
5: SELECT * FROM t WHERE x = 2 FOR UPDATE NOWAIT -- 0 rows
-- Open        -> Open        ##..#.  foo
6: COMMIT -- 0 rows
-- Open        -> NoTxn       ##..##  (none)

This becomes the second error type that supports rollbacks, with the first being duplicate key errors, which was added in 65e8045.

The primary motivation for this PR was to be able to give WriteIntentErrors an ErrorPriority of ErrorScoreUnambiguousError for #66146. However, the added functionality fell out of making that change.

Release note (sql change): ROLLBACK TO SAVEPOINT can now be used to recover from LockNotAvailable errors (pgcode 55P03), which are returned when performing a FOR UPDATE SELECT with a NOWAIT wait policy.

This commit adds support for rolling a transaction back across a
`LockNotAvailable` (pgcode 55P03) error. `LockNotAvailable` errors
are returned in two cases:
1. when a locking SELECT is run with a NOWAIT wait policy and conflicts
   with an active lock
2. when a statement is run with a `lock_timeout` and this timeout is
   exceeded (unsupported, see cockroachdb#67513)

The following test case from `pkg/sql/testdata/savepoints` demonstrates
this new capability:

```
// txn1
BEGIN
INSERT INTO t VALUES (1)
----
1: BEGIN -- 0 rows
-- NoTxn       -> Open        #.  (none)
2: INSERT INTO t VALUES (1) -- 1 row
-- Open        -> Open        ##  (none)

// txn2
BEGIN
SAVEPOINT foo
SELECT * FROM t WHERE x = 1 FOR UPDATE NOWAIT
ROLLBACK TO SAVEPOINT foo
SELECT * FROM t WHERE x = 2 FOR UPDATE NOWAIT
COMMIT
----
1: BEGIN -- 0 rows
-- NoTxn       -> Open        #.....  (none)
2: SAVEPOINT foo -- 0 rows
-- Open        -> Open        ##....  foo
3: SELECT * FROM t WHERE x = 1 FOR UPDATE NOWAIT -- pq: could not obtain lock on row (x)=(1) in t@primary
-- Open        -> Aborted     XXXXXX  foo
4: ROLLBACK TO SAVEPOINT foo -- 0 rows
-- Aborted     -> Open        ##....  foo
5: SELECT * FROM t WHERE x = 2 FOR UPDATE NOWAIT -- 0 rows
-- Open        -> Open        ##..#.  foo
6: COMMIT -- 0 rows
-- Open        -> NoTxn       ##..##  (none)
```

This becomes the second error type that supports rollbacks, with the
first being duplicate key errors, which was added in 65e8045.

The primary motivation for this PR was to be able to give
`WriteIntentErrors` an `ErrorPriority` of `ErrorScoreUnambiguousError`.
However, the added functionality fell out of making that change.

Release note (sql change): ROLLBACK TO SAVEPOINT can now be used to
recover from LockNotAvailable errors (pgcode 55P03), which are returned
when performing a FOR UPDATE SELECT with a NOWAIT wait policy.
@nvb nvb requested a review from andreimatei July 12, 2021 23:01
@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

Copy link
Copy Markdown
Contributor

@andreimatei andreimatei left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewed 2 of 12 files at r1.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @nvanbenschoten)


pkg/sql/testdata/savepoints, line 230 at r1 (raw file):

-- Open        -> NoTxn       ##..##  (none)

# NB: the txn on the "conflict" connection is aborted by the test harness even

hmm, why is that txn aborted?

Copy link
Copy Markdown
Contributor Author

@nvb nvb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TFTR!

bors r+

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @andreimatei)


pkg/sql/testdata/savepoints, line 230 at r1 (raw file):

Previously, andreimatei (Andrei Matei) wrote…

hmm, why is that txn aborted?

That's what the comment is saying. The test harness aborts it here:

_, _ = sqlConn.Exec("ABORT")

Copy link
Copy Markdown
Contributor

@andreimatei andreimatei left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @nvanbenschoten)


pkg/sql/testdata/savepoints, line 230 at r1 (raw file):

Previously, nvanbenschoten (Nathan VanBenschoten) wrote…

That's what the comment is saying. The test harness aborts it here:

_, _ = sqlConn.Exec("ABORT")

well the test harness seems silly to me. I don't think that rollback had in mind this ability to use multiple connections simultaneously.

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Jul 14, 2021

Build failed (retrying...):

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Jul 14, 2021

Build succeeded:

@craig craig bot merged commit 5a0b11e into cockroachdb:master Jul 14, 2021
@nvb nvb deleted the nvanbenschoten/writeIntentErr branch July 20, 2021 17:35
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.

3 participants