pingcap / pingcap/tidb

[br] UnmarshalDir may panic after a cloud storage WalkDir error

Open
#70,807 3 comments 0 reactions 0 assignees View on GitHub
affects-25.10 affects-26.3 affects-8.5 component/br found-by-ai severity/major 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)

1. Run BR v8.5.8 with a cloud object-storage backend and start a restore operation that reads migration or sub-compaction metadata through `storage.UnmarshalDir`.
2. During the metadata listing, let the object-storage `WalkDir` operation return a transient error after at least one metadata-read worker has already been scheduled. Examples include an object-storage 5xx response, a network timeout, a failed pagination request, or credentials being revoked during the listing.
3. Continue consuming the metadata iterator and observe the BR process.

The v8.5.8 runtime path can be reproduced with a storage test double that schedules one worker, returns a `WalkDir` error, and delays the worker's `ReadFile`/unmarshal operation until after `WalkDir` returns.

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

BR should wait for all scheduled metadata workers, return the original object-storage error, and exit the restore operation cleanly. It must not panic while reporting an external-storage failure.

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

The `UnmarshalDir` reader closes its result channel when `WalkDir` returns an error, without first waiting for the workers that were already submitted. A worker can subsequently send a parsed metadata value to the closed channel, producing:

```text
panic: send on closed channel
```

This was directly reproduced in the v8.5.8 runtime path. The trigger is not limited to a synthetic invalid metadata format: a transient failure while listing cloud objects can provide the same interleaving. Depending on the call path, this can terminate BR during restore/PiTR metadata loading and leave the operation incomplete.

Relevant code: `br/pkg/storage/helper.go`, `UnmarshalDir`.

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

TiDB v8.5.8, commit `8b857efa20363d50a8fa2ea7dd9809a85a61b115`.

Related change: [PR #64850](https://github.com/pingcap/tidb/pull/64850), which introduced the current BR compact-log-restore storage helper implementation.

Contributor guide

Open the contributing guide

Research direction

Start in br/pkg/storage/helper.go at UnmarshalDir and reproduce the reported interleaving with a storage test double that schedules a worker, returns a WalkDir error, and delays ReadFile/unmarshal. Verify the implementation waits for scheduled workers before closing or returning, preserves the original object-storage error, and completes without a send-on-closed-channel panic.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
databases
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
72/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.