pingcap / pingcap/tidb

[br] Restore can outlive its registration lease and make later PiTR lose imported rows

Open
#70,155 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/major type/bug
Dominant language
Go
Stars
40.5k
Forks
6.2k
PR merge metrics
PR metrics pending

Description

## Bug Report

### Production impact and realistic trigger

This can silently lose all rows imported by a successful BR snapshot restore
from a later point-in-time recovery.

A production-shaped timeline is:

1. A large snapshot restore starts while no log-backup task exists. BR
therefore initializes the restore without a PiTR collector.
2. The restore runs longer than the default three-minute task-registration
lease.
3. The restoring BR process temporarily loses connectivity to **all PD
endpoints visible to that process**, while TiDB, TiKV, and object storage
remain healthy. The BR process remains alive and retries.
4. The leased key under `/tidb/brie/import/restore/` expires.
5. A separate backup-automation process, which still has PD connectivity,
starts log backup and takes its bootstrap/full baseline while the original
restore is stalled before SST ingest.
6. BR-to-PD connectivity recovers. The original restore re-registers and
successfully ingests its historical SSTs.
7. A later disaster recovery uses that full backup plus log backup.

Step 3 can result from a Kubernetes NetworkPolicy rollout, a service-routing
or kube-proxy fault, DNS/LB failure for the PD service, or a host/network path
failure that affects the BR job but not another backup job. It does not require
PD quorum loss: the important condition is that every configured PD route is
temporarily unreachable from the restoring BR process.

This also does not require a TiDB or TiKV crash, multiple TiDB instances, MDL
changes, malformed SQL, or storage corruption.

The failure is delayed and silent:

- the original snapshot restore exits successfully and the current table is
complete;
- ordinary changes after the import are captured by log backup;
- the later point restore also exits successfully;
- only the historical SST rows imported by the original restore are absent.

### Trigger likelihood and scope

The data-loss consequence is severe, but the trigger is uncommon under the
documented BR workflow.

The TiDB documentation recommends running backup and restore tasks one by one
and not backing up tables that are being restored:

https://docs.pingcap.com/tidb/stable/backup-and-restore-overview/

Therefore, a normal manual workflow that waits for snapshot restore to finish
before enabling backup protection does not trigger this bug. Continuous log
backup that was already active when restore started also does not trigger this
root, because the restore initializes its PiTR collector.

The most plausible production shape is a Kubernetes or internal platform that:

1. restores a newly provisioned cluster;
2. enables a `BackupSchedule` or equivalent protection policy based on cluster
readiness without waiting for restore completion; and
3. creates or retries the log-backup task while the restore registration is
absent after a BR-local control-plane outage.

The stock TiDB Operator creates log backup immediately when a
`BackupSchedule.logBackupTemplate` first appears, but its Backup Job has
`backoffLimit=0`. Applying Restore and BackupSchedule together is therefore
not sufficient by itself: the log-backup CR must be created or recreated
during the lease gap, or an external platform must retry failed log-start
operations.

Relevant Operator source:

- immediate log-backup creation:
https://github.com/pingcap/tidb-operator/blob/fe85d868be91529c148e1f55c00349753b0deac9/pkg/backup/backupschedule/backup_schedule_manager.go#L135-L151
- Backup Job does not retry:
https://github.com/pingcap/tidb-operator/blob/fe85d868be91529c148e1f55c00349753b0deac9/pkg/backup/backup/backup_manager.go#L659-L665

This should be treated as a low-frequency, multi-event production trigger with
a critical data-loss consequence, rather than a commonly reachable critical
workflow.

### 1. Minimal reproduce step (Required)

This reproduced on TiDB/BR master
`8bab3c26d76ed27402138d5e26f5fd0ef7ee4898` with one TiDB, three PD, three real
TiKV, MDL enabled, and the default three-minute registration TTL.

The deterministic test used two BR Pods:

- `br-client`: runs the original snapshot restore;
- `br-operator`: starts log backup, takes the full baseline, and later runs
PiTR.

TiDB, TiKV, the S3-compatible object store, and `br-operator` remain available
throughout the network fault.

#### Prepare the snapshot

Create and back up a table with enough data to make SST ingest visible:

```sql
DROP DATABASE IF EXISTS br_lease_repro;
CREATE DATABASE br_lease_repro;
CREATE TABLE br_lease_repro.t (
id BIGINT PRIMARY KEY,
u BIGINT NOT NULL,
payload VARCHAR(160) NOT NULL,
UNIQUE KEY uk_u(u)
);

INSERT INTO br_lease_repro.t
SELECT n, n + 100000, RPAD(CONCAT('payload-', n), 128, 'x')
FROM (
SELECT ROW_NUMBER() OVER () AS n
FROM information_schema.columns a
CROSS JOIN information_schema.columns b
LIMIT 128000
) AS gen;
```

Back it up to storage mounted by `br-client`, then drop the database:

```bash
br backup table \
--db br_lease_repro --table t \
--pd "$PD" \
--storage local:///tmp/backup-big

mysql -h "$TIDB_HOST" -P 4000 -uroot \
-e 'DROP DATABASE br_lease_repro'
```

#### Pause restore before the first SST ingest

For deterministic timing, enable TiKV failpoints and set this one-shot
failpoint on every TiKV:

```bash
for pod in tc-tikv-0 tc-tikv-1 tc-tikv-2; do
kubectl exec sdkserver-0 -- \
curl -X PUT \
--data '1*sleep(600000)->off' \
"http://${pod}.tc-tikv-peer:20180/fail/after_apply_snapshot_ingest_latch_acquired"
done
```

