pingcap / pingcap/tidb

[txn] Pessimistic retry can advance seeded RAND and turn duplicate-key failure into success

Open
#69,823 0 comments 0 reactions 0 assignees View on GitHub
component/expression 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 (Required)

Metadata locking remains enabled throughout this reproduction.

Create two identical tables:

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

CREATE TABLE retry_key(id INT PRIMARY KEY, u INT UNIQUE);
CREATE TABLE control_key LIKE retry_key;
INSERT INTO retry_key VALUES (1, 10);
INSERT INTO control_key VALUES (1, 10);
SELECT @@global.tidb_enable_metadata_lock;
-- 1
```

In session A, start a pessimistic READ COMMITTED transaction. The constant seed makes the `RAND`
sequence deterministic. `SLEEP` only makes the concurrent schedule easy to reproduce:

```sql
USE pessimistic_seeded_rand_retry;
SET tidb_txn_mode = 'pessimistic';
SET transaction_isolation = 'READ-COMMITTED';
BEGIN;

UPDATE retry_key
SET u = IF(RAND(12345) < 0.8, 1, 2) + SLEEP(20) * 0
WHERE id = 1;
```

While session A is sleeping, run and commit the following in session B:

```sql
USE pessimistic_seeded_rand_retry;
SET tidb_txn_mode = 'pessimistic';
BEGIN;
INSERT INTO retry_key VALUES (2, 1);
INSERT INTO control_key VALUES (2, 1);
COMMIT;
```

After the `UPDATE` in session A returns, commit and inspect the table:

```sql
COMMIT;
SELECT * FROM retry_key ORDER BY id;
```

Run the same seeded expression once from the same final database state:

```sql
UPDATE control_key
SET u = IF(RAND(12345) < 0.8, 1, 2)
WHERE id = 1;
SELECT * FROM control_key ORDER BY id;
```

The following test provides a deterministic in-repository reproduction. Add it under
`pkg/executor/test/writetest`:

```go
package writetest

import (
"testing"
"time"

"github.com/pingcap/tidb/pkg/testkit"
"github.com/stretchr/testify/require"
)

func TestPessimisticRetryDoesNotChangeSeededRandUniqueKey(t *testing.T) {
store := testkit.CreateMockStore(t)
owner := testkit.NewTestKit(t, store)
competitor := testkit.NewTestKit(t, store)
owner.MustExec("use test")
competitor.MustExec("use test")
owner.MustQuery("show global variables like 'tidb_enable_metadata_lock'").
Check(testkit.Rows("tidb_enable_metadata_lock ON"))

owner.MustExec("create table rand_key_retry (id int primary key, u int unique)")
owner.MustExec("create table rand_key_control like rand_key_retry")
owner.MustExec("insert into rand_key_retry values (1, 10)")
owner.MustExec("insert into rand_key_control values (1, 10)")
owner.MustExec("set transaction_isolation = 'READ-COMMITTED'")
competitor.MustExec("set transaction_isolation = 'READ-COMMITTED'")

owner.MustExec("begin pessimistic")
errCh := make(chan error, 1)
go func() {
_, err := owner.Exec(`update rand_key_retry
set u = if(rand(12345) < 0.8, 1, 2) + sleep(0.8) * 0
where id = 1`)
errCh <- err
}()

time.Sleep(150 * time.Millisecond)
competitor.MustExec("begin pessimistic")
competitor.MustExec("insert into rand_key_retry values (2, 1)")
competitor.MustExec("insert into rand_key_control values (2, 1)")
competitor.MustExec("commit")

ownerErr := <-errCh
if ownerErr == nil {
owner.MustExec("commit")
} else {
owner.MustExec("rollback")
}
retryRows := owner.MustQuery("select * from rand_key_retry order by id").Rows()

_, controlErr := owner.Exec(`update rand_key_control
set u = if(rand(12345) < 0.8, 1, 2)
where id = 1`)
require.Error(t, controlErr)
controlRows := owner.MustQuery("select * from rand_key_control order by id").Rows()

require.Error(t, ownerErr,
"hidden retry must not advance constant-seed RAND past the conflicting first value")
require.Equal(t, controlRows, retryRows)
}
```

Run it with:

```bash
go test -tags=intest ./pkg/executor/test/writetest \
-run TestPessimisticRetryDoesNotChangeSeededRandUniqueKey -count=1
```

### 2. What did you expect to see? (Required)

`RAND(12345)` starts from a deterministic sequence. A successful transparent retry should not
advance that sequence past values consumed only by a hidden failed attempt.

For a fresh execution, the first value is below `0.8`, so the submitted expression selects `u=1`.
After session B commits `u=1`, the `UPDATE` should return the duplicate-key conflict and leave row 1
unchanged. At minimum, TiDB should expose the conflict instead of silently committing a result from
the next RNG position.

### 3. What did you see instead (Required)

Session A reports success and commits the key selected by the second seeded value:

```text
retry UPDATE: success, affected rows = 1, Exec_retry_count = 1
retry rows: (1, 2), (2, 1)
```

The direct control starts from the same final database state but returns:

```text
ERROR 1062 (23000): Duplicate entry '1' for key 'control_key.u'
control rows: (1, 10), (2, 1)
```

An independent value control shows the same sequence-position drift:

```text
single execution: CAST(RAND(12345) * 1000000000 AS UNSIGNED) = 665703432
hidden retry: committed value = 912825259
```

On real TiKV, the slow log records `Exec_retry_count=1`, `Exec_retry_time=20.001772209`,
`Query_time=40.00417287`, `Succ=1`, and `IsExplicitTxn=1`. The roughly 40-second query time also
shows that the 20-second expression was evaluated twice.

For a constant argument, `randFunctionClass.getFunction` creates one mutable `MysqlRng` before
execution. `builtinRandSig.evalReal` advances it through `Gen()`, while `builtinRandSig.Clone`
shallow-copies the same pointer. The pessimistic retry path rebuilds the executor from the already
advanced plan/expression state, so the successful attempt consumes the second seeded value.

As a counterfactual, recording that a mutable `RAND` generator was consumed and declining the
transparent retry afterward makes the same test return the original conflict and preserve both
rows. Merely deep-copying the RNG in `Clone` after the first attempt is too late and remains RED;
the state must be restored from statement entry or the replay must be declined.

### 4. What is your TiDB version? (Required)

- Real-TiKV SQL reproduction: `8.0.11-TiDB-v9.0.0-beta.2.pre-1895-g5c9198e948`
- TiDB commit: `5c9198e9484db852b8477ce0014e0422ff9ec6a9`
- Current-source local reproduction: `b8d04e17a2ca61eee1220c5ce2d641a376f75e9b`
- Global `tidb_enable_metadata_lock`: `ON`

Contributor guide

Open the contributing guide

Research direction

Run the named TestPessimisticRetryDoesNotChangeSeededRandUniqueKey in pkg/executor/test/writetest with the provided go test command. Then trace randFunctionClass.getFunction, builtinRandSig.evalReal, builtinRandSig.Clone, and the pessimistic retry path to determine how statement-entry RAND state is handled. Done means the test preserves the duplicate-key conflict and matches the control rows without advancing the seeded sequence.

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
52/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.