backup: implement store based backup to solve the long tail issue #53081

3pointer · 2024-05-07T10:08:59Z

What problem does this PR solve?

Issue Number: ref #52534

Problem Summary:

current backup(range-based backup) mechanism has 2 problems:

When a store shutdown, the retry on this store was too delayed and resulted in a long tail issue.
Too many ranges or a big range backup request is too slow and sometimes exacerbating the first issue.

What changed and how does it work?

This PR is focus on implement backup store-based backup. refer to the storeBackup goroutine and main goroutine.
Also in this PR, I'll remove some special range-based backup code. especially code related to handleFineGrained.

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

Release note

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

None

ti-chi-bot · 2024-05-07T10:09:03Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

3pointer · 2024-05-08T08:26:57Z

/ok-to-test

codecov · 2024-05-08T08:53:51Z

Codecov Report

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

Project coverage is 75.4312%. Comparing base (81da4f8) to head (5870354).
Report is 85 commits behind head on master.

Additional details and impacted files

@@               Coverage Diff                @@
##             master     #53081        +/-   ##
================================================
+ Coverage   72.0214%   75.4312%   +3.4098%     
================================================
  Files          1508       1518        +10     
  Lines        434823     451436     +16613     
================================================
+ Hits         313166     340524     +27358     
+ Misses       102084      89934     -12150     
- Partials      19573      20978      +1405

Flag	Coverage Δ
integration	`51.2828% <73.2057%> (?)`
unit	`71.3103% <10.0478%> (+0.0827%)`	⬆️

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

Components	Coverage Δ
dumpling	`53.9957% <ø> (ø)`
parser	`∅ <ø> (∅)`
br	`57.2166% <78.7081%> (+18.7394%)`	⬆️

3pointer · 2024-05-08T09:51:20Z

/retest

3pointer · 2024-05-09T07:39:41Z

/ok-to-test

3pointer · 2024-05-09T08:54:56Z

/ok-to-test

3pointer · 2024-05-09T09:23:08Z

/ok-to-test

3pointer · 2024-05-10T03:12:45Z

/test pull-br-integration-test

tiprow · 2024-05-10T03:13:06Z

@3pointer: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

/test fast_test_tiprow
/test tidb_parser_test

Use /test all to run all jobs.

In response to this:

/test pull-br-integration-test

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.

3pointer · 2024-05-10T05:48:25Z

/test pull-br-integration-test

br/tests/br_rawkv/run.sh

br/pkg/utils/misc.go

br/pkg/backup/client.go

Leavrth

rest LGTM

Leavrth · 2024-05-14T06:32:31Z

br/pkg/backup/client.go

