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

*: add --initial-election-tick-advance to configure election fast-forward on bootstrap #9591

Merged
merged 6 commits into from
Apr 23, 2018

Conversation

gyuho
Copy link
Contributor

@gyuho gyuho commented Apr 19, 2018

Fix #9333.

I can reproduce by slowing down network from leader to rejoining follower. --initial-elecition-tick-advance=true is the default behavior, so user won't see any difference. Details are explained in change log and godoc.

/cc @jpbetz @wojtek-t @mborsz

@gyuho gyuho mentioned this pull request Apr 19, 2018
18 tasks
@gyuho gyuho changed the title *: add --initial-elecition-tick-advance to configure election fast-forward on bootstrap *: add --initial-election-tick-advance to configure election fast-forward on bootstrap Apr 20, 2018
gyuho added 6 commits April 19, 2018 17:45
Signed-off-by: Gyuho Lee <gyuhox@gmail.com>
Signed-off-by: Gyuho Lee <gyuhox@gmail.com>
Signed-off-by: Gyuho Lee <gyuhox@gmail.com>
Signed-off-by: Gyuho Lee <gyuhox@gmail.com>
Signed-off-by: Gyuho Lee <gyuhox@gmail.com>
Signed-off-by: Gyuho Lee <gyuhox@gmail.com>
@codecov-io
Copy link

Codecov Report

Merging #9591 into master will decrease coverage by 0.02%.
The diff coverage is 91.3%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #9591      +/-   ##
=========================================
- Coverage   69.33%   69.3%   -0.03%     
=========================================
  Files         369     369              
  Lines       33706   33717      +11     
=========================================
- Hits        23371   23369       -2     
- Misses       8632    8635       +3     
- Partials     1703    1713      +10
Impacted Files Coverage Δ
integration/cluster.go 82.03% <100%> (+0.03%) ⬆️
embed/etcd.go 65.32% <100%> (+0.08%) ⬆️
etcdmain/config.go 82.15% <100%> (+0.08%) ⬆️
embed/config.go 56.41% <100%> (+0.1%) ⬆️
etcdserver/config.go 85.41% <100%> (+0.1%) ⬆️
etcdserver/server.go 70.7% <33.33%> (+0.28%) ⬆️
pkg/adt/interval_tree.go 85.28% <0%> (-6.61%) ⬇️
proxy/grpcproxy/watch.go 86.33% <0%> (-4.35%) ⬇️
mvcc/watcher.go 96.29% <0%> (-3.71%) ⬇️
etcdserver/api/v3rpc/lease.go 63.95% <0%> (-2.33%) ⬇️
... and 19 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 b933603...2d7cb9d. Read the comment docs.

@mborsz
Copy link
Contributor

mborsz commented Apr 20, 2018

Cool, thanks for preparing the PR! :)

Few questions:
Does it need to be '--initial-' flag (set on the cluster bootstrap)?
How can I change advanceElectionTick behavior for already existing clusters?

@gyuho
Copy link
Contributor Author

gyuho commented Apr 20, 2018

I meant to say it's only for initial election process (on boot). Advancing tick only happens when a node starts. If it's not clear, I'm open for better naming :)

How can I change advanceElectionTick behavior for already existing clusters?

This would need to be backported to 3.1, and existing cluster needs to upgrade to latest release with this patch, and set --initial-election-tick-advance=false.

@mborsz
Copy link
Contributor

mborsz commented Apr 20, 2018

IIUC https://coreos.com/etcd/docs/latest/op-guide/configuration.html#clustering-flags says that flags with '--initial-' prefix are ignored when restarting already existing member (with existing --data-dir dir?).

This flag has '--initial' prefix, so I'm wondering if that comment applies here.
If I change this flag for a member that already exists (i.e. it --data-dir exists, but the etcd process was restarted), will the change be effective?

Thanks

@gyuho
Copy link
Contributor Author

gyuho commented Apr 20, 2018

Ah, that could be confusing. That makes sense only for clustering flag. I will make sure to clarify that doc.
We have other flags (e.g. --initial-corrupt-check) as well, so --initial-* should be fine, as long as we document clearly.

If I change this flag for a member that already exists (i.e. it --data-dir exists, but the etcd process was restarted), will the change be effective?

Yes, it should just work. This flag won't be affected by existing data. --initial-election-tick-advance=false should just disable fast-forward regardless, once backported.

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