-
Notifications
You must be signed in to change notification settings - Fork 292
Switch to epoll #5911
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
edwintorok
merged 7 commits into
xapi-project:feature/perf
from
edwintorok:private/edvint/epoll4-sim
Sep 10, 2024
Merged
Switch to epoll #5911
edwintorok
merged 7 commits into
xapi-project:feature/perf
from
edwintorok:private/edvint/epoll4-sim
Sep 10, 2024
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
4e80d42
to
e4e1f5b
Compare
00f837a
to
faadb1f
Compare
We could also make the change suggested in #5910 (comment) to use proper Sets. |
faadb1f
to
eac2d31
Compare
contificate
approved these changes
Aug 27, 2024
psafont
reviewed
Aug 27, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a couple of small notes
mg12
approved these changes
Aug 29, 2024
…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]>
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]>
a76576e
to
4cda3f4
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
One small commit on top of #5910 that switches from select to epoll ~everywhere in xapi/xenopsd.
Draft because we need to merge that PR first, then have a PR to update feature/perf from master.
Will need to be rebased on #5913 once the prerequisites for that PR get merged.