Skip to content

Conversation

edwintorok
Copy link
Contributor

Pending some final testing by QA, these commits have already been reviewed on the feature branch.

This switches select calls to Unixext.select (which is implemented using epoll), and implements the few performance sensitive parts using epoll directly.
It also enables some more tests with >1024 fds.

edwintorok and others added 13 commits July 18, 2024 15:37
Update feature/perf from master
Update feature/perf from master
…quivalents

Also add xapi-stdext-unix or xapi-stext-threads as needed to dune.

Signed-off-by: Edwin Török <[email protected]>
Use SO_RCVTIMEO socket option instead.
This will cause EAGAIN or EWOULDBLOCK to be returned according to `socket(7)`.

Need to be careful about the distinction between select 0 and SO_RCVTIMEO 0.
select 0 would timeout immediately, but SO_RCVTIMEO would timeout never.

Signed-off-by: Steven Woods <[email protected]>
Signed-off-by: Edwin Török <[email protected]>
This will also allow XAPI to better defend against XSA-413 type attacks.

The default TCP connection limit is 800 (configurable), and although
unauthenticated clients shouldn't be able to exhaust the number of file descriptors easily,
authenticated clients still could, since handling an API request might require more file descriptors to be opened.

Signed-off-by: Edwin Török <[email protected]>
…API and xenopsd

This is somewhat of a hack, as it parses the disassembled binary of xapi and xenopsd,
and might break with future compiler versions that invoke functions differently.
There could also be false positives if a dependency calls Unix.select for just sleeping,
i.e. with all 3 arguments as empty lists, but so far we don't have any.

We still have the runtime test if that happens.

If the commits are reordered this does find the 'Thread.wait_timed_read' call in 'Master_database_connection',
and with the correct order of the commits it passes, offering some assurance that we don't actually call 'select' anymore,
not even indirectly via one of our dependencies.

Signed-off-by: Edwin Török <[email protected]>
We've enabled running the unixext tests in quicktest.
They open 1024 file descriptors on startup to check for the absence of select.
And although that works for unixext (it is select-free), it doesn't yet work for the rest
of quicktests on master (the required changes to make it work are on the epoll branch).

Temporarily disable this test and add a note to reenable it on the epoll branch.

Fixes: efcb7af ("CP-50448: run the QuickCheck tests in QuickTest")

Signed-off-by: Edwin Török <[email protected]>
This reverts commit 7dabd1e.

It was only needed on master, and we have to revert it on the epoll branch to reenable testing.

Signed-off-by: Edwin Török <[email protected]>
There is a conflict in 'dune' files, have to make PR from fork to fix,
and I cannot edit the origin repo on the other PR.
@edwintorok edwintorok added this pull request to the merge queue Sep 19, 2024
Merged via the queue into master with commit f51e7e7 Sep 19, 2024
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants