Add abitiy to store request context #393
Closed
Conversation
This file contains 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
Also adds a Client stub trait alias for each generated service. Now that generic associated types are stable, it's almost possible to define a trait for Channel that works with async fns on stable. `impl trait in type aliases` is still necessary (and unstable), but we're getting closer. As a proof of concept, three more implementations of Stub are implemented; 1. A load balancer that round-robins requests between different stubs. 2. A load balancer that selects a stub based on a request hash, so that the same requests go to the same stubs. 3. A stub that retries requests based on a configurable policy. The "serde/rc" feature is added to the "full" feature because the Retry stub wraps the request in an Arc, so that the request is reusable for multiple calls. Server implementors commonly need to operate generically across all services or request types. For example, a server throttler may want to return errors telling clients to back off, which is not specific to any one service.
This allows plugging in horizontal functionality, such as authorization, throttling, or latency recording, that should run before and/or after execution of every request, regardless of the request type. The tracing example is updated to show off both client stubs as well as server hooks. As part of this change, there were some changes to the Serve trait: 1. Serve's output type is now a Result<Response, ServerError>.. Serve previously did not allow returning ServerErrors, which prevented using Serve for horizontal functionality like throttling or auth. Now, Serve's output type is Result<Resp, ServerError>, making Serve a more natural integration point for horizontal capabilities. 2. Serve's generic Request type changed to an associated type. The primary benefit of the generic type is that it allows one type to impl a trait multiple times (for example, u64 impls TryFrom<usize>, TryFrom<u128), etc.). In the case of Serve impls, while it is theoretically possible to contrive a type that could serve multiple request types, in practice I don't expect that to be needed. Most users will use the Serve impl generated by #[tarpc::service], which only ever serves one type of request.
While using unstable feature type_alias_impl_trait.
Add helper fn to server::incoming module for spawning.
mem::forget is a dangerous tool, and it was being used carelessly for things that have safer alternatives. There was at least one bug where a cloned tokio::sync::mpsc::UnboundedSender used for request cancellation was being leaked on every successful server response, so its refcounts were never decremented. Because these are atomic refcounts, they'll wrap around rather than overflow when reaching the maximum value, so I don't believe this could lead to panics or unsoundness.
…uestContexts. To do this, create a Transport with a Sink/Stream of (C, Item/SinkItem). C created in the stream will be opaqualy sent back when sinking the response on the server side.
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
|
This is based on your branch containing the async trait features. |
axos88
commented
Feb 10, 2023
| deadlines: DelayQueue<u64>, | ||
| } | ||
|
|
||
| impl<C> Default for InFlightRequests<C> { |
There was a problem hiding this comment.
This is necessary, because the default Derive macro expects C: Default, but that is not a real requirement.
|
Signed the CLA. |
|
This currently breaks existing code. need to revise. |
|
Thanks for the PR! For now, I will continue the discussion on #392 and will wait to review this until I better understand the solution space. |
Also adds a Client stub trait alias for each generated service. Now that generic associated types are stable, it's almost possible to define a trait for Channel that works with async fns on stable. `impl trait in type aliases` is still necessary (and unstable), but we're getting closer. As a proof of concept, three more implementations of Stub are implemented; 1. A load balancer that round-robins requests between different stubs. 2. A load balancer that selects a stub based on a request hash, so that the same requests go to the same stubs. 3. A stub that retries requests based on a configurable policy. The "serde/rc" feature is added to the "full" feature because the Retry stub wraps the request in an Arc, so that the request is reusable for multiple calls. Server implementors commonly need to operate generically across all services or request types. For example, a server throttler may want to return errors telling clients to back off, which is not specific to any one service.
This allows plugging in horizontal functionality, such as authorization, throttling, or latency recording, that should run before and/or after execution of every request, regardless of the request type. The tracing example is updated to show off both client stubs as well as server hooks. As part of this change, there were some changes to the Serve trait: 1. Serve's output type is now a Result<Response, ServerError>.. Serve previously did not allow returning ServerErrors, which prevented using Serve for horizontal functionality like throttling or auth. Now, Serve's output type is Result<Resp, ServerError>, making Serve a more natural integration point for horizontal capabilities. 2. Serve's generic Request type changed to an associated type. The primary benefit of the generic type is that it allows one type to impl a trait multiple times (for example, u64 impls TryFrom<usize>, TryFrom<u128), etc.). In the case of Serve impls, while it is theoretically possible to contrive a type that could serve multiple request types, in practice I don't expect that to be needed. Most users will use the Serve impl generated by #[tarpc::service], which only ever serves one type of request.
While using unstable feature type_alias_impl_trait.
Add helper fn to server::incoming module for spawning.
mem::forget is a dangerous tool, and it was being used carelessly for things that have safer alternatives. There was at least one bug where a cloned tokio::sync::mpsc::UnboundedSender used for request cancellation was being leaked on every successful server response, so its refcounts were never decremented. Because these are atomic refcounts, they'll wrap around rather than overflow when reaching the maximum value, so I don't believe this could lead to panics or unsoundness.
|
Hah yeah. I am currntly using my own fork, would be great to get it merges, but currently have zero... or closer to the truth minus infinity resources to devote to this. Might reiterate on it in the future! |
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.
Resolves #392
If a transport is created that sinks/streams (C, Item/SinkItem), and that is used to create the BaseChannel, the transport will be able to generate a value (c:C) in the stream that will be opaquely forwarded to the Sink.
This can be used for example to differentiate in exaclty how a request arrived at the transport, for example if the transport's channel is aggregating multiple clients. It will be used mainly for MQTT, where the response topic is received together with the Request, and the Sink needs to be know where to publish the response.