pingcap / pingcap/tidb

br: meta-KV restore pipeline has pre-existing per-CF dedup hazard in RawKVBatchClient

Open
#68,539 3 comments 0 reactions 0 assignees View on GitHub
component/br contribution 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

`RawKVBatchClient.Put` deduplicates multiple MVCC versions of the same logical meta key by
keeping only the highest-TS entry within each column-family batch. Because `DefaultCF` and
`WriteCF` are restored through **independent** `RawKVBatchClient` instances
([`client.go:1211,1217`](https://github.com/pingcap/tidb/blob/23f315bf64/br/pkg/restore/log_client/client.go#L1211-L1217)),
this per-CF TS-based collapse can break cross-CF references in two ways.

These cases were raised by @Leavrth during review of #67268.
They are **not** introduced by that PR — the same collapse already happens in
`RawKVBatchClient.Put` at the batch-write layer, independently of any upstream file-level
deduplication.

---

### Case 1 — Rollback as max-TS WriteCF entry: committed DefaultCF value is dropped

DefaultCF: k1@100 (prewrite-1, committed), k1@120 (prewrite-2, rolled back)
WriteCF: k1@110:start_ts=100 → Put, k1@130:start_ts=120 → Rollback

`RawKVBatchClient` processes DefaultCF independently. Highest-TS wins: `k1@120` survives,
`k1@100` is dropped. After restore:

- DefaultCF contains the **rolled-back** prewrite value (`k1@120`)
- The committed value at `k1@100` — referenced by the WriteCF Put at `commit_ts=110` — is gone

---

### Case 2 — Cross-CF reference broken by independent per-CF dedup

Edit: removed case 2 example, as it does violate a percolator model used by TiDB. Writes in case 2 could not be materialized in that way.

---

### Practical impact

For these cases to be reached in a real PiTR stream, the same meta key would need concurrent
uncommitted prewrites — a scenario that TiDB's 2PC lock acquisition serializes against for all
production meta keys (DDL/auto-ID/sequence writes). This is why neither case has surfaced in
practice.

However, the hazard is structurally present in the restore pipeline and could surface if:

- future TiDB changes introduce pipelined or parallel meta-key writers, or
- a corrupt or unusual log stream is replayed

### 1. Minimal reproduce step (Required)

[`br/pkg/restore/internal/rawkv/rawkv_client_test.go`](https://github.com/vldmit/tidb/tree/pitr-meta-kv-dedup-concern/br/pkg/restore/internal/rawkv/rawkv_client_test.go)
on branch `pitr-meta-kv-dedup-concern` contains two tests —
`TestRawKVBatchClientDedupConcernCase1` and `TestRawKVBatchClientDedupConcernCase2` —
that verify and document both failure modes. Both tests pass (they demonstrate existing behavior).

```bash
go test -v -run 'TestRawKVBatchClientDedupConcernCase' \
./br/pkg/restore/internal/rawkv/...
```

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

No consistency issues

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

Data loss

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

master branch

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.