pingcap / pingcap/tidb-operator
[v1] Backup: failed snapshot-backup gc prematurely
Nobody has claimed this yet.
- Dominant language
- Go
- Stars
- 1.3k
- Forks
- 540
- Avg merge
- 3d 2h
- Merged PRs (30d)
- 18
Description
just like: https://github.com/pingcap/tidb-operator/issues/5517
Bug Report
What version of Kubernetes are you using?
What version of TiDB Operator are you using?
1.4.7 , it also contains in master branch
What storage classes exist in the Kubernetes cluster and what are used for PD/TiKV pods?
What's the status of the TiDB cluster pods?
What did you do?
- Run snapshot backup via backup schedule, with maxReservedTime set
- Some backup failed
What did you expect to see?
Newly failed backup can be retained for a while
What did you see instead?
The failed backup was GC prematurely.
If backup in invalid/completed/failed phase, sort by creationTimestamp , the earlier the earlier, the higher the front
https://github.com/pingcap/tidb-operator/blob/fe85d868be91529c148e1f55c00349753b0deac9/pkg/backup/backupschedule/backup_schedule_manager.go#L665-L668
https://github.com/pingcap/tidb-operator/blob/fe85d868be91529c148e1f55c00349753b0deac9/pkg/backup/backupschedule/backup_schedule_manager.go#L766-L789
If logbackup doesn't exist, into func 'calculateExpiredBackups'
https://github.com/pingcap/tidb-operator/blob/fe85d868be91529c148e1f55c00349753b0deac9/pkg/backup/backupschedule/backup_schedule_manager.go#L682-L687
Traverse from the beginning to the end until commitTs is later than the timeout
But failed-backup doesn't save commitTs, func 'ParseTSString' will return 0
https://github.com/pingcap/tidb-operator/blob/fe85d868be91529c148e1f55c00349753b0deac9/pkg/backup/backupschedule/backup_schedule_manager.go#L863-L876
https://github.com/pingcap/tidb-operator/blob/fe85d868be91529c148e1f55c00349753b0deac9/pkg/apis/util/config/config.go#L396-L412
There are two case about it
Case1:
Backups in order: failed1, complete2, failed3 (all in reservedTime)
Retained: complete2, failed3Case2:
Backups in order: failed1 (in reservedTime)
Retained: (none, deleted directly)
This behavior is not expected.
Failed backups should also be retained as expected.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with pkg/backup/backupschedule/backup_schedule_manager.go, especially calculateExpiredBackups, the backup ordering logic, and ParseTSString; then inspect the related configuration code in pkg/apis/util/config/config.go. Trace the two failed-backup cases described in the issue and verify that failed backups within maxReservedTime are retained rather than deleted prematurely.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go, kubernetes
- Domain
- databases, devops
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 48/100