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

Horizontally scalable endpoints #1578

Draft
wants to merge 12 commits into
base: main
Choose a base branch
from
Draft

Horizontally scalable endpoints #1578

wants to merge 12 commits into from

Conversation

Ralith
Copy link
Collaborator

@Ralith Ralith commented May 28, 2023

Having a proper go at #1576. Reduces the inter-task communication required at the quinn layer by allowing connection tasks to manipulate the endpoint directly. Forms a strong foundation for a more incremental and performant attempt at #1219.

The high-level architecture is largely unchanged for now, but this is already enough for a hypothetical motivated quinn-proto user to implement good horizontal scaling.

This PR is organized into four major sections:

  1. Incidental cleanup/fixes
  2. Refactoring of mutable state inside proto::Endpoint
  3. Placement of remaining mutable endpoint state behind locks, allowing all methods to take &self.
  4. Refactoring proto::Connection to call directly into proto::Endpoint rather than relying on the caller (e.g. quinn) to pass messages

Next steps (probably in future PRs) might be to move ownership of proto::Connections into the proto::Endpoint, followed by finally attempting to unify the drivers again. We'll need to work out how to get multiple coarse endpoint drivers to play nicely with tokio before this can be completed.

@Ralith Ralith force-pushed the shared-endpoint branch 4 times, most recently from 2262a6f to 23afc97 Compare May 29, 2023 01:57
@djc
Copy link
Member

djc commented Jun 2, 2023

Maybe split off the incidental cleanups/fixes into a separate PR? Easier to get through, and less prone to change (as in your latest insights from #1576, which I assume might affect this PR). (Maybe also the refactoring of mutable state, not sure how much sense that makes on its own?)

@Ralith
Copy link
Collaborator Author

Ralith commented Jun 2, 2023

Yep, I'll try to split this up some. I'm indeed wary of committing to the direction explored here prematurely; in particular I don't intend that we merge any major redesigns to how work is distributed/tasks are synchronized without careful benchmarking demonstrating similar or better performance under a broad range of workloads. There's a risk of optimizing for one shiny hypothetical case at the cost of other more immediate ones.

@Ralith
Copy link
Collaborator Author

Ralith commented Jun 2, 2023

Regarding the interior mutability, I do like the simpler API and behavior it entails, but that comes at the cost of greater risk of contention under worst-case conditions, which an attacker might deliberately produce. Not sure if it'll pan out.

@Ralith Ralith mentioned this pull request Jun 3, 2023
@Ralith Ralith force-pushed the shared-endpoint branch from 23afc97 to 35f7eb2 Compare June 3, 2023 22:13
@Ralith Ralith marked this pull request as draft June 4, 2023 01:07
@Ralith Ralith force-pushed the shared-endpoint branch from 35f7eb2 to 2275870 Compare June 4, 2023 01:11
@Ralith Ralith force-pushed the shared-endpoint branch from 2275870 to 0ba3461 Compare June 25, 2023 19:11
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.

2 participants