-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
consul reload - watch configs leaks file descriptors #3018
Comments
@dhv Do you see the same behavior with a later version 0.7.[2345] or 0.8.[01] ? |
@magiconair I reproduced this with v0.8.2 and 3 watches. Connections are still building up.
|
@dhv thanks for the report. I reproduced the above, and also noticed that the old watch handlers still stay alive. So in addition to the leaky file handles, it also executes old watch handlers. I had one watch handler sending output to stdout, and on three reloads after I updated the key being watched I saw the same output to stdout thrice, indicating that out of scope old watch handlers were still alive and triggered. This is a good bug find! |
This should be fixed by #3189 - the reload handler now properly stops old watches before registering new ones. @preetapan do you have any concerns before we close this out? |
Let's leave this one open for now, I want to reproduce the file handle leak again. On Friday I saw it even after the fix in #3189(except your last two commits). I think there's more to do inside the watch handler func to make sure and close file descriptors correctly. This is an older bug, and was happening before the regressions fixed by #3189 |
Fixed by PR #3048 . Note that established connections on the server take up to a minute to close after "consul reload" after the client disconnect their previously established watch connections |
This problem is present again in Consul versions 1.0.1+. Event watch connections leak until agent service runs out of file descriptors. No leaks after downgrading to 1.0.0. $ consul version $ consul info |
@toomasp can you open a new issue with full details of what you are seeing and a link here? It's hard to conduct new investigation on an old PR - we didn't revert the change to my knowledge so it's likely to be an unrelated or subtly different code path causing your issue. |
consul version
for both Client and ServerClient:
0.7.1
Server:
0.7.1
consul info
for both Client and ServerClient:
Server:
Operating system and Environment details
ubuntu 12.04 precise
Description of the Issue (and unexpected/desired result)
consul reload
, consul opens all new connections per watch, without closing the old connectionsFrom what I can tell the connections remain and do not get garbage collected.
I looked briefly at the source through the code and found that this is likely because the watch plan uses consulapi with
cleanhttp.DefaultPooledTransport()
to use the same connection per watch. Is there a way to close these connections onconsul reload
when the watch plan gets replaced?https://github.com/hashicorp/consul/blob/master/watch/plan.go
https://github.com/hashicorp/go-cleanhttp/blob/master/cleanhttp.go#L19
Reproduction steps
The text was updated successfully, but these errors were encountered: