backup: advacned prepare implementation #48439

YuJuncen · 2023-11-08T13:14:52Z

What problem does this PR solve?

Issue Number: close #50359

Problem Summary:
This PR implemented the client side of the enhanced prepare stage of snapshot backup.

What is changed and how it works?

Generally this is a state machine that tries to collect the whole range. If a region has applied to the last index, the region's range will be collected. When the whole key space has been collected, we are done.

Check List

Tests

Unit test (TBD)
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

Signed-off-by: hillium <[email protected]>

tiprow · 2023-11-08T13:15:09Z

Hi @YuJuncen. Thanks for your PR.

PRs from untrusted users cannot be marked as trusted with /ok-to-test in this repo meaning untrusted PR authors can never trigger tests themselves. Collaborators can still trigger tests on the PR using /test all.

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.

Signed-off-by: hillium <[email protected]>

…d-prepare Signed-off-by: hillium <[email protected]>

Signed-off-by: hillium <[email protected]>

…nto advacned-prepare

Signed-off-by: hillium <[email protected]>

YuJuncen · 2023-11-17T06:57:11Z

/build

Signed-off-by: hillium <[email protected]>

…nto advacned-prepare

codecov · 2023-11-17T07:54:41Z

Codecov Report

Merging #48439 (7a2948e) into master (9fb89b0) will decrease coverage by 14.5137%.
Report is 23 commits behind head on master.
The diff coverage is 69.2041%.

Additional details and impacted files

@@                Coverage Diff                @@
##             master     #48439         +/-   ##
=================================================
- Coverage   70.0608%   55.5471%   -14.5137%     
=================================================
  Files          1445       1563        +118     
  Lines        420102     587661     +167559     
=================================================
+ Hits         294327     326429      +32102     
- Misses       105477     238413     +132936     
- Partials      20298      22819       +2521

Flag	Coverage Δ
integration	`36.7136% <48.9619%> (?)`
unit	`70.1537% <59.6885%> (+0.0929%)`	⬆️

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

Components	Coverage Δ
dumpling	`54.0269% <ø> (ø)`
parser	`∅ <ø> (∅)`
br	`56.4434% <69.1228%> (+6.8186%)`	⬆️

…d-prepare

Signed-off-by: hillium <[email protected]>

…nto advacned-prepare

Signed-off-by: hillium <[email protected]>

3pointer · 2024-01-17T09:58:02Z

br/pkg/backup/prepare_snap/env.go

