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

etcdserver: clear message in cluster version decision #10159

Merged
merged 1 commit into from
Oct 10, 2018

Conversation

gyuho
Copy link
Contributor

@gyuho gyuho commented Oct 8, 2018

Only leader can decide cluster version.
Clarify the logging that this local node is the leader.

Signed-off-by: Gyuho Lee leegyuho@amazon.com

@codecov-io
Copy link

codecov-io commented Oct 8, 2018

Codecov Report

Merging #10159 into master will decrease coverage by 0.06%.
The diff coverage is 40%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #10159      +/-   ##
==========================================
- Coverage    71.5%   71.44%   -0.07%     
==========================================
  Files         390      390              
  Lines       36306    36306              
==========================================
- Hits        25962    25940      -22     
- Misses       8524     8536      +12     
- Partials     1820     1830      +10
Impacted Files Coverage Δ
etcdserver/cluster_util.go 58.74% <40%> (ø) ⬆️
auth/store.go 67.68% <0%> (-7.85%) ⬇️
pkg/fileutil/purge.go 65.9% <0%> (-6.82%) ⬇️
embed/config_logging.go 34.13% <0%> (-6.59%) ⬇️
pkg/testutil/recorder.go 77.77% <0%> (-3.71%) ⬇️
clientv3/leasing/txn.go 88.09% <0%> (-3.18%) ⬇️
lease/leasehttp/http.go 61.02% <0%> (-2.95%) ⬇️
etcdserver/api/v3rpc/lease.go 67.04% <0%> (-2.28%) ⬇️
etcdctl/ctlv3/command/printer_simple.go 72.48% <0%> (-1.35%) ⬇️
etcdserver/api/v3rpc/watch.go 82.35% <0%> (-0.66%) ⬇️
... and 13 more

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 b046a37...59dd78d. Read the comment docs.

@@ -203,7 +203,7 @@ func decideClusterVersion(lg *zap.Logger, vers map[string]*version.Versions) *se
if lv.LessThan(*v) {
if lg != nil {
lg.Warn(
"local etcd version is not up-to-date",
"leader found higher-versioned peer",
Copy link
Contributor

Choose a reason for hiding this comment

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

higher-visioned member. we only use peer in raft layer.

Copy link
Contributor

Choose a reason for hiding this comment

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

zap.String also needs to be fixed.

Only leader can decide cluster version.
Clarify the logging that this local node is the leader.

Signed-off-by: Gyuho Lee <leegyuho@amazon.com>
@xiang90
Copy link
Contributor

xiang90 commented Oct 10, 2018

lgtm

@gyuho gyuho merged commit 0f0919c into etcd-io:master Oct 10, 2018
@gyuho gyuho deleted the version-log branch October 10, 2018 01:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants