br: refactor error handle mechanism to tolerant unexpect kv errors. #48646

3pointer · 2023-11-16T09:48:19Z

What problem does this PR solve?

Issue Number: close #47656

Problem Summary:
record the state of unknown kv error and give some tolerance of these unknown error.

What is changed and how it works?

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 · 2023-11-16T09:48:22Z

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

tiprow · 2023-11-16T09:48:36Z

Hi @3pointer. 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.

codecov · 2023-12-05T06:58:26Z

Codecov Report

Merging #48646 (7d10267) into master (28cb579) will increase coverage by 1.0382%.
Report is 32 commits behind head on master.
The diff coverage is 0.0000%.

Additional details and impacted files

@@               Coverage Diff                @@
##             master     #48646        +/-   ##
================================================
+ Coverage   71.0454%   72.0836%   +1.0382%     
================================================
  Files          1368       1407        +39     
  Lines        402969     417762     +14793     
================================================
+ Hits         286291     301138     +14847     
- Misses        96737      97753      +1016     
+ Partials      19941      18871      -1070

Flag	Coverage Δ
integration	`43.8085% <0.0000%> (?)`
unit	`71.0510% <ø> (+0.0056%)`	⬆️

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

Components	Coverage Δ
dumpling	`53.9663% <ø> (ø)`
parser	`∅ <ø> (∅)`
br	`47.9098% <0.0000%> (-5.0616%)`	⬇️

YuJuncen

rest lgtm

YuJuncen · 2023-12-05T06:57:17Z

br/pkg/utils/retry.go

+
+const (
+	// This type can be retry but consume the backoffer attempts.
+	Retry ErrorStrategy = iota


I will prefer add a prefix Strategy to this or extract this retry related utilities to another package. utils.Retry looks like a function.

YuJuncen · 2023-12-05T07:07:52Z

br/pkg/utils/retry.go

+	if messageIsNotFoundStorageError(msg) {
+		reason := fmt.Sprintf("File or directory not found on TiKV Node (store id: %v). "+
+			"work around:please ensure br and tikv nodes share a same storage and the user of br and tikv has same uid.",
+			uuid)


Why store id is a uuid?🤔

because during restore. we cannot get store id

YuJuncen · 2023-12-05T07:35:03Z

br/pkg/utils/retry.go

+	return ErrorResult{GiveUp, "unknown error and retry too many times, give up"}
+}
+
+func (ec *ErrorContext) HandleErrorPb(e *backuppb.Error, uuid uint64, canIgnore bool) ErrorResult {


canIgnore looks so magical here, I guess it should be the requirement of the coarse-grained backup. I will prefer make another function to handle the scenario.

pushBackup need it, but fine-grained doesn't. Do you think make it a field of errContext is better?

I think we can make two functions: HandleErrAndIgnoreStoreErrors and HandleErr or something. 🤔

ti-chi-bot · 2023-12-07T03:29:34Z

[FORMAT CHECKER NOTIFICATION]

Notice: To remove the do-not-merge/needs-tests-checked label, please finished the tests then check the finished items in description.

For example:

Tests

Unit test

Integration test

Manual test (add detailed scripts or steps below)

No code

_{📖 For more info, you can check the "Contribute Code" section in the development guide.}

ti-chi-bot · 2023-12-07T04:01:20Z

[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 · 2023-12-07T04:01:24Z

[LGTM Timeline notifier]

Timeline:

2023-12-07 03:37:08.110530446 +0000 UTC m=+1671456.775756642: ☑️ agreed by Leavrth.
2023-12-07 04:01:23.63277732 +0000 UTC m=+1672912.298003512: ☑️ agreed by YuJuncen.

3pointer · 2023-12-08T05:15:13Z

/test mysql-test

tiprow · 2023-12-08T05:15:36Z

@3pointer: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test message.

In response to this:

/test mysql-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.

ti-chi-bot · 2023-12-08T05:21:36Z

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

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

ti-chi-bot · 2023-12-08T05:22:19Z

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

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

…48646) (#49276) close #47656

ti-chi-bot · 2024-02-04T05:32:50Z

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

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

…48646) (#50951) close #47656

…48646) (#49274) close #47656

3pointer added 4 commits November 16, 2023 13:58

backup error refactor

f144aed

import error refactor

e7a8eb8

update

940bb9c

udpate

b7f1d3a

ti-chi-bot bot added do-not-merge/needs-tests-checked do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels Nov 16, 2023

3pointer added 3 commits November 16, 2023 21:29

lock

7912ea0

Merge branch 'master' into error_refactor

34b91cb

update test

4a6b938

ti-chi-bot bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Dec 5, 2023

add more test

10e22af

3pointer marked this pull request as ready for review December 5, 2023 06:46

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 Dec 5, 2023

more test

1405f58

YuJuncen reviewed Dec 5, 2023

View reviewed changes

3pointer added 2 commits December 5, 2023 18:12

address comment

6a65e40

address comment

26819c9

3pointer force-pushed the error_refactor branch from e2c5f46 to 26819c9 Compare December 6, 2023 10:12

3pointer added 4 commits December 6, 2023 18:29

fix bazel

c2df58e

fix lint

c8b880e

fix lint

7b495df

fix test

dd62063

address comment

7d10267

Leavrth approved these changes Dec 7, 2023

View reviewed changes

ti-chi-bot bot added approved needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Dec 7, 2023

YuJuncen approved these changes Dec 7, 2023

View reviewed changes

ti-chi-bot bot added lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Dec 7, 2023

3pointer removed the do-not-merge/needs-tests-checked label Dec 8, 2023

ti-chi-bot bot merged commit 6c30c6e into pingcap:master Dec 8, 2023
27 of 31 checks passed

ti-chi-bot mentioned this pull request Dec 8, 2023

br: refactor error handle mechanism to tolerant unexpect kv errors. (#48646) #49274

Merged

13 tasks

ti-chi-bot pushed a commit to ti-chi-bot/tidb that referenced this pull request Dec 8, 2023

This is an automated cherry-pick of pingcap#48646

2efc9ba

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

ti-chi-bot mentioned this pull request Dec 8, 2023

br: refactor error handle mechanism to tolerant unexpect kv errors. (#48646) #49276

Merged

13 tasks

ti-chi-bot pushed a commit to ti-chi-bot/tidb that referenced this pull request Dec 8, 2023

This is an automated cherry-pick of pingcap#48646

5e97ef3

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

ti-chi-bot bot pushed a commit that referenced this pull request Dec 11, 2023

br: refactor error handle mechanism to tolerant unexpect kv errors. (#…

eacc493

…48646) (#49276) close #47656

3pointer added the needs-cherry-pick-release-7.5 Should cherry pick this PR to release-7.5 branch. label Feb 4, 2024

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

This is an automated cherry-pick of pingcap#48646

82baa38

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>

ti-chi-bot mentioned this pull request Feb 4, 2024

br: refactor error handle mechanism to tolerant unexpect kv errors. (#48646) #50951

Merged

13 tasks

ti-chi-bot bot pushed a commit that referenced this pull request Feb 4, 2024

br: refactor error handle mechanism to tolerant unexpect kv errors. (#…

2ca45ac

…48646) (#50951) close #47656

ti-chi-bot bot pushed a commit that referenced this pull request Feb 21, 2024

br: refactor error handle mechanism to tolerant unexpect kv errors. (#…

532100f

…48646) (#49274) close #47656

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

br: refactor error handle mechanism to tolerant unexpect kv errors. #48646

br: refactor error handle mechanism to tolerant unexpect kv errors. #48646

3pointer commented Nov 16, 2023

ti-chi-bot bot commented Nov 16, 2023

tiprow bot commented Nov 16, 2023

codecov bot commented Dec 5, 2023 •

edited

Loading

YuJuncen left a comment

YuJuncen Dec 5, 2023

3pointer Dec 5, 2023

YuJuncen Dec 5, 2023

3pointer Dec 5, 2023

YuJuncen Dec 5, 2023

3pointer Dec 5, 2023

YuJuncen Dec 6, 2023

ti-chi-bot bot commented Dec 7, 2023

ti-chi-bot bot commented Dec 7, 2023

ti-chi-bot bot commented Dec 7, 2023

3pointer commented Dec 8, 2023

tiprow bot commented Dec 8, 2023

ti-chi-bot commented Dec 8, 2023

ti-chi-bot commented Dec 8, 2023

ti-chi-bot commented Feb 4, 2024

br: refactor error handle mechanism to tolerant unexpect kv errors. #48646

br: refactor error handle mechanism to tolerant unexpect kv errors. #48646

Conversation

3pointer commented Nov 16, 2023

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

ti-chi-bot bot commented Nov 16, 2023

tiprow bot commented Nov 16, 2023

codecov bot commented Dec 5, 2023 • edited Loading

Codecov Report

YuJuncen left a comment

Choose a reason for hiding this comment

YuJuncen Dec 5, 2023

Choose a reason for hiding this comment

3pointer Dec 5, 2023

Choose a reason for hiding this comment

YuJuncen Dec 5, 2023

Choose a reason for hiding this comment

3pointer Dec 5, 2023

Choose a reason for hiding this comment

YuJuncen Dec 5, 2023

Choose a reason for hiding this comment

3pointer Dec 5, 2023

Choose a reason for hiding this comment

YuJuncen Dec 6, 2023

Choose a reason for hiding this comment

ti-chi-bot bot commented Dec 7, 2023

ti-chi-bot bot commented Dec 7, 2023

ti-chi-bot bot commented Dec 7, 2023

[LGTM Timeline notifier]

3pointer commented Dec 8, 2023

tiprow bot commented Dec 8, 2023

ti-chi-bot commented Dec 8, 2023

ti-chi-bot commented Dec 8, 2023

ti-chi-bot commented Feb 4, 2024

codecov bot commented Dec 5, 2023 •

edited

Loading