Skip to content
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

Just use epoll for qcmp #1012

Merged
merged 2 commits into from
Sep 5, 2024
Merged

Just use epoll for qcmp #1012

merged 2 commits into from
Sep 5, 2024

Conversation

Jake-Shadle
Copy link
Collaborator

This just uses the simple epoll QCMP loop for linux, io_uring gives no benefits as we are only ever doing a single recv or send request at a time. For now it just spawns a task on the main tokio runtime, so in high CPU usage scenarios this will affect the timing, but...that might be a good thing? Unsure.

@Jake-Shadle
Copy link
Collaborator Author

FWIW this is not necessarily an attempt to fix the issue we saw with QCMP as I never once found a test failure either automatic or manually with the io-uring code, but this at least removes all the complexity to make any issue easier to track down.

@Jake-Shadle Jake-Shadle enabled auto-merge (squash) September 3, 2024 13:27
Copy link
Contributor

@markmandel markmandel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@quilkin-bot
Copy link
Collaborator

Build Succeeded 🥳

Build Id: ad02fafd-eb07-4ae9-ac6c-fb687b90b27c

The following development images have been built, and will exist for the next 30 days:

To build this version:

git fetch [email protected]:googleforgames/quilkin.git pull/1012/head:pr_1012 && git checkout pr_1012
cargo build

@Jake-Shadle Jake-Shadle merged commit 3392942 into main Sep 5, 2024
12 checks passed
@Jake-Shadle Jake-Shadle deleted the fix-qcmp branch September 5, 2024 17:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants