-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Start health checks early. #1296
Conversation
1a25f87
to
51b6e5e
Compare
@containous/traefik PTAL. |
Do not wait a full tick cycle to execute the first health check. Additional changes: - Make request timeout configurable (for testing purposes). - Support synchronizing on health check goroutine termination through an internal wait group (for testing purposes). - Stop leaking by closing the HTTP response body. - Extend health check logging and use WARNING level for (continuously) failing health checks.
51b6e5e
to
9cb5d9a
Compare
return &BackendHealthCheck{ | ||
URL: URL, | ||
Interval: interval, | ||
requestTimeout: 5 * time.Second, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is a multiplier of 5 chosen here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's Go's way of saying "5 seconds".
(Technically, durations are just float type aliases.)
5 secs was the default before, so I kept it but made it configurable for testing purposes.
defer hc.wg.Done() | ||
log.Debugf("Initial healthcheck for currentBackend %s ", currentBackendID) | ||
checkBackend(currentBackend) | ||
ticker := time.NewTicker(currentBackend.Interval) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Notice that I moved the ticker up. Didn't make sense to reinitialize the ticket inside the loop.
I'm closing this in favor of #1319, which is the exact same branch but hosted on the containous repo; the reasoning being that I have another PR based on this branch waiting in line, which I want to push and base onto this branch now. |
Do not wait a full tick cycle to execute the first healh check.
Additional changes where (mostly) driven by the need for testability: