br: Fail backup if any snapshots failed

[michaelmdeng]

What problem does this PR solve?

During EBS snapshot backup, if any snapshots fail, the backup will be stuck indefinitely checking the snapshots that failed. Since we only run a single backup at a time, this also blocks subsequent backups.

Issue Number: close #53037

Problem Summary: Backup stuck when EBS snapshot failed

What changed and how does it work?

Update snapshot backup process to fail if it detects snapshots in failed state. We observe rare snapshot failures due to underlying EBS hardware issues.

Check List

Tests

• Unit test
• Integration test
• Manual test (add detailed scripts or steps below)
• No need to test

  • I checked and no code files have been changed.

Side effects

• Performance regression: Consumes more CPU
• Performance regression: Consumes more Memory
• Breaking backward compatibility

Documentation

• Affects user behaviors
• Contains syntax changes
• Contains variable changes
• Contains experimental features
• Changes MySQL compatibility

Release note

Please refer to Release Notes Language Style Guide to write a quality release note.

None

[sre-bot]

All committers have signed the CLA.

[ti-chi-bot]

Welcome @michaelmdeng!

It looks like this is your first PR to pingcap/tidb 🎉.

I'm the bot to help you request reviewers, add labels and more, See available commands.

We want to make sure your contribution gets all the attention it needs!



Thank you, and welcome to pingcap/tidb. 😃

[ti-chi-bot]

Hi @michaelmdeng. Thanks for your PR.

I'm waiting for a pingcap member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[nkg-]

Looks great Backup metadata is still generated, and contains the snapshot ids. So cleanup should delete the snapshots.

tidb/br/pkg/task/backup_ebs.go

Lines 228 to 255 in ede3585

	snapIDMap, volAZs, err = ec2Session.CreateSnapshots(backupInfo)
	if err != nil {
	// TODO maybe we should consider remove snapshots already exists in a failure
	return errors.Trace(err)
	}
	// Step.3 save backup meta file to s3.
	// NOTE: maybe define the meta file in kvproto in the future.
	// but for now json is enough.
	backupInfo.SetClusterVersion(normalizedVer.String())
	backupInfo.SetFullBackupType(string(cfg.FullBackupType))
	backupInfo.SetResolvedTS(resolvedTs)
	backupInfo.SetSnapshotIDs(snapIDMap)
	backupInfo.SetVolumeAZs(volAZs)
	err = saveMetaFile(c, backupInfo, client.GetStorage())
	if err != nil {
	return err
	}
	if !cfg.SkipPauseGCAndScheduler {
	log.Info("snapshot started, restore schedule")
	if restoreE := goBackupToNormal(ctx); restoreE != nil {
	log.Warn("failed to restore removed schedulers, you may need to restore them manually", zap.Error(restoreE))
	}
	}
	log.Info("wait async snapshots finish")
	totalSize, err = ec2Session.WaitSnapshotsCreated(snapIDMap, progress)

[nkg-]

I wonder though, how much time does it take for AWS to mark the snapshots as failed.

[ti-chi-bot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: BornChanger, YuJuncen

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• br/OWNERS [BornChanger,YuJuncen]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[ti-chi-bot]

[LGTM Timeline notifier]

Timeline:

• 2024-05-08 09:26:00.62802725 +0000 UTC m=+1040514.385162823: ☑️ agreed by YuJuncen.
• 2024-05-08 09:39:45.780055231 +0000 UTC m=+1041339.537190803: ☑️ agreed by BornChanger.

[codecov]

Codecov Report

Attention: Patch coverage is 0% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 74.3287%. Comparing base (b421b72) to head (1447e43).
Report is 23 commits behind head on master.

Additional details and impacted files

@@               Coverage Diff                @@
##             master     #53038        +/-   ##
================================================
+ Coverage   72.0329%   74.3287%   +2.2957%     
================================================
  Files          1499       1510        +11     
  Lines        431222     443332     +12110     
================================================
+ Hits         310622     329523     +18901     
+ Misses       101291      93586      -7705     
- Partials      19309      20223       +914     

Flag	Coverage Δ
integration	48.0147% <0.0000%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
dumpling	53.9957% <ø> (ø)
parser	∅ <ø> (∅)
br	50.6534% <0.0000%> (+12.3075%)	⬆️

[tiprow]

@michaelmdeng: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
fast_test_tiprow	b15b738	link	true	/test fast_test_tiprow

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

[nkg-]

@BornChanger : Is the test failure relevant.

[michaelmdeng]

@BornChanger : Is the test failure relevant.

Can I get a retest? updated to fix lint errors

[nkg-]

Please cherrypick to release-6.5 and release-7.5

[ti-chi-bot]

In response to a cherrypick label: new pull request created to branch release-7.5: #53112.

[ti-chi-bot]

In response to a cherrypick label: new pull request created to branch release-6.5: #53113.

[ti-chi-bot]

In response to a cherrypick label: new pull request created to branch release-8.1: #55009.