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

Expose swarmkit's Raft tuning parameters in engine config #36726

Merged
merged 1 commit into from Mar 30, 2018

Conversation

@chungers
Contributor

chungers commented Mar 29, 2018

Signed-off-by: David Chung david.chung@docker.com

- What I did
This PR exposes the HeartbeatTick and ElectionTick parameters for Swarmkit's Raft quorum. This allows tuning of the leader election behavior of the Swarm managers. This PR contains default values so these settings are strictly optional. Changing these settings can adjust the sensitivity of Swarm managers to transient events in their environment and make the quorum more stable.

- How I did it
Swarmkit already exposes these parameters but they are previously hardcoded. This PR plumbs these settings through to expose them in the daemon's config.

- How to verify it
Changing the daemon's config JSON should override the default values.

- Description for the changelog

Make Swarm manager Raft quorum parameters configurable in daemon config.

- A picture of a cute animal (not mandatory but encouraged)

David Chung
Expose swarm raft tuning parameters in engine config
Signed-off-by: David Chung <david.chung@docker.com>
@codecov

This comment has been minimized.

codecov bot commented Mar 29, 2018

Codecov Report

❗️ No coverage uploaded for pull request base (master@18d1688). Click here to learn what that means.
The diff coverage is 0%.

@@            Coverage Diff            @@
##             master   #36726   +/-   ##
=========================================
  Coverage          ?    35.2%           
=========================================
  Files             ?      613           
  Lines             ?    45562           
  Branches          ?        0           
=========================================
  Hits              ?    16038           
  Misses            ?    27396           
  Partials          ?     2128
@boaz1337

LGTM 👍

@vdemeester

SGTM

@stevvooe

This comment has been minimized.

Contributor

stevvooe commented Mar 29, 2018

LGTM

1 similar comment
@tonistiigi

This comment has been minimized.

Member

tonistiigi commented Mar 30, 2018

LGTM

@tonistiigi tonistiigi merged commit 8b6a827 into moby:master Mar 30, 2018

7 of 8 checks passed

codecov/patch 0% of diff hit (target 50%)
Details
codecov/project No report found to compare against
Details
dco-signed All commits are signed
experimental Jenkins build Docker-PRs-experimental 40033 has succeeded
Details
janky Jenkins build Docker-PRs 48755 has succeeded
Details
powerpc Jenkins build Docker-PRs-powerpc 9211 has succeeded
Details
windowsRS1 Jenkins build Docker-PRs-WoW-RS1 20224 has succeeded
Details
z Jenkins build Docker-PRs-s390x 9170 has succeeded
Details
@thaJeztah

This comment has been minimized.

Member

thaJeztah commented Mar 30, 2018

Looks like this needs an update in the docs, also;

  • should this be exposed as a flag as well?
  • is this configuration live-reloadable?
}
if config.RaftElectionTick == 0 {
// 10X heartbeat tick is the recommended ratio according to etcd docs.
config.RaftElectionTick = 10 * config.RaftHeartbeatTick

This comment has been minimized.

@thaJeztah

thaJeztah Apr 5, 2018

Member

What happens if I configure RaftElectionTick < RaftHeartbeatTick? Should RaftElectionTick always be multiple times RaftHeartbeatTick? If so; should we add validation here, or have a ratio instead as configuration?

This comment has been minimized.

@chungers

chungers Apr 10, 2018

Contributor

This here doesn't enforce that RaftElectionTick should always be > than RaftHearbeatTick and should be at least 10 (which is the default value when nothing is set).

I can add a check to return an error when RaftElectionTick is strictly greater than RaftHeartbeatTick and prevent the daemon from starting up. Is this acceptable?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment