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

Move QCMP to a seperate port #741

Merged
merged 2 commits into from
Jul 11, 2023
Merged

Move QCMP to a seperate port #741

merged 2 commits into from
Jul 11, 2023

Conversation

XAMPPRocky
Copy link
Collaborator

This PR moves QCMP onto its own port and worker in the proxy, I initially I thought I needed this, but after I wrote I found a better solution in the client, but I decided to open the PR anyway as makes the implementation cleaner and in theory faster as it avoids allocating on the heap (though I haven't actually measured if this is noticeable). This has the added benefit of allowing operators to choose to disable the protocol by not exposing the port, or allowing operators to have QCMP only instances.

Also I removed the shutdown_rx in the downstream workers as that would cause the process to stop accepting packets after it shut down.

@XAMPPRocky
Copy link
Collaborator Author

@markmandel 503 on the lib.rs again, should we just change it to crates.io to avoid this problem?

@markmandel
Copy link
Contributor

@markmandel 503 on the lib.rs again, should we just change it to crates.io to avoid this problem?

We ignore crates.io links as well, since they also don't like to be crawled. I'll mark this link as ignored as well in my prep for 0.7.0 PR, and update the User Agent we use as well.

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.

Looks good!

@XAMPPRocky XAMPPRocky enabled auto-merge (squash) July 11, 2023 08:36
@XAMPPRocky XAMPPRocky merged commit 75a85b0 into main Jul 11, 2023
@quilkin-bot
Copy link
Collaborator

Build Succeeded 🥳

Build Id: beace98f-0d6e-4249-a4f6-d32ac8262aa8

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

To build this version:

git fetch git@github.com:googleforgames/quilkin.git pull/741/head:pr_741 && git checkout pr_741
cargo build

@markmandel markmandel deleted the ep/new-qcmp-port branch August 3, 2023 23:27
@markmandel markmandel added kind/cleanup Refactoring code, fixing up documentation, etc kind/breaking Changes to functionality, API, etc. labels Oct 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/breaking Changes to functionality, API, etc. kind/cleanup Refactoring code, fixing up documentation, etc size/s
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants