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

Close server connections and shutdown coroutines immediately on disconnect #7209

Merged
merged 1 commit into from
Jun 5, 2019

Conversation

Al2Klimov
Copy link
Member

No description provided.

@Al2Klimov Al2Klimov requested a review from dnsmichi May 29, 2019 09:11
@Al2Klimov
Copy link
Member Author

Theoretically ready, but I'd like to

@dnsmichi dnsmichi added this to the 2.11.0 milestone May 29, 2019
@dnsmichi dnsmichi added area/api REST API area/distributed Distributed monitoring (master, satellites, clients) labels May 29, 2019
@Al2Klimov Al2Klimov self-assigned this Jun 5, 2019
@Al2Klimov Al2Klimov removed the request for review from dnsmichi June 5, 2019 07:43
@Al2Klimov Al2Klimov force-pushed the bugfix/immediately-close-sockets branch from dee4c63 to a3f04be Compare June 5, 2019 08:34
@Al2Klimov Al2Klimov force-pushed the bugfix/immediately-close-sockets branch from a3f04be to ad28380 Compare June 5, 2019 08:42
@Al2Klimov Al2Klimov changed the title Close server connections immediately on disconnect Close server connections and shutdown coroutines immediately on disconnect Jun 5, 2019
@Al2Klimov Al2Klimov removed their assignment Jun 5, 2019
@Al2Klimov Al2Klimov requested a review from dnsmichi June 5, 2019 08:43
@dnsmichi
Copy link
Contributor

dnsmichi commented Jun 5, 2019

Hmmm if I overload the API with many requests, and then run into too many open files, the sockets will stay in CLOSE_WAIT - but only after it heals itself with closing requests then.

This PR

A typical test case with 100 parallel connections and 5 curl requests works just fine.

michi@mbpmif ~ $ pidof icinga2
13115 13129
michi@mbpmif ~ $ watch -n 1 -c 'lsof -p 13115 | grep TCP | wc -l && lsof -p 13115 | grep TCP'
for i in {1..100}; do sh -c "curl -k -s -u root:icinga 'https://localhost:5665/v1/objects/services?[1-5]' >/dev/null &"; done

At first, this starts with 100+1.

Screen Shot 2019-06-05 at 14 20 52

Then it jumps over to ~200 with sockets in close_wait and the next curl request.

Screen Shot 2019-06-05 at 14 21 30

After a while all requests are finished and the only socket remaining is the listening socket.

Screen Shot 2019-06-05 at 14 22 57

@dnsmichi
Copy link
Contributor

dnsmichi commented Jun 5, 2019

2.10.2 doesn't leak either. The accepts there are faster, pushing the thread pool and my macbook to the limits. So it is good that we take care of this, preventing strange customer problems.

@dnsmichi dnsmichi merged commit 18211dd into master Jun 5, 2019
@dnsmichi dnsmichi deleted the bugfix/immediately-close-sockets branch June 5, 2019 12:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/api REST API area/distributed Distributed monitoring (master, satellites, clients)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants