pingcap / pingcap/tidb

[txn] Pessimistic DML retry can change a SETVAR-derived unique key

Open
#69,791 0 comments 0 reactions 0 assignees View on GitHub
component/executor found-by-ai may-affects-7.5 may-affects-8.1 may-affects-8.5 severity/major sig/transaction type/bug
Dominant language
Go
Stars
40.5k
Forks
6.2k
PR merge metrics
PR metrics pending

Description

## Bug Report

### 1. Minimal reproduce step

Create the test table:

```sql
DROP DATABASE IF EXISTS pessimistic_setvar_retry;
CREATE DATABASE pessimistic_setvar_retry;
USE pessimistic_setvar_retry;

CREATE TABLE t(id INT PRIMARY KEY, u INT UNIQUE);
INSERT INTO t VALUES (1, 10);
```

Start the following statements in session A:

```sql
USE pessimistic_setvar_retry;
SET tidb_txn_mode = 'pessimistic';
SET tidb_pessimistic_txn_fair_locking = OFF;
SET @x = 0;
BEGIN;

UPDATE t
SET u = SLEEP((@x := @x + 1) + 7) * 0 + @x
WHERE id = 1;

SELECT @x;
COMMIT;
SELECT id, u FROM t ORDER BY id;
```

While session A is executing the `UPDATE`, run in session B:

```sql
USE pessimistic_setvar_retry;
INSERT INTO t VALUES (2, 1);
```

The `SLEEP` opens a deterministic window after session A has evaluated `@x := @x + 1` and before
its pessimistic lock phase finishes. Session B commits the unique key `u=1` in that window.

This reproduced without failpoints on a real TiKV cluster. Session A returned:

```text
@x
2

id u
1 2
2 1
```

### 2. What did you expect to see?

Transparent pessimistic statement retry should preserve the semantics of the submitted statement.
The retry should evaluate the statement from its entry state, where `@x=0`, and compute `u=1`
again. Because session B has committed `u=1`, session A should return a duplicate-key error and row
1 should remain `(1,10)`.

Expected final rows after handling the failed statement:

```text
1 10
2 1
```

### 3. What did you see instead?

Session A returned success. The first failed attempt had already changed `@x` from 0 to 1. The
automatic retry started with that mutated session state, changed `@x` to 2, computed `u=2`, and
committed `(1,2)`. The expected duplicate-key error was silently suppressed.

`SETVAR` writes directly to `SessionVars.UserVars` during expression evaluation. Pessimistic DML
can then receive a retryable write conflict from `LockKeys`. `handlePessimisticLockError` rebuilds
the executor and calls `StmtRollback`, but statement rollback cleans transaction statement state
without restoring user variables. The rebuilt executor therefore consumes a side effect from the
failed attempt.

A deterministic unit-test matrix also isolated the boundary:

```text
no conflict: v=1, @x=1
write conflict before SETVAR: v=1, @x=1
write conflict after SETVAR: v=2, @x=2
late conflict with idempotent := 7: v=7, @x=7
```

Snapshotting user variables at statement entry and restoring them before an accepted retry made the
post-evaluation and natural unique-key race tests pass. A production fix could instead track only
the variables touched by the failed attempt, or decline transparent retry for statements with
non-transactional expression side effects.

### 4. What is your TiDB version?

- Current master: `13282a8bd06bd33324a4dbfd3c1c03685f3cd9aa`
- SQL-only reproduction with real TiKV:
`8.0.11-TiDB-v9.0.0-beta.2.pre-1895-g5c9198e948`

Contributor guide

Open the contributing guide

Research direction

Trace handlePessimisticLockError, StmtRollback, and SessionVars.UserVars to understand how a rebuilt executor handles SETVAR side effects after LockKeys conflicts. Use the deterministic unit-test matrix and the natural unique-key race reproduction; done means an accepted retry starts from the statement-entry user-variable state and the race returns a duplicate-key error without changing row 1.

Written by the indexing model from the issue text.

Assessment

Tech stack
go, sql
Domain
databases, distributed-systems
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.