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

client.go: Degrade log level when passing unavailable endpoint to client #1852

Merged
merged 2 commits into from
Oct 28, 2019

Conversation

july2993
Copy link
Contributor

What problem does this PR solve?

when some endpoint passing into the client is unavailable
in calling NewClient() will log the error log, but the service is
supported to be still available if one of the endpoints can work.
error level log will be too serious and confuse the user.

What is changed and how it works?

client.go: Degrade log level when passing unavailable endpoint to client

Check List

Tests

  • Manual test (add detailed scripts or steps below)
    log after this pr:
[2019/10/28 18:51:04.975 +08:00] [INFO] [client.go:144] ["[pd] create pd client with endpoints"] [pd-address="[localhost:9999,localhost:2379]"]
[2019/10/28 18:51:04.987 +08:00] [WARN] [client.go:200] ["[pd] failed to get cluster id"] [url=http://localhost:9999] [error="rpc error: code = Unavailable desc = all SubConns are in TransientFailure, latest connection error: connection error: desc = \"transport: Error while dialing dial tcp [::1]:9999: connect: connection refused\""] [errorVerbose="rpc error: code = Unavailable desc = all SubConns are in TransientFailure, latest connection error: connection error: desc = \"transport: Error while dialing dial tcp [::1]:9999: connect: connection refused\"\ngithub.com/pingcap/pd/client.(*client).getMembers\n\t/Users/huangjiahao/go/pkg/mod/github.com/july2993/pd@v1.1.0-beta.0.20191028103916-ec504e3dce8f/client/client.go:235\ngithub.com/pingcap/pd/client.(*client).initClusterID\n\t/Users/huangjiahao/go/pkg/mod/github.com/july2993/pd@v1.1.0-beta.0.20191028103916-ec504e3dce8f/client/client.go:197\ngithub.com/pingcap/pd/client.(*client).initRetry\n\t/Users/huangjiahao/go/pkg/mod/github.com/july2993/pd@v1.1.0-beta.0.20191028103916-ec504e3dce8f/client/client.go:184\ngithub.com/pingcap/pd/client.NewClient\n\t/Users/huangjiahao/go/pkg/mod/github.com/july2993/pd@v1.1.0-beta.0.20191028103916-ec504e3dce8f/client/client.go:157\nmain.main\n\t/Users/huangjiahao/go/src/github.com/pingcap/tidb-binlog/cmd/pd.go:10\nruntime.main\n\t/usr/local/go/src/runtime/proc.go:203\nruntime.goexit\n\t/usr/local/go/src/runtime/asm_amd64.s:1357"]
[2019/10/28 18:51:04.994 +08:00] [INFO] [client.go:252] ["[pd] switch leader"] [new-leader=http://127.0.0.1:2379] [old-leader=]
[2019/10/28 18:51:04.994 +08:00] [INFO] [client.go:163] ["[pd] init cluster id"] [cluster-id=6752802479515318396]

Code changes

Side effects

Related changes

  • Need to cherry-pick to the release branch

when some endpoint passing into the client is unavailable
in calling NewClient() will log the error log, but the service is
supported to be still available if one of the endpoint can work.
error level log will be too serious and confuse user.
@CLAassistant
Copy link

CLAassistant commented Oct 28, 2019

CLA assistant check
All committers have signed the CLA.

@codecov-io
Copy link

codecov-io commented Oct 28, 2019

Codecov Report

Merging #1852 into master will decrease coverage by 0.09%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #1852     +/-   ##
=========================================
- Coverage    78.1%   78.01%   -0.1%     
=========================================
  Files         168      168             
  Lines       17019    17019             
=========================================
- Hits        13293    13277     -16     
- Misses       2644     2654     +10     
- Partials     1082     1088      +6
Impacted Files Coverage Δ
client/client.go 69.89% <0%> (-0.44%) ⬇️
server/kv/etcd_kv.go 72.72% <0%> (-9.1%) ⬇️
server/schedulers/random_merge.go 58.13% <0%> (-4.66%) ⬇️
server/tso/tso.go 76.11% <0%> (-1.5%) ⬇️
server/grpc_service.go 58.78% <0%> (-0.66%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 628ac1b...a83af5f. Read the comment docs.

@july2993 july2993 added the status/can-merge Indicates a PR has been approved by a committer. label Oct 28, 2019
@sre-bot
Copy link
Contributor

sre-bot commented Oct 28, 2019

/run-all-tests

@sre-bot
Copy link
Contributor

sre-bot commented Oct 28, 2019

@july2993 merge failed.

@lhy1024
Copy link
Contributor

lhy1024 commented Oct 28, 2019

/merge

@sre-bot
Copy link
Contributor

sre-bot commented Oct 28, 2019

Your auto merge job has been accepted, waiting for 1851

@sre-bot
Copy link
Contributor

sre-bot commented Oct 28, 2019

/run-all-tests

@sre-bot sre-bot merged commit d2de9ac into tikv:master Oct 28, 2019
@july2993 july2993 added needs-cherry-pick-release-2.1 The PR needs to cherry pick to release-2.1 branch. needs-cherry-pick-release-3.0 The PR needs to cherry pick to release-3.0 branch. needs-cherry-pick-release-3.1 The PR needs to cherry pick to release-3.1 branch. labels Oct 29, 2019
@july2993 july2993 deleted the degrade_log branch October 29, 2019 03:29
@sre-bot
Copy link
Contributor

sre-bot commented Oct 29, 2019

cherry pick to release-3.0 in PR #1856

@sre-bot
Copy link
Contributor

sre-bot commented Oct 29, 2019

cherry pick to release-2.1 failed

@sre-bot
Copy link
Contributor

sre-bot commented Oct 29, 2019

cherry pick to release-3.1 in PR #1857

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-cherry-pick-release-2.1 The PR needs to cherry pick to release-2.1 branch. needs-cherry-pick-release-3.0 The PR needs to cherry pick to release-3.0 branch. needs-cherry-pick-release-3.1 The PR needs to cherry pick to release-3.1 branch. status/can-merge Indicates a PR has been approved by a committer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants