diff mbox series

[ovs-dev,v3] python: poller: Use poll(2) instead of select(2).

Message ID 20260917143815epcms3p3528d5bb5a215faa3a8caf7728b27b2ba@epcms3p3
State New
Headers show
Series [ovs-dev,v3] python: poller: Use poll(2) instead of select(2). | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed
ovsrobot/github-robot-_FreeBSD_Build_and_Test success github build: passed

Commit Message

이규영 Sept. 17, 2026, 2:38 p.m. UTC 
From: Kyuyeong Lee <kyu0.lee@samsung.com>
Date: Thu, 17 Sep 2026 20:00:33 +0900
Subject: [PATCH v3] python: poller: Use poll(2) instead of select(2).

Poller has always allocated SelectPoll, that is _SelectSelect, which
emulates poll(2) on top of select.select.  The comment above it offers to
swap in select.poll by hand when eventlet and gevent are not in use, but
the name it mentions, _SelectPoll, does not exist anywhere in the tree, so
uncommenting that line has no effect.

Emulating poll(2) with the real select(2) is not merely slower.  select(2)
cannot represent a file descriptor numbered FD_SETSIZE (1024) or higher,
and Python raises

    ValueError: filedescriptor out of range in select()

instead of corrupting the fd_set.  A long lived process that holds more
than a thousand file descriptors therefore breaks as soon as a descriptor
the poller watches lands above the limit, for example when a connection is
re-established late in the process lifetime.  Poller.block() does not catch
ValueError, so it reaches the caller, and a caller that retries its loop
spins on a full CPU from then on.

This has been masked for eventlet users, which is most of them: eventlet
replaces select.select with eventlet.green.select, which multiplexes
through the hub and has no FD_SETSIZE limit.  As OpenStack removes its
eventlet dependency the emulation starts calling the real select(2) and the
limit applies.  It was hit in production in a neutron-server whose API
workers each held around 1200 file descriptors: the OVSDB connection thread
wedged permanently at 100% CPU and every later transaction in that worker
failed until the process was restarted.

Use poll(2), which has no FD_SETSIZE limit, and keep the emulation where
select.select is green.  There it is not subject to the limit either, and
the real poll(2) would block the whole interpreter rather than yield to the
other greenlets, which is precisely what the emulation exists to avoid.

get_system_poll() cannot be reused here.  It deliberately returns the
original, blocking poll under eventlet, which is what the non-blocking
poll(0) probe in socket_util.check_connection_completion() wants, but not
what Poller.block() needs.

Signed-off-by: Kyuyeong Lee <kyu0.lee@samsung.com>
---
v2: No code change. Resent with an in-body From: line so that the author
    matches the Signed-off-by; our mail gateway rewrites the display name
    in the From header.
v3: No code change. v2 was mangled in transit, the tabs in
    python/automake.mk were turned into spaces. Resent after checking
    that they survive this time.

 python/automake.mk              |  3 ++-
 python/ovs/poller.py            | 22 ++++++++++++++---
 python/ovs/tests/test_poller.py | 44 +++++++++++++++++++++++++++++++++
 3 files changed, 64 insertions(+), 5 deletions(-)
 create mode 100644 python/ovs/tests/test_poller.py
diff mbox series

Patch

diff --git a/python/automake.mk b/python/automake.mk
index c3e960c82..91ff76212 100644
--- a/python/automake.mk
+++ b/python/automake.mk
@@ -49,7 +49,8 @@  ovs_pytests = \
 	python/ovs/tests/test_kv.py \
 	python/ovs/tests/test_list.py \
 	python/ovs/tests/test_odp.py \
-	python/ovs/tests/test_ofp.py
+	python/ovs/tests/test_ofp.py \
+	python/ovs/tests/test_poller.py
 
 ovs_flowviz = \
 	python/ovs/flowviz/__init__.py \
diff --git a/python/ovs/poller.py b/python/ovs/poller.py
index 12f40993f..0565e82a3 100644
--- a/python/ovs/poller.py
+++ b/python/ovs/poller.py
@@ -110,9 +110,23 @@  class _SelectSelect(object):
 
 
 SelectPoll = _SelectSelect
-# If eventlet/gevent isn't used, we can use select.poll by replacing
-# _SelectPoll with select.poll class
-# _SelectPoll = select.poll
+
+
+def _get_poll():
+    """Return a poll object suitable for the current environment.
+
+    Under eventlet or gevent, select.select is replaced by a green
+    implementation that multiplexes through the hub.  _SelectSelect is
+    therefore both cooperative and free of the FD_SETSIZE limit there, so
+    keep using it.  Everywhere else select.select is the real select(2),
+    which cannot handle a file descriptor numbered FD_SETSIZE or higher, so
+    use poll(2), which has no such limit.
+    """
+    if _using_eventlet_green_select() or (
+            gevent_monkey and
+            gevent_monkey.is_object_patched('select', 'select')):
+        return _SelectSelect()
+    return select.poll()
 
 
 class Poller(object):
@@ -220,7 +234,7 @@  class Poller(object):
                     vlog.dbg("%s on fd %d" % (s, fd))
 
     def __reset(self):
-        self.poll = SelectPoll()
+        self.poll = _get_poll()
         self.timeout = -1
 
 
diff --git a/python/ovs/tests/test_poller.py b/python/ovs/tests/test_poller.py
new file mode 100644
index 000000000..9ff878413
--- /dev/null
+++ b/python/ovs/tests/test_poller.py
@@ -0,0 +1,44 @@ 
+import select
+
+from unittest import mock
+
+import ovs.poller
+
+# select.poll is a builtin function, not a type, so capture the type of the
+# object it returns in order to be able to assert on it.
+POLL_TYPE = type(select.poll())
+
+
+def test_poller_uses_poll_by_default():
+    """Poller must use poll(2) when select.select is the real select(2).
+
+    select(2) raises ValueError for a file descriptor numbered FD_SETSIZE
+    (1024) or higher, poll(2) has no such limit.
+    """
+    with mock.patch.object(ovs.poller, '_using_eventlet_green_select',
+                           return_value=False), \
+            mock.patch.object(ovs.poller, 'gevent_monkey', None):
+        assert isinstance(ovs.poller.Poller().poll, POLL_TYPE)
+
+
+def test_poller_uses_select_under_eventlet():
+    """Poller must keep emulating poll with select.select under eventlet.
+
+    The green select has no FD_SETSIZE limit either and, unlike the real
+    poll(2), it yields to the hub instead of blocking the interpreter.
+    """
+    with mock.patch.object(ovs.poller, '_using_eventlet_green_select',
+                           return_value=True):
+        assert isinstance(ovs.poller.Poller().poll, ovs.poller._SelectSelect)
+
+
+def test_poller_uses_select_under_gevent():
+    """Same as under eventlet, for a gevent patched select.select."""
+    gevent_monkey = mock.Mock()
+    gevent_monkey.is_object_patched.return_value = True
+    with mock.patch.object(ovs.poller, '_using_eventlet_green_select',
+                           return_value=False), \
+            mock.patch.object(ovs.poller, 'gevent_monkey', gevent_monkey):
+        assert isinstance(ovs.poller.Poller().poll, ovs.poller._SelectSelect)
+    gevent_monkey.is_object_patched.assert_called_once_with('select',
+                                                            'select')