[IMP] core: support more than 255 http workers

[Julien00859]

Use the most efficient Selector implementation available on the current platform

Odoo supports only SelectSelector but it is a little obsolete

python >= 3.4 supports a new high-level library Selectors:

• https://docs.python.org/es/3/library/selectors.html

It could to auto-choose the following ones:

• SelectSelector
• PollSelector
• EpollSelector
• DevpollSelector
• KqueueSelector

Using the DefaultSelector class the most efficient implementation available on the current platform will be use:

• https://docs.python.org/3/library/selectors.html#selectors.DefaultSelector

It helps to support better the resources of the system

Using SelectSelector you are not able to run workers >=255

If you set ulimit -n 10240 and run odoo-bin --workers=255
the following error is raised:

Traceback (most recent call last):
File "odoo/service/server.py", line 926, in run
    self.sleep()
File "odoo/service/server.py", line 852, in sleep
    sel.select(self.beat)
File "python3.8/lib/python3.8/selectors.py", line 323, in select
    r, w, _ = self._select(self._readers, self._writers, [], timeout)
ValueError: filedescriptor out of range in select()

But using PollSelector it is not reproduced even using more workers

Most of platform supports PollSelector but using DefaultSelector we can be sure
that even too old system are supported too

And using this High-level library will allow to use the future new improvements

e.g. Epoll has better performance improvements

More info about:

• https://devarea.com/linux-io-multiplexing-select-vs-poll-vs-epoll
• Update connection polling to use something other than select.select() redis/redis-py#486

Co-Authored-By: Julien Castiaux juc@odoo.com

[robodoo]

Pull request status dashboard.

[Julien00859]

Benchmark: empty basebase, only base web and test_http, started with 16 http workers.

bench_master.txt
bench_epoll.txt

[moylop260]

Hi @Julien00859

Thank you for the review and the improves in the changes

I just ran the following command in my local environment:

odoo (master-256_http_workers-juc)$
ulimit -n 10240 && ./odoo-bin -d 255w --workers=255 --log-level=error

And the result was:

2022-08-09 16:58:21,143 40673 ERROR ? odoo.service.server: Worker WorkerHTTP (40673) Exception occurred, exiting...
Traceback (most recent call last):
  File "odoo/service/server.py", line 1064, in _runloop
    self.sleep()
  File "odoo/service/server.py", line 978, in sleep
    select.select([self.multi.socket, self.wakeup_fd_r], [], [], self.multi.beat)
ValueError: filedescriptor out of range in select()

[Julien00859]

Hello @moylop260,

Thank you for letting me know, indeed there is an issue. I thought select(2) was not able to watch more than 1024 fd at a same time, independently from the file descriptor value. The reality is, it cannot watch any file descriptor which int value is higher than 1024.

POSIX allows an implementation to define an upper limit, advertised via the constant FD_SETSIZE, on the range of file descriptors that can be specified in a file descriptor set. The Linux kernel imposes no fixed limit, but the glibc implementation makes fd_set a fixed-size type, with FD_SETSIZE defined as 1024, and the FD_*() macros operating according to that limit. To monitor file descriptors greater than 1023, use poll(2) instead.

I'll remplace select.select by select.poll everywhere in the codebase. The advantage of using select.poll over using selectors is that the API is stateless. I don't want to create a lasting selector object and to register/unregister file descriptors as the sockets are created/deallocated. For the many places where we just watch a bunch of fd, it's just way easier to use poll. Are you alright with it?

[Julien00859]

My above statement is invalid, windows lacks select.poll and is too unable to use select with fd>=1024

[moylop260]

@Julien00859

I just updated your code and it is not raising the error 🎉
Thank you!

I am not even sure why are you removing the InterruptedError exception:

except select.error as e:
    if e.args[0] not in [errno.EINTR]:

[Julien00859]

I am not even sure why are you removing the InterruptedError exception:

except select.error as e:
    if e.args[0] not in [errno.EINTR]:

It is no more necessary, the actual low-level select call done in selectors is wrapped around a try: ... except InterruptedError:. See:

https://github.com/python/cpython/blob/8b34e914bba2ccd6ae39609410db49d0beb19cb1/Lib/selectors.py#L319-L325

[moylop260]

Hi @Julien00859

I just realized that odoo@16.0 is using selectors in the following lines of code:

• odoo/addons/bus/models/bus.py

  Line 152 in daded01

  selectors.DefaultSelector() as sel:

• odoo/addons/bus/websocket.py

  Line 242 in 5471bd4

  self._selector = selectors.DefaultSelector()

However, the issue of using many workers is still reproduced since there are lines of code using directly select.select:

• odoo/odoo/service/server.py

  Line 458 in d7b3734

  select.select([pg_conn], [], [], SLEEP_INTERVAL + number)

• odoo/odoo/service/server.py

  Line 887 in d7b3734

  ready = select.select(fd_in, [], [], self.beat)

• odoo/odoo/service/server.py

  Line 1015 in d7b3734

  select.select([self.multi.socket, self.wakeup_fd_r], [], [], self.multi.beat)

• odoo/odoo/service/server.py

  Line 1169 in d7b3734

  select.select([self.wakeup_fd_r, self.dbcursor._cnx], [], [], interval)

It could be using different selector for bus and service
Is this expected?

[Julien00859]

I did review the PR that changed the bus model to use websockets and I suggested to use selector as the underlying network library citing this PR's effort.

So you are absolutely right, there are code using selector in 16.0 :) We took the opportunity that websocket was being extensively tested to include that change.

[Julien00859]

@odoo/rd-framework-py @odoo/rd-security up

[xmo-odoo]

Might be a good idea to use the opportunity to freshen a few bits of code:

• use super instead of Worker.start(self)
• type multi: PreforkServer in Worker.__init__ as currently it's really obscure when reading the code (renaming multi would be a large change so probably a bad idea)
• create the attributes of Worker subclasses in their __init__ to avoid warnings, maybe use __slots__ for clarity
• self.eintr_pipe can be removed, it's only used for os.close, which can be called directly on wakeup_fd_r and wakeup_fd_w

[Julien00859]

use super instead of Worker.start(self)
type multi: PreforkServer in Worker.init as currently it's really obscure when reading the code (renaming multi would be a large change so probably a bad idea)
self.eintr_pipe can be removed, it's only used for os.close, which can be called directly on wakeup_fd_r and wakeup_fd_w

done, also changed py2 super to py3

create the attributes of Worker subclasses in their init to avoid warnings, maybe use slots for clarity

I didn't understand, can you clarify?

[xmo-odoo]

create the attributes of Worker subclasses in their init to avoid warnings, maybe use slots for clarity

I didn't understand, can you clarify?

Currently the Worker subclasses create a bunch of their attributes in non-__init__ methods, which various tools warn about e.g. WorkerHTTP.server or WorkerCron.dbcursor in their respective start methods.