Repository navigation
selector.EpollSelector: EPOLLEXCLUSIVE, round 2 #89114
Description
| BPO | 44951 |
|---|---|
| Nosy | @dgilman |
| PRs |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
assignee = None
closed_at = None
created_at = <Date 2021-08-19.02:49:09.107>
labels = ['library']
title = 'selector.EpollSelector: EPOLLEXCLUSIVE, round 2'
updated_at = <Date 2021-08-19.12:18:04.697>
user = 'https://github.com/dgilman'bugs.python.org fields:
activity = <Date 2021-08-19.12:18:04.697>
actor = 'David.Gilman'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)']
creation = <Date 2021-08-19.02:49:09.107>
creator = 'David.Gilman'
dependencies = []
files = []
hgrepos = []
issue_num = 44951
keywords = ['patch']
message_count = 3.0
messages = ['399880', '399881', '399912']
nosy_count = 1.0
nosy_names = ['David.Gilman']
pr_nums = ['27819']
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = None
url = 'https://bugs.python.org/issue44951'
versions = []
Activity
dgilman commentedon Aug 19, 2021
Note that this is a different approach from the one taken in https://bugs.python.org/issue35517 although the issue is still the same.
I've written a patch that allows users of selector.EpollSelector to enable EPOLLEXCLUSIVE on their file descriptors. This PR adds a setter and read only property to only the EpollSelector class instead of trying to expand the entire selector API like the other patch. The other discussion mentioned that there are some useful flags that could be passed down like this one. If other useful behavioral flags emerged in the future I think they should get their own API similar to how I've done it here. However, the other flags available so far for epoll are not useful for the selector module: EPOLLONESHOT and EPOLLET are incompatible with the design of the selector API and EPOLLWAKEUP is only marginally useful, not even getting exported into the select module after nearly a decade (Linux 3.5 was released in 2012).
My API uses a getter/method instead of a read/write property because my understanding is that property access shouldn't raise exceptions, but if that doesn't matter here, it could be a read/write property.
Justification:
First, this is a useful flag that improves performance of epoll under even moderate load. I was going to turn it on by default in this patch but unfortunately Linux prevents you from doing epoll_mod() on anything that has EPOLLEXCLUSIVE set on it, breaking the python-level API. With this patch if you try to modify() after EPOLLEXCLUSIVE is set you'll get an EINVAL but I think the trade-off here is worth it. You don't enable EPOLLEXCLUSIVE on accident and you're reading the manpage for EPOLLEXCLUSIVE where this exact behavior is mentioned before turning anything on, right? And of course the Python docs also warn you about modify().
Second, the thundering herd problem solved by EPOLLEXCLUSIVE is somewhat of a sore spot for Python's PR. In the past year two widely disseminated articles have brought up this issue. This PR isn't going to be a silver bullet however it can make a huge impact in gunicorn, the 3rd party library mentioned in both articles. Gunicorn is a popular WSGI web server and its gthread worker (not the default but the one most often used in production) directly uses the selector module from the standard library. Honestly, it's pretty cool that they were able to make such efficient use of a standard library module like this - how far we've come from the days of asynchat! There is nothing in gunicorn's threaded worker that calls modify() so there would be no API breakage there.
Gunicorn thundering herd articles:
https://blog.clubhouse.com/reining-in-the-thundering-herd-with-django-and-gunicorn/
https://rachelbythebay.com/w/2020/03/07/costly/
dgilman commentedon Aug 19, 2021
I also played with making another whole subclass that has it on by default, see this package https://github.com/dgilman/selector-epoll-exclusive
That class could have EPOLLEXCLUSIVE on by default but could raise NotImplemented if you try and modify() it. Putting it in as a second class means you can also drop it into existing code unmodified, something that will play very nicely with all existing users of selector.
dgilman commentedon Aug 19, 2021
Reflecting on this a bit I wonder if the best of all worlds is to have an EpollExclusiveSelector whose modify() implementation just unregisters and registers the file descriptor.