store/driver: update client-go and errors

[disksing]

Signed-off-by: disksing i@disksing.com

What problem does this PR solve?

Issue Number: close #29294

What is changed and how it works?

• update client-go dependency
• update pingcap/errors to latest version
• replace errors.Equal usages with errors.Is and errors.As

Check List

Tests

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

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

None

[ti-chi-bot]

[REVIEW NOTIFICATION]

This pull request has been approved by:

• JmPotato
• ngaut

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

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

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

[iosmanthus]

LGTM

[ti-chi-bot]

@iosmanthus: Thanks for your review. The bot only counts approvals from reviewers and higher roles in list, but you're still welcome to leave your comments.

In response to this:

LGTM

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 ti-community-infra/tichi repository.

[disksing]

/wip
investigating some test errors.

[disksing]

Investigated the CI error and found that the failing test was introduced by the previous client errors feature (#22382). In that PR, errors.As was used to check the type of the error.

Unfortunately, previous versions of pingcap/errors did not support the Unwrap function required by std errors.{Is, As} until pingcap/errors#35.

This PR happens to have upgraded pingcap/errors, which will cause cases that previously failed with errors.As conversion to now succeed.

It looks like extracting errors.Error from the wrap-over errors is the result we want. So I think we should change the expect result of test here, please help to confirm /cc @morgo

[morgo]

Investigated the CI error and found that the failing test was introduced by the previous client errors feature (#22382). In that PR, errors.As was used to check the type of the error.

Yes, specifically it is only used by this code:

		if cc.ctx != nil && cc.ctx.WarningCount() > 0 {
			for _, err := range cc.ctx.GetWarnings() {
				var warn *errors.Error
				if ok := goerr.As(err.Err, &warn); ok {
					code := uint16(warn.Code())
					errno.IncrementWarning(code, cc.user, cc.peerHost)
				}
			}
		}

It only needs to extract the code as a uint16 from each warning. I will confess I don't understand the error code types that well. If you can simplify this code, it will still work fine.

[ngaut]

LGTM

[disksing]

Investigated the CI error and found that the failing test was introduced by the previous client errors feature (#22382). In that PR, errors.As was used to check the type of the error.

Yes, specifically it is only used by this code:

		if cc.ctx != nil && cc.ctx.WarningCount() > 0 {
			for _, err := range cc.ctx.GetWarnings() {
				var warn *errors.Error
				if ok := goerr.As(err.Err, &warn); ok {
					code := uint16(warn.Code())
					errno.IncrementWarning(code, cc.user, cc.peerHost)
				}
			}
		}

It only needs to extract the code as a uint16 from each warning. I will confess I don't understand the error code types that well. If you can simplify this code, it will still work fine.

I didn't mean to change this code here, but upgrading of pingcap/errors caused errors.As to behave differently than before. This means that some scenarios where errors.Error could not be extracted before, can now be extracted via errors.As. I'm wonder if this change is acceptable.

[morgo]

I didn't mean to change this code here, but upgrading of pingcap/errors caused errors.As to behave differently than before. This means that some scenarios where errors.Error could not be extracted before, can now be extracted via errors.As. I'm wonder if this change is acceptable.

Yes, this is an improvement! If errors could not be extracted before it would mean that some warnings would not publish to the infoschema table. Having them there now is great (it's actually a bug fix; the code might not have tested all scenarios correctly.)

[disksing]

I didn't mean to change this code here, but upgrading of pingcap/errors caused errors.As to behave differently than before. This means that some scenarios where errors.Error could not be extracted before, can now be extracted via errors.As. I'm wonder if this change is acceptable.

Yes, this is an improvement! If errors could not be extracted before it would mean that some warnings would not publish to the infoschema table. Having them there now is great (it's actually a bug fix; the code might not have tested all scenarios correctly.)

Thanks for your reply. I'll update the tests.

[disksing]

/run-check_dev_2

[disksing]

/merge

[ti-chi-bot]

This pull request has been accepted and is ready to merge.

Commit hash: 695d4bb