+	bo := tikv.NewBackoffer(ctx, regionCacheMaxBackoffMs)
+	if len(endKey) == 0 {
+		// This is encoded [0xff; 8].
+		// Workaround for https://github.com/tikv/client-go/issues/1051.


This issue has been fixed, can we remove the workaround and upgrade client-go?

That may not be picked to release version and upgrading client-go may have other side effects. I think we'd better keep this unchanged for the consistency between all versions.

3pointer · 2024-01-17T10:23:02Z

br/pkg/backup/prepare_snap/stream.go

+}
+
+func (p *prepareStream) convertToEvent(resp *brpb.PrepareSnapshotBackupResponse) (event, bool) {
+	switch resp.Ty {


What's the response type of PrepareSnapshotBackupRequestType_Finish?

Update lease result.

3pointer · 2024-01-17T10:33:23Z

br/pkg/backup/prepare_snap/prepare.go

+			p.nextRetry = time.NewTimer(p.RetryBackoff)
+			return nil
+		}
+		if item, ok := p.waitApplyDoneRegions.ReplaceOrInsert(r); ok {


when overlapped happened. Do we need set overlapped range to fail?

If that range is larger, it may be retry after we checking the holes in AdvanceStep, if it is smaller, I think it can be ignored.

3pointer · 2024-01-17T10:53:14Z

br/pkg/backup/prepare_snap/env.go

+
+func (s SplitRequestClient) Send(req *brpb.PrepareSnapshotBackupRequest) error {
+	// Try best to keeping the request untouched.
+	rs := req.Regions


seems it canbe moved into if req.Ty == xxxxx, :)

3pointer

rest LGTM

ti-chi-bot · 2024-01-17T10:55:07Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 3pointer, BornChanger

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~~ [3pointer]
~~br/OWNERS~~ [3pointer,BornChanger]

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-01-17T10:55:11Z

[LGTM Timeline notifier]

Timeline:

2024-01-17 10:10:15.027941251 +0000 UTC m=+350656.592238956: ☑️ agreed by BornChanger.
2024-01-17 10:55:10.392956378 +0000 UTC m=+353351.957254082: ☑️ agreed by 3pointer.

tiprow · 2024-01-17T11:02:03Z

@YuJuncen: 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
tiprow_fast_test	`b13969c`	link	true	`/test tiprow_fast_test`

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/test-infra repository. I understand the commands that are listed here.

Signed-off-by: hillium <[email protected]>

BornChanger · 2024-01-17T13:22:53Z

/label needs-cherry-pick-release-6.5

BornChanger · 2024-01-17T13:23:06Z

/label needs-cherry-pick-release-7.1

BornChanger · 2024-01-17T13:23:12Z

needs-cherry-pick-release-7.5

ti-chi-bot · 2024-01-17T13:23:38Z

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

Signed-off-by: ti-chi-bot <[email protected]>

ti-chi-bot · 2024-01-17T13:23:51Z

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

close #50359

close pingcap#50359

… (pingcap#39) close pingcap#50359 Co-authored-by: Ti Chi Robot <[email protected]>

ti-chi-bot · 2024-04-12T08:34:18Z

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

Signed-off-by: ti-chi-bot <[email protected]>

YuJuncen added 2 commits November 8, 2023 21:02

added client for preparing snapshot

e34b096

Signed-off-by: hillium <[email protected]>

make bazel_prepare

50cbcde

Signed-off-by: hillium <[email protected]>

ti-chi-bot bot added do-not-merge/invalid-title do-not-merge/needs-linked-issue 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 Nov 8, 2023

YuJuncen changed the title ~~Advacned prepare~~ backup: advacned prepare implementation Nov 8, 2023

ti-chi-bot bot removed the do-not-merge/invalid-title label Nov 8, 2023

YuJuncen and others added 8 commits November 10, 2023 12:25

WIP

0390885

Signed-off-by: hillium <[email protected]>

Merge branch 'master' of https://github.com/pingcap/tidb into advacne…

b59bf5d

…d-prepare Signed-off-by: hillium <[email protected]>

added more test case

17045f7

Signed-off-by: hillium <[email protected]>

make bazel_prepare

4689cb6

Merge branch 'advacned-prepare' of https://github.com/YuJuncen/tidb i…

45d4c22

…nto advacned-prepare

added more test cases

a70504b

Signed-off-by: hillium <[email protected]>

move test cases to test pacakge

f388412

Signed-off-by: hillium <[email protected]>

make bazel_prepare

7ef7c30

YuJuncen added 2 commits November 17, 2023 15:41

fix build

1831260

Signed-off-by: hillium <[email protected]>

Merge branch 'advacned-prepare' of https://github.com/YuJuncen/tidb i…

73d47a5

…nto advacned-prepare

ti-chi-bot bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 20, 2023

Merge branch 'master' of https://github.com/pingcap/tidb into advance…

90d53a2

…d-prepare

BornChanger mentioned this pull request Nov 29, 2023

EBS BR support to multiple k8s cluster TiDB pingcap/tidb-operator#5003

Open

YuJuncen added 6 commits December 7, 2023 15:12

update kvproto

e221a8d

Signed-off-by: hillium <[email protected]>

update reps

d434e79

Signed-off-by: hillium <[email protected]>

Merge branch 'advacned-prepare' of https://github.com/YuJuncen/tidb i…

8f391d5

…nto advacned-prepare

added retry on failure

ea8739d

Signed-off-by: hillium <[email protected]>

connect to all store

63fc060

Signed-off-by: hillium <[email protected]>

optimized log

d8807d9

Signed-off-by: hillium <[email protected]>

3pointer reviewed Jan 17, 2024

View reviewed changes

3pointer approved these changes Jan 17, 2024

View reviewed changes

ti-chi-bot bot added approved lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Jan 17, 2024

address comments

7a2948e

Signed-off-by: hillium <[email protected]>

ti-chi-bot bot merged commit bbbada0 into pingcap:master Jan 17, 2024
22 checks passed

ti-chi-bot bot added the needs-cherry-pick-release-6.5 Should cherry pick this PR to release-6.5 branch. label Jan 17, 2024

ti-chi-bot bot added the needs-cherry-pick-release-7.1 Should cherry pick this PR to release-7.1 branch. label Jan 17, 2024

ti-chi-bot pushed a commit to ti-chi-bot/tidb that referenced this pull request Jan 17, 2024

This is an automated cherry-pick of pingcap#48439

1fe7fc0

Signed-off-by: ti-chi-bot <[email protected]>

ti-chi-bot mentioned this pull request Jan 17, 2024

backup: advacned prepare implementation (#48439) #50520

Merged

13 tasks

ti-chi-bot pushed a commit to ti-chi-bot/tidb that referenced this pull request Jan 17, 2024

This is an automated cherry-pick of pingcap#48439

d71c63d

Signed-off-by: ti-chi-bot <[email protected]>

ti-chi-bot mentioned this pull request Jan 17, 2024

backup: advacned prepare implementation (#48439) #50521

Open

13 tasks

ti-chi-bot bot pushed a commit that referenced this pull request Jan 18, 2024

backup: advacned prepare implementation (#48439) (#50520)

e850d08

close #50359

YuJuncen added a commit to ti-chi-bot/tidb that referenced this pull request Feb 20, 2024

backup: advacned prepare implementation (pingcap#48439)

85f3ee8

close pingcap#50359

YuJuncen mentioned this pull request Feb 20, 2024

ebs_br: allow temporary TiKV unreachable during starting snapshot backup (#49154) #50443

Merged

5 tasks

guoshouyan pushed a commit to guoshouyan/tidb that referenced this pull request Mar 5, 2024

backup: advacned prepare implementation (pingcap#48439) (pingcap#50520)…

ede3585

… (pingcap#39) close pingcap#50359 Co-authored-by: Ti Chi Robot <[email protected]>

BornChanger added the needs-cherry-pick-release-7.5 Should cherry pick this PR to release-7.5 branch. label Apr 12, 2024

ti-chi-bot pushed a commit to ti-chi-bot/tidb that referenced this pull request Apr 12, 2024

This is an automated cherry-pick of pingcap#48439

49dc8b1

Signed-off-by: ti-chi-bot <[email protected]>

ti-chi-bot mentioned this pull request Apr 12, 2024

backup: advacned prepare implementation (#48439) #52569

Closed

13 tasks

ti-chi-bot bot removed the needs-cherry-pick-release-7.1 Should cherry pick this PR to release-7.1 branch. label Nov 7, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

backup: advacned prepare implementation #48439

backup: advacned prepare implementation #48439

YuJuncen commented Nov 8, 2023 •

edited

Loading

tiprow bot commented Nov 8, 2023

YuJuncen commented Nov 17, 2023

codecov bot commented Nov 17, 2023 •

edited

Loading

3pointer Jan 17, 2024

YuJuncen Jan 17, 2024

3pointer Jan 17, 2024

YuJuncen Jan 17, 2024

3pointer Jan 17, 2024

YuJuncen Jan 17, 2024

3pointer Jan 17, 2024

3pointer left a comment

ti-chi-bot bot commented Jan 17, 2024

ti-chi-bot bot commented Jan 17, 2024

tiprow bot commented Jan 17, 2024

BornChanger commented Jan 17, 2024

BornChanger commented Jan 17, 2024

BornChanger commented Jan 17, 2024

ti-chi-bot commented Jan 17, 2024

ti-chi-bot commented Jan 17, 2024

ti-chi-bot commented Apr 12, 2024

backup: advacned prepare implementation #48439

backup: advacned prepare implementation #48439

Conversation

YuJuncen commented Nov 8, 2023 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

tiprow bot commented Nov 8, 2023

YuJuncen commented Nov 17, 2023

codecov bot commented Nov 17, 2023 • edited Loading

Codecov Report

3pointer Jan 17, 2024

Choose a reason for hiding this comment

YuJuncen Jan 17, 2024

Choose a reason for hiding this comment

3pointer Jan 17, 2024

Choose a reason for hiding this comment

YuJuncen Jan 17, 2024

Choose a reason for hiding this comment

3pointer Jan 17, 2024

Choose a reason for hiding this comment

YuJuncen Jan 17, 2024

Choose a reason for hiding this comment

3pointer Jan 17, 2024

Choose a reason for hiding this comment

3pointer left a comment

Choose a reason for hiding this comment

ti-chi-bot bot commented Jan 17, 2024

ti-chi-bot bot commented Jan 17, 2024

[LGTM Timeline notifier]

tiprow bot commented Jan 17, 2024

BornChanger commented Jan 17, 2024

BornChanger commented Jan 17, 2024

BornChanger commented Jan 17, 2024

ti-chi-bot commented Jan 17, 2024

ti-chi-bot commented Jan 17, 2024

ti-chi-bot commented Apr 12, 2024

YuJuncen commented Nov 8, 2023 •

edited

Loading

codecov bot commented Nov 17, 2023 •

edited

Loading