BR: 2 passes pre-split causing restore performance regression
- Dominant language
- Go
- Stars
- 40.5k
- Forks
- 6.2k
- PR merge metrics
- PR metrics pending
Description
## Bug Report
Please answer these questions before submitting your issue. Thanks!
### 1. Minimal reproduce step (Required)
Run PiTR restore on the same test topology and similar data volume with the following two TiDB commits:
- Fast commit: `81ec977cb8bf97e0c9805dfc0be8ffcbd4b0bbeb`
- Slow commit: `e70762ebf8bc466aa251e893799a7d8e42b89a69`
Clinic links:
- Fast restore: https://clinic.pingcap.com.cn/portal/#/orgs/33/clusters/7645071090219078379?from=1780006732&to=1780013527
- Slow restore: https://clinic.pingcap.com.cn/portal/#/orgs/33/clusters/7647273150717798736?from=1780519436&to=1780534238
Both clusters have the same basic topology:
- 1 TiDB
- 6 TiKV
- 1 PD
- TiDB version: nightly/master build
The main BR-related change between the two commits is:
```text
72e854aac662020f67e280cc04a0964f066c7838 br: two-pass pre-split for PiTR log restore (#67310)
```
The slow commit adds a PiTR restore pre-split path:
```go
preSplitDone, preSplitErr := client.PreSplitRegions(ctx, rewriteRules, splitSize, splitKeys)
```
When `preSplitDone == true`, the log restore path skips the original per-batch split wrapper and only keeps checkpoint filtering:
```go
logFilesIterWithSplit, err = client.WrapLogFilesIterWithCheckpointFilter(...)
```
instead of the old path:
```go
logFilesIterWithSplit, err = client.WrapLogFilesIterWithSplitHelper(...)
```
### 2. What did you expect to see? (Required)
PiTR restore performance should not regress after introducing two-pass pre-split.
After pre-split completes, BR should still feed TiKV import/download continuously. TiKV import download throughput and apply task rate should stay close to the previous commit for the same topology and similar data volume.
Expected behavior:
- TiKV import download should remain continuously active.
- `tikv_import_download_bytes_sum` throughput should be close to the fast commit.
- `tikv_import_download_bytes_count` should not drop significantly.
- Overall restore duration should not increase by around 2x when TiKV download/read latency itself is unchanged.
### 3. What did you see instead (Required)
The restore on the slow commit took much longer:
```text
Fast restore duration: ~113 min
Slow restore duration: ~247 min
Regression: ~2.18x slower
```
The total restored/write volume is similar, but the slow restore has much lower effective TiKV import download throughput.
Key metrics:
```text
tikv_import_download_bytes_sum avg:
fast: ~357 MB/s
slow: ~206 MB/s
slow/fast: ~58%
tikv_import_download_bytes_count avg:
fast: ~1432 ops/s
slow: ~734 ops/s
slow/fast: ~51%
download read avg:
fast: ~6.47s
slow: ~6.53s
download read p95:
fast: ~15.5s
slow: ~17.6s
download >100MB/s active ratio:
fast: ~62%
slow: ~31%
tikv_import_ingest_bytes_sum avg:
fast: ~752 MB/s
slow: ~344 MB/s
slow/fast: ~46%
```
TiKV does not appear saturated in the slow case:
```text
TiKV CPU avg:
fast: ~25.7 cores
slow: ~17.4 cores
apply log p99:
fast: ~94 ms
slow: ~8 ms
request wait p99:
fast: ~1.9 ms
slow: ~0.07 ms
```
This suggests the bottleneck is not TiKV download latency, TiKV CPU, raft apply, or disk write latency. The slow case looks like TiKV import/download is underfed: each download is not much slower, but BR submits far fewer download/apply tasks and leaves TiKV idle more often.
Possible cause:
The new two-pass pre-split path changes the log restore file iterator path. When pre-split succeeds, BR skips `WrapLogFilesIterWithSplitHelper` and uses `WrapLogFilesIterWithCheckpointFilter`. The skipped wrapper is not only doing split; it is also part of the streaming restore pipeline before `RestoreKVFiles`. Removing it may change the cadence and region/batch shape seen by `ApplyKVFilesWithBatchMethod`, causing fewer `ApplyKVFile` requests and lower TiKV import download concurrency.
There is also a potential amplification factor in `PreSplitRegions`: it currently merges all DML files into the split helper, including files <= 1MB. The old per-batch `LogSplitStrategy.Accumulate` skipped files <= `SplitFileThresholdDefault` to avoid excessive split/scatter and BTreeMap overhead. The new pre-split path does not apply that threshold.
### 4. What is your TiDB version? (Required)
Fast commit:
```text
81ec977cb8bf97e0c9805dfc0be8ffcbd4b0bbeb
```
Slow commit:
```text
e70762ebf8bc466aa251e893799a7d8e42b89a69
```
Both are nightly/master builds according to Clinic metadata.
Contributor guide
Assessment
This issue has not been assessed yet.