-
Notifications
You must be signed in to change notification settings - Fork 781
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
Don't wait for splay when stopping child runner #1141
Don't wait for splay when stopping child runner #1141
Conversation
When consul-template itself receives its kill signal, and tries to gracefully shutdown the child runner, it actually waits for splay to occur before shutting down the child. This is undesireable behaviour because the splay is intended to protect against stampeding herds of simultaneous child _starts_, not _stops_.
Having a second look at this I think I need to rethink this - |
We probably _do_ want to wait for the splay when the reload signal is received, just not the kill signal.
So I pushed the choice as to whether to wait for splay or not all the way up to the CLI module, so that we can have different behaviour for the reload and kill signal. It will also make it easier to gate the behaviour on a flag if that's what we want to do. |
Hey @KJTsanaktsidis, since you submitted your pull request we've implemented a policy of requiring a signed CLA before merging. Would you please consider signing the CLA? Thanks. |
@eikenb Is it possible for our organization to sign the CLA on behalf of the organization to cover this PR instead of having the individual developers sign the CLA? |
Hey @plonergan. I asked and it seems that isn't possible. Githubs CLA tech is oriented strictly towards individual contributors, not organizations, and we geared are CLA toward that. Ie. it is not possible technically and the document reflects that. Is the problem that @KJTsanaktsidis left your organization since submitting this? |
Hey there! No, was just away for a couple of weeks :) I’ll have a look at
this in the next couple of days (I’ll also make sure the PR is still
relevant).
…On Wed, 26 Jun 2019 at 5:33 am, John Eikenberry ***@***.***> wrote:
Hey @plonergan <https://github.com/plonergan>. I asked and it seems that
isn't possible. Githubs CLA tech is oriented strictly towards individual
contributors, not organizations, and we geared are CLA toward that. Ie. it
is not possible technically and the document reflects that.
Is the problem that @KJTsanaktsidis <https://github.com/KJTsanaktsidis>
left your organization since submitting this?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1141?email_source=notifications&email_token=AAK2HQKIXXSMPOCEGNHJTQ3P4JXJJA5CNFSM4FWNX2FKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODYRLECQ#issuecomment-505590282>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAK2HQP7GLFOBZUJDIT272LP4JXJJANCNFSM4FWNX2FA>
.
|
I resolved conflicts and signed the CLA - @plonergan said that we've agreed to a corporate CLA but I needed to click the buttons in Github to make all the machinery work, so I've gone ahead and done that. |
When consul-template itself receives its kill signal, and tries to gracefully shutdown the child runner, it actually waits for splay to occur before shutting down the child. This is, in my opinion, undesireable behaviour because the splay is intended to protect against stampeding herds of simultaneous child starts, not stops.
My particular use case is that I run consul-template as a parent in a Kubernetes container; when doing a deployment, the old pods take a long time to shut down because of the splay timeout. However, there's no need to wait for the splay in this case, because Kubernetes itself is not restarting all instances of the container at the same time.
If the original behaviour is actually intended or desired, I'm happy to put this change behind a flag as well.