errs: remove redundant `FastGenWithCause` in `ZapError` #7497

CabinfeverB · 2023-12-06T03:18:39Z

What problem does this PR solve?

Issue Number: Close #7496

What is changed and how does it work?

Check List

Tests

Manual test (add detailed scripts or steps below)
after

[2023/12/06 10:43:25.636 +08:00] [ERROR] [tso_dispatcher.go:498] ["[tso] getTS error after processing requests"] [dc-location=global] [stream-addr=http://127.0.0.1:64317] [error="[PD:client:ErrClientGetTSO]get TSO failed, rpc error: code = Unknown desc = [PD:tso:ErrGenerateTimestamp]generate timestamp failed, requested pd is not leader of cluster"]

some example

[2023/12/06 11:16:43.550 +08:00] [ERROR] [client_test.go:506] ["[pd] failed to close grpc clientConn"] [error="[PD:grpc:ErrCloseGRPCConn]close gRPC connection failed: test"] [stack="github.com/tikv/pd/tests/integrations/client_test.TestCustomTimeout\n\t/Users/jiangyongbo/github/pd/tests/integrations/client/client_test.go:506\ntesting.tRunner\n\t/usr/local/go/src/testing/testing.go:1595"]
[2023/12/06 11:16:43.551 +08:00] [ERROR] [client_test.go:507] ["[tso] fetch pending tso requests error"] [error="[PD:client:ErrClientGetTSO]get TSO failed: test"] [stack="github.com/tikv/pd/tests/integrations/client_test.TestCustomTimeout\n\t/Users/jiangyongbo/github/pd/tests/integrations/client/client_test.go:507\ntesting.tRunner\n\t/usr/local/go/src/testing/testing.go:1595"]
[2023/12/06 11:16:43.551 +08:00] [ERROR] [client_test.go:508] ["send heartbeat message fail"] [error="[PD:grpc:ErrGRPCSend]send request error: test"] [stack="github.com/tikv/pd/tests/integrations/client_test.TestCustomTimeout\n\t/Users/jiangyongbo/github/pd/tests/integrations/client/client_test.go:508\ntesting.tRunner\n\t/usr/local/go/src/testing/testing.go:1595"]
[2023/12/06 11:16:43.551 +08:00] [ERROR] [client_test.go:509] ["watch gc safe point v2 error"] [error="[PD:client:ErrClientWatchGCSafePointV2Stream]watch gc safe point v2 stream failed: test"] [stack="github.com/tikv/pd/tests/integrations/client_test.TestCustomTimeout\n\t/Users/jiangyongbo/github/pd/tests/integrations/client/client_test.go:509\ntesting.tRunner\n\t/usr/local/go/src/testing/testing.go:1595"]

Code changes

Has the configuration change
Has HTTP API interfaces changed (Don't forget to add the declarative for the new API)
Has persistent data change

Side effects

Possible performance regression
Increased code complexity
Breaking backward compatibility

Related changes

PR to update pingcap/docs/pingcap/docs-cn:
PR to update pingcap/tiup:
Need to cherry-pick to the release branch

Release note

None.

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

ti-chi-bot · 2023-12-06T03:18:42Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

lhy1024
nolouch

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.

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

codecov · 2023-12-06T03:46:21Z

Codecov Report

Merging #7497 (98f5a6f) into master (0e220b0) will decrease coverage by 0.07%.
Report is 1 commits behind head on master.
The diff coverage is 20.00%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #7497      +/-   ##
==========================================
- Coverage   74.66%   74.59%   -0.07%     
==========================================
  Files         454      454              
  Lines       50142    50142              
==========================================
- Hits        37437    37404      -33     
- Misses       9392     9403      +11     
- Partials     3313     3335      +22

Flag	Coverage Δ
unittests	`74.59% <20.00%> (-0.07%)`	⬇️

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

lhy1024 · 2023-12-07T01:43:24Z

client/errs/errs.go

@@ -27,7 +27,7 @@ func ZapError(err error, causeError ...error) zap.Field {
 	}
 	if e, ok := err.(*errors.Error); ok {
 		if len(causeError) >= 1 {
-			err = e.Wrap(causeError[0]).FastGenWithCause()


Do we need to check other FastGenWithCause?

OK, I forgot about other direct use cases

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

nolouch

lgtm

nolouch · 2023-12-13T11:16:24Z

/merge

ti-chi-bot · 2023-12-13T11:16:27Z

@nolouch: It seems you want to merge this PR, I will help you trigger all the tests:

/run-all-tests

You only need to trigger /merge once, and if the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes.

If you have any questions about the PR merge process, please refer to pr process.

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.

ti-chi-bot · 2023-12-13T11:16:29Z

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

Commit hash: 3efb9a3

ti-chi-bot · 2023-12-13T11:27:58Z

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

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

ti-chi-bot · 2023-12-13T11:28:35Z

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

close #7496 Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com> Co-authored-by: Cabinfever_B <cabinfeveroier@gmail.com>

close #7496 Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io> Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com> Co-authored-by: Yongbo Jiang <cabinfeveroier@gmail.com> Co-authored-by: Cabinfever_B <cabinfeveroier@gmail.com> Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com>

…ikv#245) Co-authored-by: Yongbo Jiang <cabinfeveroier@gmail.com> Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com> close tikv#7496

CabinfeverB added 2 commits December 6, 2023 10:44

fix wrap

4f4442c

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

fix wrap

c90566b

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

CabinfeverB requested review from lhy1024 and rleungx December 6, 2023 03:18

CabinfeverB changed the title ~~errs: fix wrong wrap in ZapError~~ errs: remove redundant FastGenWithCause in ZapError Dec 6, 2023

fix wrap

c835fb8

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

CabinfeverB requested a review from JmPotato December 7, 2023 01:14

lhy1024 approved these changes Dec 7, 2023

View reviewed changes

ti-chi-bot bot added the status/LGT1 Indicates that a PR has LGTM 1. label Dec 7, 2023

address comment

f6c3d2e

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

ti-chi-bot bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Dec 8, 2023

add test

3efb9a3

Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com>

ti-chi-bot bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Dec 8, 2023

nolouch approved these changes Dec 13, 2023

View reviewed changes

ti-chi-bot bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Dec 13, 2023

ti-chi-bot bot added the status/can-merge Indicates a PR has been approved by a committer. label Dec 13, 2023

Merge branch 'master' into fix_zap_error

98f5a6f

ti-chi-bot bot merged commit f51f913 into tikv:master Dec 13, 2023
24 of 26 checks passed

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

errs: remove redundant FastGenWithCause in ZapError (#7497) #7544

Merged

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

This is an automated cherry-pick of tikv#7497

91dbdb3

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

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

errs: remove redundant FastGenWithCause in ZapError (#7497) #7545

Merged

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

errs: remove redundant FastGenWithCause in ZapError (#7497) (#7545)

c9c9979

close #7496 Signed-off-by: Cabinfever_B <cabinfeveroier@gmail.com> Co-authored-by: Cabinfever_B <cabinfeveroier@gmail.com>

JmPotato deleted the fix_zap_error branch January 2, 2024 08:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

errs: remove redundant `FastGenWithCause` in `ZapError` #7497

errs: remove redundant `FastGenWithCause` in `ZapError` #7497

CabinfeverB commented Dec 6, 2023

ti-chi-bot bot commented Dec 6, 2023 •

edited

Loading

codecov bot commented Dec 6, 2023 •

edited

Loading

lhy1024 Dec 7, 2023

CabinfeverB Dec 7, 2023

nolouch left a comment

nolouch commented Dec 13, 2023

ti-chi-bot bot commented Dec 13, 2023

ti-chi-bot bot commented Dec 13, 2023

ti-chi-bot commented Dec 13, 2023

ti-chi-bot commented Dec 13, 2023

errs: remove redundant FastGenWithCause in ZapError #7497

errs: remove redundant FastGenWithCause in ZapError #7497

Conversation

CabinfeverB commented Dec 6, 2023

What problem does this PR solve?

What is changed and how does it work?

Check List

Release note

ti-chi-bot bot commented Dec 6, 2023 • edited Loading

codecov bot commented Dec 6, 2023 • edited Loading

Codecov Report

lhy1024 Dec 7, 2023

Choose a reason for hiding this comment

CabinfeverB Dec 7, 2023

Choose a reason for hiding this comment

nolouch left a comment

Choose a reason for hiding this comment

nolouch commented Dec 13, 2023

ti-chi-bot bot commented Dec 13, 2023

ti-chi-bot bot commented Dec 13, 2023

ti-chi-bot commented Dec 13, 2023

ti-chi-bot commented Dec 13, 2023

errs: remove redundant `FastGenWithCause` in `ZapError` #7497

errs: remove redundant `FastGenWithCause` in `ZapError` #7497

ti-chi-bot bot commented Dec 6, 2023 •

edited

Loading

codecov bot commented Dec 6, 2023 •

edited

Loading