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

Respect splay for all ways a child process can get a reload signal #1671

Closed
komapa opened this issue Nov 16, 2022 · 1 comment · Fixed by #1690
Closed

Respect splay for all ways a child process can get a reload signal #1671

komapa opened this issue Nov 16, 2022 · 1 comment · Fixed by #1690
Milestone

Comments

@komapa
Copy link

komapa commented Nov 16, 2022

Consul Template version

0.29.5

Configuration

log_level = "debug"

# Setting this value to the empty string will cause consul-template
# to not listen for any reload signals but pass them down to nginx instead.
reload_signal = ""

exec {
  kill_signal = "SIGQUIT"
  reload_signal = "SIGHUP"
  kill_timeout = "5m"

  splay = "15s"

  command = ["nohup", "sleep", "100000"]
}

Expected behavior

In current release version of consul-template does not apply the splay time when a reload comes from outside of consul-template in the case when we set reload_signal = "" at the top level config. I provide a sample code that I was experimenting with just to give an idea of where the problem could be addressed.

The real end goal of what we are trying to achieve is to "debounce" reload signals but respecting the splay time (any reloads that come while the splay is ticking, should be dropped and use that very first reload to actually reload the exec process. We have a problem where our nginx process is reloading many times per second, causing stability issues (just to give more context).

References

// Signal sends the signal to the child process, returning any errors that occur.
func (c *Child) Signal(s os.Signal) error {
	c.logger.Printf("[INFO] (child) receiving signal %q", s.String())
	c.RLock()
	defer c.RUnlock()

	switch s {
	case c.reloadSignal:
		return c.reload()
	case c.killSignal:
		c.kill(true)
		return nil
	default:
		return c.signal(s)
	}
}
@eikenb
Copy link
Contributor

eikenb commented Dec 6, 2022

Hey @kirooshu, thanks for filing this.

This makes sense to me and the fix seems obvious. Convert your example code into a PR and I'll merge it.

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

Successfully merging a pull request may close this issue.

2 participants