+			}()
+			cases := make([]reflect.SelectCase, 0)
+			for _, ch := range storeBackupChs {
+				cases = append(cases, reflect.SelectCase{Dir: reflect.SelectRecv, Chan: reflect.ValueOf(ch)})


maybe need a ctx.Done？

YuJuncen

rest lgtm.

YuJuncen · 2024-05-15T06:12:32Z

br/pkg/backup/push.go

+	bCli, err := client.Backup(ctx, &req)
+	failpoint.Inject("reset-retryable-error", func(val failpoint.Value) {
+		switch val.(string) {
+		case "Unavaiable":


Shuold be "Unavailable"?

seems this failpoint never works before..

ti-chi-bot · 2024-05-15T07:45:44Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Leavrth, 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:

~~OWNERS~~ [Leavrth,YuJuncen]
~~br/OWNERS~~ [Leavrth,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 · 2024-05-15T07:45:48Z

[LGTM Timeline notifier]

Timeline:

2024-05-14 06:32:53.020598525 +0000 UTC m=+1548526.777734095: ☑️ agreed by Leavrth.
2024-05-15 07:45:47.300081645 +0000 UTC m=+1639301.057217219: ☑️ agreed by YuJuncen.

3pointer · 2024-05-16T02:21:21Z

/test unit-test

ti-chi-bot · 2024-05-16T02:21:23Z

@3pointer: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

/test build
/test check-dev
/test check-dev2
/test mysql-test
/test pull-br-integration-test
/test pull-integration-ddl-test
/test pull-lightning-integration-test
/test pull-mysql-client-test
/test unit-test

The following commands are available to trigger optional jobs:

/test canary-notify-when-compatibility-sections-changed
/test pingcap/tidb/canary_ghpr_unit_test
/test pull-common-test
/test pull-e2e-test
/test pull-integration-common-test
/test pull-integration-copr-test
/test pull-integration-jdbc-test
/test pull-integration-mysql-test
/test pull-integration-nodejs-test
/test pull-sqllogic-test
/test pull-tiflash-test

Use /test all to run the following jobs that were automatically triggered:

pingcap/tidb/ghpr_build
pingcap/tidb/ghpr_check
pingcap/tidb/ghpr_check2
pingcap/tidb/ghpr_mysql_test
pingcap/tidb/ghpr_unit_test
pingcap/tidb/pull_br_integration_test
pingcap/tidb/pull_integration_ddl_test
pingcap/tidb/pull_mysql_client_test

In response to this:

/test unit-test

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.

tiprow · 2024-05-16T02:21:42Z

@3pointer: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

/test fast_test_tiprow
/test tidb_parser_test

Use /test all to run all jobs.

In response to this:

/test unit-test

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.

3pointer · 2024-05-16T03:10:18Z

/test unit-test

tiprow · 2024-05-16T03:10:40Z

@3pointer: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

/test fast_test_tiprow
/test tidb_parser_test

Use /test all to run all jobs.

In response to this:

/test unit-test

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.

3pointer · 2024-05-16T09:42:12Z

/test unit-test

tiprow · 2024-05-16T09:42:33Z

@3pointer: The specified target(s) for /test were not found.
The following commands are available to trigger required jobs:

/test fast_test_tiprow
/test tidb_parser_test

Use /test all to run all jobs.

In response to this:

/test unit-test

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.

…ngcap#53081) ref pingcap#52534

3pointer added 2 commits May 7, 2024 16:33

fix commit

df971d2

Merge branch 'master' into store_backup

d8dccf9

ti-chi-bot bot added the do-not-merge/needs-tests-checked label May 7, 2024

ti-chi-bot bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. release-note-none Denotes a PR that doesn't merit a release note. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels May 7, 2024

3pointer added 4 commits May 7, 2024 18:15

update

70b2655

remove useless code

70bbd10

use context to control backup flow

1b730f6

udpate some comments

f8ab841

ti-chi-bot bot added the ok-to-test Indicates a PR is ready to be tested. label May 8, 2024

fix typo

0ca511a

add some unit cases

17cb445

ti-chi-bot bot removed the do-not-merge/needs-tests-checked label May 9, 2024

fix build

51c2de1

fix build

ad3be1c

3pointer requested review from YuJuncen and Leavrth May 9, 2024 09:42

update integration tests

87dc635

fix tests

cedc6ac

ti-chi-bot bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 13, 2024

3pointer added 3 commits May 13, 2024 14:22

fix ut

99f6634

fix build

a2ee4e3

fix check

3a21d6d

Leavrth reviewed May 13, 2024

View reviewed changes

br/tests/br_rawkv/run.sh Show resolved Hide resolved

br/pkg/utils/misc.go Show resolved Hide resolved

br/pkg/backup/client.go Show resolved Hide resolved

br/pkg/backup/client.go Outdated Show resolved Hide resolved

Leavrth reviewed May 13, 2024

View reviewed changes

br/pkg/backup/client.go Show resolved Hide resolved

br/pkg/backup/client.go Show resolved Hide resolved

br/pkg/backup/client.go Outdated Show resolved Hide resolved

3pointer added 2 commits May 14, 2024 09:55

address comments

7271c3e

remove global variable

574360c

Leavrth reviewed May 14, 2024

View reviewed changes

br/pkg/backup/client.go Show resolved Hide resolved

br/pkg/backup/client.go Show resolved Hide resolved

address comment

8cec73f

Leavrth approved these changes May 14, 2024

View reviewed changes

ti-chi-bot bot added approved needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels May 14, 2024

YuJuncen reviewed May 15, 2024

View reviewed changes

fix typo

5870354

YuJuncen approved these changes May 15, 2024

View reviewed changes

ti-chi-bot bot added lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels May 15, 2024

ti-chi-bot bot merged commit 835ea04 into pingcap:master May 16, 2024
31 of 32 checks passed

terry1purcell pushed a commit to terry1purcell/tidb that referenced this pull request May 17, 2024

backup: implement store based backup to solve the long tail issue (pi…

33f1c6e

…ngcap#53081) ref pingcap#52534

RidRisR pushed a commit to RidRisR/tidb that referenced this pull request May 23, 2024

backup: implement store based backup to solve the long tail issue (pi…

378e37d

…ngcap#53081) ref pingcap#52534

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

backup: implement store based backup to solve the long tail issue #53081

backup: implement store based backup to solve the long tail issue #53081

3pointer commented May 7, 2024 •

edited

Loading

ti-chi-bot bot commented May 7, 2024

3pointer commented May 8, 2024

codecov bot commented May 8, 2024 •

edited

Loading

3pointer commented May 8, 2024

3pointer commented May 9, 2024

3pointer commented May 9, 2024

3pointer commented May 9, 2024

3pointer commented May 10, 2024

tiprow bot commented May 10, 2024

3pointer commented May 10, 2024

Leavrth left a comment

Leavrth May 14, 2024

YuJuncen left a comment

YuJuncen May 15, 2024

3pointer May 15, 2024

ti-chi-bot bot commented May 15, 2024

ti-chi-bot bot commented May 15, 2024

3pointer commented May 16, 2024

ti-chi-bot bot commented May 16, 2024

tiprow bot commented May 16, 2024

3pointer commented May 16, 2024

tiprow bot commented May 16, 2024

3pointer commented May 16, 2024

tiprow bot commented May 16, 2024

backup: implement store based backup to solve the long tail issue #53081

backup: implement store based backup to solve the long tail issue #53081

Conversation

3pointer commented May 7, 2024 • edited Loading

What problem does this PR solve?

What changed and how does it work?

Check List

Release note

ti-chi-bot bot commented May 7, 2024

3pointer commented May 8, 2024

codecov bot commented May 8, 2024 • edited Loading

Codecov Report

3pointer commented May 8, 2024

3pointer commented May 9, 2024

3pointer commented May 9, 2024

3pointer commented May 9, 2024

3pointer commented May 10, 2024

tiprow bot commented May 10, 2024

3pointer commented May 10, 2024

Leavrth left a comment

Choose a reason for hiding this comment

Leavrth May 14, 2024

Choose a reason for hiding this comment

YuJuncen left a comment

Choose a reason for hiding this comment

YuJuncen May 15, 2024

Choose a reason for hiding this comment

3pointer May 15, 2024

Choose a reason for hiding this comment

ti-chi-bot bot commented May 15, 2024

ti-chi-bot bot commented May 15, 2024

[LGTM Timeline notifier]

3pointer commented May 16, 2024

ti-chi-bot bot commented May 16, 2024

tiprow bot commented May 16, 2024

3pointer commented May 16, 2024

tiprow bot commented May 16, 2024

3pointer commented May 16, 2024

tiprow bot commented May 16, 2024

3pointer commented May 7, 2024 •

edited

Loading

codecov bot commented May 8, 2024 •

edited

Loading