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

avoid loop without yield when count of sockets is too high #138

Merged
merged 1 commit into from
Oct 5, 2019
Merged

Conversation

gawen
Copy link
Contributor

@gawen gawen commented Oct 3, 2019

For Python 2.x, sleep_interval is 0 if len(self.sockets) is above
self.ping_timeout (default: 60). With eventlet, self.sleep(0) looks it does not
yield to the event loop, queueing work which is not processed. In this
condition, the symptom is the server using 100% of CPU after some time, even
when count of sockets drop below len(self.sockets).

This fix forces sleep_interval to be a float, not equal to 0, forcing eventlet
to yield, processing the queued work and avoiding CPU to be at 100%.

This fix does nothing for Python 3.x.

For Python 2.x, sleep_interval is 0 if len(self.sockets) is above
self.ping_timeout (default: 60). With eventlet, self.sleep(0) looks it
does not yield to the event loop, queueing work which is not processed.
In this condition, the symptom is the server using 100% of CPU after
some time, even when count of sockets drop below self.sockets.

This fix forces sleep_interval to be a float, not equal to 0, forcing
eventlet to yield, processing the queued work and avoiding CPU to be at
100%.

This fix does nothing for Python 3.x.
@gawen gawen changed the title avoid loop without yield when sockets are >60 avoid loop without yield when count of sockets are >60 Oct 3, 2019
@gawen gawen changed the title avoid loop without yield when count of sockets are >60 avoid loop without yield when count of sockets is too high Oct 3, 2019
@miguelgrinberg miguelgrinberg self-assigned this Oct 3, 2019
@miguelgrinberg
Copy link
Owner

I'll test this. I find it odd that eventlet does not release the CPU with sleep(0), even if it is only Python 2.

@maximebf
Copy link

maximebf commented Oct 3, 2019

I can confirm that this fixes the issue we were having with 100% CPU usage.

Here is our load graph before and after applying the patch:
image

(The load was 100% confirmed to be from the python socketio process)

@miguelgrinberg
Copy link
Owner

So how is this not a bug with eventlet then?

@gawen
Copy link
Contributor Author

gawen commented Oct 3, 2019

I find it odd that eventlet does not release the CPU with sleep(0), even if it is only Python 2.

To be fair, this is my interpretation, and this might be incorrect.

So how is this not a bug with eventlet then?

It is my understanding that sleep_interval should be a float. Making so for Python 2 fixes the 100% CPU loop which never ceases when len(self.sockets) is too high.

@miguelgrinberg
Copy link
Owner

I thought about this and even though I think there is something fishy going on with eventlet, this is a good change, so I'm merging.

@miguelgrinberg miguelgrinberg merged commit 5c6fbea into miguelgrinberg:master Oct 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants