[br] KS3 CopyFrom swallows ordinary CopyObject errors
- Dominant language
- Go
- Stars
- 40.5k
- Forks
- 6.2k
- PR merge metrics
- PR metrics pending
Description
## Bug Report
### 1. Minimal reproduce step (Required)
1. Configure BR to use KS3 (or a KS3-compatible endpoint) for the destination log-backup storage and use another KS3 storage as the source.
2. Start a PiTR/log-backup operation that needs to copy an SST object with `CopyObject`.
3. Make the KS3 `CopyObject` request fail with a normal service error, for example `AccessDenied`, `NoSuchKey`, `SlowDown`, a 5xx response, or a network error. A minimal protocol-level reproducer is an endpoint returning an XML error such as:
```xml
AccessDenied
access denied
```
4. Check the return value of the KS3 storage `CopyFrom` operation and the subsequent metadata recorded for the copied SST.
### 2. What did you expect to see? (Required)
`CopyFrom` should return the KS3 error for every `CopyObject` failure except the explicitly handled already-exists case. The caller should not record the SST as copied when the destination object was not created.
### 3. What did you see instead (Required)
In v8.5.8, `KS3Storage.CopyFrom` returns `nil` for an `awserr.Error` that is not recognized as `ObjectAlreadyExists`. The error is therefore treated as success by the PiTR collector (`putSST`), which records the destination SST in metadata even though the copy may have failed.
The behavior was directly reproduced against the real KS3 SDK code path with an endpoint returning `AccessDenied`: `CopyFrom` returned `nil`.
In a real deployment, this can happen when credentials or bucket policy changes, the source object is missing, KS3 throttles the request, KS3 returns a 5xx, or the network fails. The immediate restore/backup step can appear successful and the failure may only be discovered later when the recorded SST is read, making diagnosis and recovery harder.
Relevant code: `br/pkg/storage/ks3.go`, `KS3Storage.CopyFrom`.
### 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 KS3 storage implementation. The PR was a cherry-pick of the compact-log-backup work tracked by [Issue #64814](https://github.com/pingcap/tidb/issues/64814).
Contributor guide
Research direction
Start in br/pkg/storage/ks3.go at KS3Storage.CopyFrom, then follow its caller in the PiTR collector at putSST. Exercise CopyObject failures such as AccessDenied and verify that only the explicitly handled already-exists case is treated as success and failed copies are not recorded in metadata.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- databases
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 78/100