The failpoint only keeps the restore alive before the first SST ingest. It
does not change registration, log-backup admission, SST contents, or PiTR.

Start the original restore in `br-client`:

```bash
br restore table \
--db br_lease_repro --table t \
--pd "$PD" \
--storage local:///tmp/backup-big
```

Wait until `br_lease_repro.t` exists with zero rows. At this point the restore
process is live and its leased key exists.

From `br-operator`, try to start log backup:

```bash
br log start \
--task-name regloss \
--pd "$PD" \
--storage s3://brdata/log-regloss \
--s3.endpoint "$S3_ENDPOINT" \
--check-requirements=false
```

It is correctly rejected:

```text
There are some lightning/restore tasks running:
[ key: /tidb/brie/import/restore/restore-..., ttl: 133s ]
```

#### Expire only the restore registration

Apply a 240-second bidirectional NetworkChaos between `br-client` and all PD
Pods. Replace the namespace and Pod names as needed:

```yaml
apiVersion: chaos-mesh.org/v1alpha1
kind: NetworkChaos
metadata:
name: br-client-to-pd-partition
spec:
action: partition
direction: both
duration: 240s
mode: all
selector:
namespaces: [tidb-test]
pods:
tidb-test: [br-client]
target:
mode: all
selector:
namespaces: [tidb-test]
pods:
tidb-test: [tc-pd-0, tc-pd-1, tc-pd-2]
```

After the default three-minute lease expires, verify both facts:

```text
the original restore process is still alive
/tidb/brie/import/restore contains no key
```

Run the same `br log start` command from the unaffected `br-operator`. It now
exits 0 and creates the log task.

Before releasing the SST ingest, take the full baseline:

```bash
br backup full \
--pd "$PD" \
--storage s3://brdata/full-regloss \
--s3.endpoint "$S3_ENDPOINT" \
--check-requirements=false
```

The baseline contains the table schema and zero rows.

Remove NetworkChaos and let the original restore continue. It re-registers,
then exits 0:

```text
total-ranges=7
ranges-succeed=7
total-kv=256000
```

The current table contains all 128000 imported rows and passes exact checksum,
forced unique-index count, and `ADMIN CHECK TABLE`.

#### Prove ordinary log replay works, then run PiTR

Insert one ordinary SQL witness after the successful restore:

```sql
INSERT INTO br_lease_repro.t
VALUES (200000, 999999, 'log-witness');
```

Wait for the log-backup global checkpoint to pass this commit, record a target
TSO after it, and stop `regloss`. Then drop the database and restore point:

```bash
br restore point \
--pd "$PD" \
--storage s3://brdata/log-regloss \
--full-backup-storage s3://brdata/full-regloss \
--restored-ts "$TARGET_TSO" \
--s3.endpoint "$S3_ENDPOINT" \
--check-requirements=false
```

Observed:

```text
Full Restore success
restore log success
normal log total-kv-count = 20
restore-sst-kv-count = 0
```

The final table contains exactly the ordinary witness:

```text
200000 999999 log-witness
```

All 128000 snapshot-restored rows are absent. `ADMIN CHECK TABLE` still passes
because the whole imported row/index set was omitted.

#### Strict control

The control uses S3 for both the snapshot source and log storage:

1. Start log backup before snapshot restore.
2. Take the same zero-row full baseline.
3. Restore the same 128000-row snapshot from S3.
4. Insert the same witness and run the same later PiTR.

The control reports:

```text
restore-sst-kv-count = 256000
restore-sst-kv-size = 74624000
final rows = 128001
ADMIN CHECK TABLE = PASS
```

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

Loss of the leased restore registration should revoke or pause the restore's
authority to perform later SST ingest until mutual exclusion has been safely
re-established.

Log backup must not start while a still-live restore that initialized without
a PiTR collector can later ingest historical SSTs.

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

The registration lease expires independently of the restore process.
Log-backup admission sees no key and starts successfully. After connectivity
recovers, the original restore recreates its registration and completes SST
ingest without a PiTR collector.

Both the original restore and the later point restore exit successfully, but
the later PiTR silently omits every imported SST row.

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

TiDB/BR master `8bab3c26d76ed27402138d5e26f5fd0ef7ee4898`.

Likely root cause and fix direction

`br/pkg/utils/register.go` gives task registration a default three-minute
lease. `keepaliveLoop` recreates an expired lease and key, but it does not
cancel or pause the protected restore while the key is absent.

`br/pkg/task/stream.go::checkImportTaskRunning` admits log backup based only on
currently visible registration keys.

`br/pkg/task/restore.go::RunRestore` checks for an existing log task once at
startup. `newPiTRColl` returns a disabled collector when no task exists at that
time. That decision is not re-evaluated after registration recovery.

Log backup cannot capture historical `Ingest` SSTs directly, so the
later-started task has no record of the SSTs imported by the original restore.

A fail-closed fix should bind irreversible restore progress to the live lease:

- pause or cancel before further ingest when lease ownership cannot be proven;
- after re-registration, re-check log-backup state before any later ingest;
- either initialize the required collector safely or reject coexistence;
- make log-backup admission fail closed while owner liveness is ambiguous.

Contributor guide

Open the contributing guide

Research direction

Start with br/pkg/utils/register.go and its keepaliveLoop, then read br/pkg/task/stream.go::checkImportTaskRunning and br/pkg/task/restore.go::RunRestore and newPiTRColl. Run the two-BR reproduction described in the issue, including the lease-expiry partition. Done means lease loss cannot permit unsafe later SST ingest or log-backup admission, and a later PiTR retains the imported rows.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
databases, distributed-systems
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
38/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.