Skip to content

Increase size of buffer for signal forwarding with --sig-proxy #6508

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

Merged
merged 1 commit into from
Jul 7, 2014

Conversation

mheon
Copy link
Contributor

@mheon mheon commented Jun 18, 2014

Sending signals rapidly to a container (for example, through a script) frequently results in some signals being dropped. Root cause of this seems to be the chan for holding signals to be forwarded has a buffer size of 1, and if 2 or more new signals arrived while one was being forwarded, signals were lost. Increasing the buffer should fix this.

The value 128 here is somewhat arbitrary, but seems reasonable for normal use cases. Open to adjusting this up or down.

The chan struct used to forward signals to containers was one element only,
which caused some signals to be dropped when many were being received.
Increasing the size of the chan buffer makes this much less likely to happen.

Docker-DCO-1.1-Signed-off-by: Matt Heon <mheon@redhat.com> (github: mheon)
@rhatdan
Copy link
Contributor

rhatdan commented Jun 18, 2014

With this fix does the signal test work?

@mheon
Copy link
Contributor Author

mheon commented Jun 18, 2014

Depends on how many signals you wish to send. It's definitely still possible to drop signals - at least on my machine, Docker takes longer to forward the signal than it does for another process to send a new signal, so it's still possible to overflow the buffer with a continuous stream of signals. However, I don't know of a use case where this is necessary. The buffer is there in case a large number of signals arrive in a brief period of time.

@crosbymichael
Copy link
Contributor

How are the signals being lost? they are just buffered and should be delivered as the chan is drained

@mheon
Copy link
Contributor Author

mheon commented Jun 19, 2014

You can see the relevant code here: https://github.com/dotcloud/docker/blob/master/api/client/commands.go#L538

Signals arrive in the buffer, and are then pulled off by the goroutine which does a bit of processing, then forwards the signal. The issue arises when two or more signals arrive during that goroutine - as the chan was initialized with size 1, anything after the first one will be lost in this case.

I can consistently make this happen with a simple Bash script calling kill in a loop - a few signals get through, but the vast majority are lost. Increasing the buffer size fixes this issue

@tiborvass
Copy link
Contributor

@crosbymichael: @mheon seems right:
From http://godoc.org/os/signal#Notify

Package signal will not block sending to c: the caller must ensure that c has sufficient buffer space to keep up with the expected signal rate. For a channel used for notification of just one signal value, a buffer of size 1 is sufficient.

We could either set the channel buffer size to a little-higher value and/or have (again) some option... (server-side I guess but one could argue it could also be on docker run).

@unclejack
Copy link
Contributor

LGTM

ping @crosbymichael @vieux @tiborvass

@vieux
Copy link
Contributor

vieux commented Jul 7, 2014

LGTM

@vieux
Copy link
Contributor

vieux commented Jul 7, 2014

ping @crosbymichael

@crosbymichael
Copy link
Contributor

LGTM

crosbymichael pushed a commit that referenced this pull request Jul 7, 2014
Increase size of buffer for signal forwarding with --sig-proxy
@crosbymichael crosbymichael merged commit 860c664 into moby:master Jul 7, 2014
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.

6 participants