dupdetect: gRPC cancel should trigger retry #58542
Conversation
Signed-off-by: lance6716 <[email protected]>
ti-chi-bot
bot
added
the
release-note-none
ti-chi-bot
bot
added
the
size/XS
|
Hi @lance6716. Thanks for your PR. PRs from untrusted users cannot be marked as trusted with 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-sigs/prow repository. |
Signed-off-by: lance6716 <[email protected]>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #58542 +/- ##
================================================
+ Coverage 73.5134% 74.6694% +1.1559%
================================================
Files 1681 1711 +30
Lines 464459 486416 +21957
================================================
+ Hits 341440 363204 +21764
+ Misses 102157 101683 -474
- Partials 20862 21529 +667
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
/cc @D3Hunter |
| @@ -997,7 +998,7 @@ func (m *dupeDetector) processRemoteDupTask( | |||
| } | |||
| return nil | |||
| } | |||
| if log.IsContextCanceledError(err) { | |||
| if stderrors.Is(err, context.Canceled) { | |||
There was a problem hiding this comment.
will err be nested? maybe check ctx.Err() != nil
There was a problem hiding this comment.
I see https://pkg.go.dev/errors#Is can handle nested error by checking Unwrap interface. However I think check the ctx argument is more direct. I'll check it soon.
D3Hunter
changed the title
Signed-off-by: lance6716 <[email protected]>
ti-chi-bot
bot
added
approved
needs-1-more-lgtm
|
LGTM |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: D3Hunter, wjhuang2016 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:
Approvers can indicate their approval by writing |
ti-chi-bot
bot
added
lgtm
and removed
needs-1-more-lgtm
[LGTM Timeline notifier]Timeline:
|
|
/retest |
|
@lance6716: Cannot trigger testing until a trusted user reviews the PR and leaves an In response to this:
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. |
|
/retest |
|
@lance6716: Cannot trigger testing until a trusted user reviews the PR and leaves an In response to this:
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. |
…long-vector * commit '510d0037b18f258f505abc6cf13a8128563e9359': *: upgrade pd client to make sure tso client initiate successfully (#58752) ttl, test: scale TTL workers during the fault tests (#58750) planner: improve warning messages for unsupported HASH_JOIN hints (#58646) planner: prealloc the slices in the SplitCorColAccessCondFromFilters (#58785) ddl: supports non-unique global index (#58678) util/stmtsummary: add the network traffic related fields (#58101) var: enable `pd_enable_follower_handle_region` as default (#58385) statistics: refactor stats meta handling to use DeltaUpdate for multi-table support (#58657) parser: move 'model' to 'ast' pkg (#58704) statistics: add recover to protect background task (#58739) disttask: cancel subtask context if scheduled away (#58615) *: don't handle live updates of column size (#58596) *: fix a bug for default_authentication_plugin (2) (#58723) dupdetect: gRPC cancel should trigger retry (#58542) *: fix a bug for default_authentication_plugin (#57391) distsql: Fix backoff execution info inaccurate issue (#58707)
What problem does this PR solve?
Issue Number: close #58523
Problem Summary:
What changed and how does it work?
gRPC "canceled" error may be caused by network problems, for example the server thinks the client is gone so it cancels the RPC, and then the client receives canceled error, because streaming RPC has 2 connections independently for C->S and S->C. In lightning use case, we only want to break the retry when the user cancels it.
Check List
Tests
Side effects
Documentation
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.