selector.EpollSelector: add new parameter to support EPOLLEXCLUSIVE

[Zheaoli]

BPO	35517
Nosy	@vstinner, @giampaolo, @1st1, @zhangyangyu, @Zheaoli
PRs	gh-79698: selector.EpollSelector: add new parameter to support extra events #11193

^{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 2018-12-17.10:31:38.448>
labels = ['3.8', 'type-feature', 'library']
title = 'selector.EpollSelector: add new parameter to support EPOLLEXCLUSIVE'
updated_at = <Date 2019-01-30.14:43:45.137>
user = 'https://github.com/Zheaoli'

bugs.python.org fields:

activity = <Date 2019-01-30.14:43:45.137>
actor = 'Manjusaka'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)']
creation = <Date 2018-12-17.10:31:38.448>
creator = 'Manjusaka'
dependencies = []
files = []
hgrepos = []
issue_num = 35517
keywords = ['patch']
message_count = 20.0
messages = ['331969', '331970', '331971', '331972', '331973', '331974', '331977', '331979', '331991', '331994', '331996', '331998', '331999', '332000', '332017', '332018', '332019', '332038', '332039', '334562']
nosy_count = 6.0
nosy_names = ['vstinner', 'giampaolo.rodola', 'neologix', 'yselivanov', 'xiang.zhang', 'Manjusaka']
pr_nums = ['11193']
priority = 'normal'
resolution = None
stage = None
status = 'open'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue35517'
versions = ['Python 3.8']

[Zheaoli]

Add a keyword argument for selector.EpollSelector with default value.

This can help people use the EPOLLEXCLUSIVE since Python 3.7 and Linux Kernel 4.5 to avoid the herd effect

like this

 def register(self, fileobj, events, data=None, exclusive=False):
 key = super().register(fileobj, events, data)
 epoll_events = 0
 if events & EVENT_READ:
 epoll_events |= select.EPOLLIN
 if events & EVENT_WRITE:
 epoll_events |= select.EPOLLOUT
 try:
 if exclusive and hasattr(select, "EPOLLEXCLUSIVE"):
 epoll_events |= select.EPOLLEXCLUSIVE
 self._epoll.register(key.fd, epoll_events)
 except BaseException:
 super().unregister(fileobj)
 raise
 return key

[vstinner]

Add a keyword argument for selector.EpollSelector with default value.

I suggest to use a keyword-only parameter:

def register(self, fileobj, events, data=None, *, exclusive=False):

Do you want to work on a pull request?

[vstinner]

EPOLLEXCLUSIVE doc from Linux manual page:

"""
EPOLLEXCLUSIVE (since Linux 4.5)

Sets an exclusive wakeup mode for the epoll file descriptor that
is being attached to the target file descriptor, fd. When a
wakeup event occurs and multiple epoll file descriptors are
attached to the same target file using EPOLLEXCLUSIVE, one or
more of the epoll file descriptors will receive an event with
epoll_wait(2). The default in this scenario (when EPOLLEXCLU‐
SIVE is not set) is for all epoll file descriptors to receive an
event. EPOLLEXCLUSIVE is thus useful for avoiding thundering
herd problems in certain scenarios.

If the same file descriptor is in multiple epoll instances, some
with the EPOLLEXCLUSIVE flag, and others without, then events
will be provided to all epoll instances that did not specify
EPOLLEXCLUSIVE, and at least one of the epoll instances that did
specify EPOLLEXCLUSIVE.

The following values may be specified in conjunction with
EPOLLEXCLUSIVE: EPOLLIN, EPOLLOUT, EPOLLWAKEUP, and EPOLLET.
EPOLLHUP and EPOLLERR can also be specified, but this is not
required: as usual, these events are always reported if they
occur, regardless of whether they are specified in events.
Attempts to specify other values in events yield an error.
EPOLLEXCLUSIVE may be used only in an EPOLL_CTL_ADD operation;
attempts to employ it with EPOLL_CTL_MOD yield an error. If
EPOLLEXCLUSIVE has been set using epoll_ctl(), then a subsequent
EPOLL_CTL_MOD on the same epfd, fd pair yields an error. A call
to epoll_ctl() that specifies EPOLLEXCLUSIVE in events and spec‐
ifies the target file descriptor fd as an epoll instance will
likewise fail. The error in all of these cases is EINVAL.
"""

See also https://lwn.net/Articles/633422/

[vstinner]

if exclusive and hasattr(select, "EPOLLEXCLUSIVE"):
 epoll_events |= select.EPOLLEXCLUSIVE

Maybe NotImplementedError would be more appropriate rather than silently ignore the error?

[Zheaoli]

I will work on a PR

[Zheaoli]

"a keyword-only parameter" is good I'll take it done

[giampaolo]

I'm not sure I understand what EPOLLEXCLUSIVE is about. Could you provide a use case?

Also, there are other constants which may also be useful such as EPOLLWAKEUP and EPOLLONESHOT:
http://man7.org/linux/man-pages/man2/epoll_ctl.2.html
So perhaps it makes more sense to expose a lower-level "extra_events" argument instead, which is more flexible and also saves us from the hassle of describing what the new argument is about (which really is a Linux kernel thing) in the doc:

>> s.register(fd, EVENT_READ, extra_events=select.EPOLLEXCLUSIVE | select.EPOLLONESHOT)

That raises the question whether we should also have an "extra_events" argument for modify() method (probably yes).

But again, I think it makes sense to understand what's the use case of these constants first, because if they are rarely used maybe who needs them can simply look at the source and use s._selector.modify() directly.

[Zheaoli]

Hello rodola

Here's detail:

In the server, we use multiprocess to handle the request with share the same FD.

OK, when a request comes, all the process wake up and try to accept the request but only one process will accept success and the others will raise an Exception if we use the epoll without EPOLLEXCLUSIVE.

That means there will waste the performance. This effect is named thundering herd.

But if we use epoll with EPOLLEXCLUSIVE, when the envents happen, only one process will wake and try to handle the event.

So, I think it's necessary to support the EPOLLEXCLUSIVE in selectors.EpollSelector

Actually, Python support EPOLLEXCLUSIVE official since 3.7. It's time to support it in selectors

[giampaolo]

I see. Then I would say it's a matter of deciding what's the best API to provide. Another possibility is to promote the underlying epoll() instance as a public property, so one can do:

>> s = selectors.EpollSelector()
>> s.register(fd, EVENT_READ)
>> s.selector.modify(fd, select.EPOLLEXCLUSIVE)

That raises the question whether all selector classes should have a public "selector" attribute. poll() and devpoll() related classes may need it for POLLPRI, POLLRDHUP, POLLWRBAND or others (whatever their use case is). kqueue() also has it's own specific constants (KQ_FILTER_* and KQ_EV_*). The only one where a public "selector" property would be useless is SelectSelector.