Conversation
einarmo
force-pushed
the
parallel-services
branch
from
b5f24a0 to
40ae7ac
Compare
svanharmelen
approved these changes
Jan 30, 2026
Contributor
|
Guess this needs a rebase when PR #195 is merged. |
Contributor
Author
|
Yep. It seems like the same list of clippy lints. |
This implements a general way to run some of our services concurrently, notably _just_ the ones that are supposed to be run on one node manager specifically, like `Read` and `Write`. This involved some slightly fiddly async code, but the result isn't too bad, and allows us to generalize a few things that we're not very consistent about, like setting `current_node_manager_index` between calls. Doing this properly required cloning the `RequestContext`. I'm concerned about the cost of this clone, which up until now we've been pretty lazy about, so I moved the shared part into an `inner` field behind an `Arc`. To avoid this breaking every server implementation I added a `Deref<RequestContextInner>` to `RequestContext` which is a bad practice, but does seem alright in this case.
einarmo
force-pushed
the
parallel-services
branch
from
40ae7ac to
2e762fd
Compare
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
I've had this lying around for some time, and figured I'd finish it up. It's on the todo list.
This implements a general way to run some of our services concurrently, notably just the ones that are supposed to be run on one node manager specifically, like
ReadandWrite.This involved some slightly fiddly async code, but the result isn't too bad, and allows us to generalize a few things that we're not very consistent about, like setting
current_node_manager_indexbetween calls.Doing this properly required cloning the
RequestContext. I'm concerned about the cost of this clone, which up until now we've been pretty lazy about, so I moved the shared part into aninnerfield behind anArc. To avoid this breaking every server implementation I added aDeref<RequestContextInner>toRequestContextwhich is a bad practice, but does seem alright in this case. The alternative is to make the fields non-public and expose getters, but then every node manager implementation will have a hundred breaking changes.