pingcap / pingcap/tidb

[br] BR snapshot restore can report success after losing its future PiTR data

Open
#70,133 0 comments 0 reactions 0 assignees View on GitHub
component/br found-by-ai may-affects-7.5 may-affects-8.1 may-affects-8.5 severity/critical 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)

This reproduces on TiDB master `05b396fb6636f73b3bc06b09107cf43f2c725c35`
with one TiDB, one PD, real TiKV, MDL enabled, and the default log-backup flush
interval.

The injected error represents a terminal failure of the final
`extbackupmeta` overwrite. It changes only that storage result:

```diff
diff --git a/br/pkg/restore/snap_client/pitr_collector.go b/br/pkg/restore/snap_client/pitr_collector.go
--- a/br/pkg/restore/snap_client/pitr_collector.go
+++ b/br/pkg/restore/snap_client/pitr_collector.go
@@
import (
"context"
"fmt"
+ "os"
@@
c.ingestedSSTMeta.msg.AsIfTs = ts
+ if os.Getenv("BR_TEST_FAIL_FINAL_PITR_META") == "1" {
+ return 0, errors.New("injected final PiTR metadata commit failure")
+ }
return ts, c.persistExtraBackupMeta(ctx)
}
```

Build BR, point `BR` and `PD` at the binary and a running test cluster, then
run:

```bash
set -euo pipefail

BR=${BR:-./bin/br}
PD=${PD:-127.0.0.1:2379}
MYSQL=${MYSQL:-mysql}
ROOT=$(mktemp -d /tmp/br-pitr-final-meta.XXXXXX)
TASK=br-pitr-final-meta-$RANDOM
SRC=br_pitr_imported
BASE=br_pitr_baseline

sql() {
"$MYSQL" -h 127.0.0.1 -P 4000 -u root "$@"
}

stop_log() {
"$BR" log stop --task-name "$TASK" --pd "$PD" \
--storage "local://$ROOT/log" --status-addr '' \
--check-requirements=false >/dev/null 2>&1 || true
}
trap stop_log EXIT

sql -e "
DROP DATABASE IF EXISTS $SRC;
DROP DATABASE IF EXISTS $BASE;
CREATE DATABASE $SRC;
CREATE TABLE $SRC.t (
id BIGINT PRIMARY KEY,
v VARCHAR(100) NOT NULL,
payload VARBINARY(200) NOT NULL
);
INSERT INTO $SRC.t
SELECT ROW_NUMBER() OVER (),
CONCAT('row-', ROW_NUMBER() OVER ()),
RPAD(UNHEX(SHA2(CONCAT('payload-', ROW_NUMBER() OVER ()), 256)), 128, 0x5a)
FROM information_schema.columns
LIMIT 5000;"

"$BR" backup db --db "$SRC" --pd "$PD" \
--storage "local://$ROOT/source" --status-addr '' \
--check-requirements=false

sql -e "
DROP DATABASE $SRC;
CREATE DATABASE $BASE;
CREATE TABLE $BASE.marker (id BIGINT PRIMARY KEY, phase VARCHAR(40) NOT NULL);
INSERT INTO $BASE.marker VALUES (1, 'before-snapshot');"

"$BR" log start --task-name "$TASK" --pd "$PD" \
--storage "local://$ROOT/log" --status-addr '' \
--check-requirements=false

"$BR" backup full --pd "$PD" --storage "local://$ROOT/full" \
--filter "$BASE.*" --filter "$SRC.*" --status-addr '' \
--check-requirements=false

BR_TEST_FAIL_FINAL_PITR_META=1 "$BR" restore db --db "$SRC" --pd "$PD" \
--storage "local://$ROOT/source" --status-addr '' \
--check-requirements=false

sql -e "
UPDATE $BASE.marker SET phase='after-import';
INSERT INTO $SRC.t VALUES
(6000, 'post-import-log-row',
RPAD(UNHEX(SHA2('post-import-log-row', 256)), 128, 0x5a));
ADMIN CHECK TABLE $SRC.t;
SELECT COUNT(*), SUM(id) FROM $SRC.t;"

# Wait for the default TiKV log-backup flush interval.
sleep 190
stop_log
trap - EXIT

sql -e "DROP DATABASE $SRC; DROP DATABASE $BASE;"

"$BR" restore point --pd "$PD" \
--storage "local://$ROOT/log" \
--full-backup-storage "local://$ROOT/full" \
--filter "$BASE.*" --filter "$SRC.*" --status-addr '' \
--check-requirements=false

sql --table -e "
SELECT * FROM $BASE.marker;
SELECT COUNT(*) AS rows_after_pitr, SUM(id) AS id_sum_after_pitr FROM $SRC.t;
SELECT id,v FROM $SRC.t ORDER BY id;
ADMIN CHECK TABLE $SRC.t;"
```

Observed first restore:

```text
failed on close snap importer:
failed to commit pitrCollector:
injected final PiTR metadata commit failure

DataBase Restore success summary
```

The table contains all 5000 imported rows after this command. After the later
point restore, BR again exits successfully, the ordinary log row with id 6000
and the baseline marker survive, but all 5000 snapshot-restored rows are gone:

```text
before disaster: rows=5001, id_sum=12508500
after PiTR: rows=1, id_sum=6000
remaining row: 6000, post-import-log-row
```

With only the injected storage error removed, the same point restore contains
all 5001 rows.

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

If the final PiTR metadata publication fails, snapshot restore should return a
nonzero error. It must not advertise success until the metadata that makes the
imported SSTs visible to future point-in-time recovery is durable.

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

The final metadata error is logged but discarded. Snapshot restore exits 0.
A later point restore also exits 0 while silently omitting every SST row
imported by the earlier successful restore.

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

TiDB master `05b396fb6636f73b3bc06b09107cf43f2c725c35`.

Likely root cause and fix direction

`pitrCollector.close` returns the final metadata commit error.
`SnapFileImporter.Close` logs callback errors but returns only the gRPC-client
close error. `SnapClient.Close` logs the restorer close error and has no return
value, while `runSnapshotRestore` defers that close and publishes success first.

The close path should be an explicit terminal join before the success summary:
propagate the collector error through `SnapFileImporter.Close` and
`SnapClient.Close`, and keep the final metadata write idempotent so retry can
complete publication safely.

Contributor guide

Open the contributing guide

Research direction

Start with br/pkg/restore/snap_client/pitr_collector.go and trace pitrCollector.close through SnapFileImporter.Close, SnapClient.Close, and the deferred close in runSnapshotRestore. Run the supplied reproduction with BR_TEST_FAIL_FINAL_PITR_META=1, then verify that a final metadata failure produces a nonzero restore result and that a later point restore retains the imported SST rows.

Written by the indexing model from the issue text.

Assessment

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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.