Skip to content

On clusterLeave, notify only if there are peers#1713

Merged
sanimej merged 1 commit intomoby:masterfrom
aboch:nse
Apr 23, 2017
Merged

On clusterLeave, notify only if there are peers#1713
sanimej merged 1 commit intomoby:masterfrom
aboch:nse

Conversation

@aboch
Copy link
Copy Markdown
Contributor

@aboch aboch commented Apr 8, 2017

  • Otherwise operation will unnecessarely block
    for five seconds.
  • This is particularly noticeable on graceful
    shutdown of daemon in one node cluster.

Related to moby/moby/issues/32446

@sanimej
Copy link
Copy Markdown

sanimej commented Apr 10, 2017

Is the error failed to send node leave: timed out broadcasting node event seen only in the single node case ?

Single node clusters are unlikely in real deployments and IMO, we shouldn't optimize for that.

@aboch
Copy link
Copy Markdown
Contributor Author

aboch commented Apr 10, 2017

I see your point, for clusters.

There sure are cases where a user might be using a single node swarm for feature development or testing. For those cases, I do not think the current is an acceptable behavior, considering this would delay things like service docker stop/restart, and docker swarm integration ci for that matter.

@aaronlehmann
Copy link
Copy Markdown

Single node clusters are unlikely in real deployments and IMO, we shouldn't optimize for that.

BTW, we are planning to change swarm mode to default to "on" in the future. The purpose will be to make features like services and secrets available without needing to run a special command first. This will make single-node installations very common.

@mavenugo
Copy link
Copy Markdown
Contributor

@aaronlehmann thats a good point.
@sanimej @aboch we have to consider the fact that single-node swarm-mode will be the most common case.

@aaronlehmann
Copy link
Copy Markdown

Is it possible to get this in for 17.05? It would be great to avoid the extra delay for graceful shutdowns.

@sanimej
Copy link
Copy Markdown

sanimej commented Apr 19, 2017

@aboch sendNodeEvent is an intentional blocking call till the event gets broadcasted. Its better to handle the special case inside of this function. Please add the check here before the wait https://github.com/docker/libnetwork/blob/master/networkdb/broadcast.go#L89.

- Otherwise operation will unnecessarely block
  for five seconds.
- This is particularly noticeable on graceful
  shutdown of daemon in one node cluster.

Signed-off-by: Alessandro Boch <aboch@docker.com>
@aboch
Copy link
Copy Markdown
Contributor Author

aboch commented Apr 21, 2017

Updated. PTAL.

@sanimej
Copy link
Copy Markdown

sanimej commented Apr 23, 2017

LGTM

@sanimej sanimej merged commit 1acf40a into moby:master Apr 23, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants