Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

server: Refine log error format #2873

Merged
merged 27 commits into from
Sep 4, 2020
Merged

server: Refine log error format #2873

merged 27 commits into from
Sep 4, 2020

Conversation

ZenoTan
Copy link
Contributor

@ZenoTan ZenoTan commented Sep 1, 2020

Signed-off-by: ZenoTan zenotan1998@gmail.com

What problem does this PR solve?

Refine log format in package server. Solve #2704

What is changed and how it works?

log.Error and errors.Errorf parts are changed

Check List

Tests

  • No code

Code changes

  • Has log error change

Release note

  • No release note

Signed-off-by: ZenoTan <zenotan1998@gmail.com>
@ti-srebot ti-srebot added the contribution Indicates that the PR was contributed by an external member. label Sep 1, 2020
@sre-bot
Copy link
Contributor

sre-bot commented Sep 1, 2020

@sre-bot
Copy link
Contributor

sre-bot commented Sep 1, 2020

@sre-bot
Copy link
Contributor

sre-bot commented Sep 1, 2020

Signed-off-by: ZenoTan <zenotan1998@gmail.com>
@sre-bot
Copy link
Contributor

sre-bot commented Sep 1, 2020

@sre-bot
Copy link
Contributor

sre-bot commented Sep 1, 2020

@sre-bot
Copy link
Contributor

sre-bot commented Sep 1, 2020

@sre-bot
Copy link
Contributor

sre-bot commented Sep 1, 2020

@sre-bot
Copy link
Contributor

sre-bot commented Sep 1, 2020

@sre-bot
Copy link
Contributor

sre-bot commented Sep 1, 2020

@sre-bot
Copy link
Contributor

sre-bot commented Sep 1, 2020

@sre-bot
Copy link
Contributor

sre-bot commented Sep 1, 2020

@sre-bot
Copy link
Contributor

sre-bot commented Sep 1, 2020

@sre-bot
Copy link
Contributor

sre-bot commented Sep 1, 2020

@sre-bot
Copy link
Contributor

sre-bot commented Sep 1, 2020

@nolouch
Copy link
Contributor

nolouch commented Sep 4, 2020

please address the comments. @ZenoTan

Signed-off-by: ZenoTan <zenotan1998@gmail.com>
@@ -96,7 +96,7 @@ func (s *heartbeatStreams) run() {
if stream, ok := s.streams[storeID]; ok {
if err := stream.Send(msg); err != nil {
log.Error("send heartbeat message fail",
zap.Uint64("region-id", msg.RegionId), errs.ZapError(err))
zap.Uint64("region-id", msg.RegionId), errs.ZapError(errs.ErrPbStreamSend.Wrap(err).GenWithStackByArgs()))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think gRPC send is enough for this?

Signed-off-by: ZenoTan <zenotan1998@gmail.com>
@@ -15,6 +15,7 @@ package core

import (
"context"
"github.com/tikv/pd/pkg/errs"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need to change

Signed-off-by: ZenoTan <zenotan1998@gmail.com>
Signed-off-by: ZenoTan <zenotan1998@gmail.com>
Copy link
Contributor

@nolouch nolouch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ti-srebot ti-srebot added the status/LGT1 Indicates that a PR has LGTM 1. label Sep 4, 2020
@nolouch
Copy link
Contributor

nolouch commented Sep 4, 2020

plz resolve the conflict.

ZenoTan and others added 2 commits September 4, 2020 14:51
Signed-off-by: ZenoTan <zenotan1998@gmail.com>
pkg/errs/errno.go Outdated Show resolved Hide resolved
Copy link
Member

@rleungx rleungx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The rest LGTM

@ti-srebot ti-srebot removed the status/LGT1 Indicates that a PR has LGTM 1. label Sep 4, 2020
@ti-srebot ti-srebot added the status/LGT2 Indicates that a PR has LGTM 2. label Sep 4, 2020
ZenoTan and others added 2 commits September 4, 2020 15:54
Co-authored-by: Ryan Leung <rleungx@gmail.com>
Signed-off-by: ZenoTan <zenotan1998@gmail.com>
Signed-off-by: ZenoTan <zenotan1998@gmail.com>
@rleungx
Copy link
Member

rleungx commented Sep 4, 2020

/merge

@ti-srebot ti-srebot added the status/can-merge Indicates a PR has been approved by a committer. label Sep 4, 2020
@ti-srebot
Copy link
Contributor

/run-all-tests

@ti-srebot ti-srebot merged commit 12a08b1 into tikv:master Sep 4, 2020
@ZenoTan
Copy link
Contributor Author

ZenoTan commented Sep 7, 2020

/label needs-cherry-pick-4.0

@ti-srebot ti-srebot added the needs-cherry-pick-release-4.0 The PR needs to cherry pick to release-4.0 branch. label Sep 7, 2020
ti-srebot pushed a commit to ti-srebot/pd that referenced this pull request Sep 7, 2020
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-srebot
Copy link
Contributor

cherry pick to release-4.0 in PR #2911

ti-srebot added a commit that referenced this pull request Sep 7, 2020
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
Signed-off-by: ZenoTan <zenotan1998@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution Indicates that the PR was contributed by an external member. needs-cherry-pick-release-4.0 The PR needs to cherry pick to release-4.0 branch. status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants