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

Abstract service over AsyncRead and AsyncWrite. #155

Closed
kkimdev opened this issue May 17, 2017 · 3 comments
Closed

Abstract service over AsyncRead and AsyncWrite. #155

kkimdev opened this issue May 17, 2017 · 3 comments

Comments

@kkimdev
Copy link

kkimdev commented May 17, 2017

Please refer to #39 for the prior discussion.

For example, https://github.com/kmcallister/urpc abstracts service over Read + Write trait. The benefit of doing this includes:

  1. Being able to plug-in any transport layer easily, stdin/out, IPC, UDP+KCP, Websocket, etc... Feature request: more transports (was: stdin/stdout transport for IPC) #38
  2. Multiple services on one server/port. Multiple services on one server/port #153
  3. Supporting event-notification pattern RFC: server to client event/notification system #39

Considerations:

  • I don't know if this will affect API ergonomics, but I suspect that it will be trivial to provide a helper function, if it affects.
  • Simply using multiple services for 2 and 3 might not be ideal as we won't have the holistic view of the server state and make it harder to support features like rate-limiting, load-balancing, etc. But Using multiple services won't introduce additional complexity to tarpc so I think that's fine as a mid-term solution.

@tikue do you have any additional comment?

@tikue
Copy link
Collaborator

tikue commented May 17, 2017

I don't think abstracting over read/write inherently enables multiplexing services on a single port. Do you agree with that?

@kkimdev
Copy link
Author

kkimdev commented May 18, 2017

Yeah it does not directly enable that. What I was thinking is that a user needs to provide a multiplexed component so that one TCP connection can be shared as multiple AsyncRead and AsyncWrite, conceptually something like this:

fn multiplex_write_2(x: AsyncWrite) -> (/*W1*/ impl AsyncWrite, /*W2*/ impl AsyncWrite) {/* ... */}
fn multiplex_read_2(x: AsyncRead) -> (/*R1*/ impl AsyncRead, /*R2*/ impl AsyncRead) {/* ... */}

So that writing to W1 should be received by R1 on the other endpoint, and the same for W2 and R2. And use each pair to construct two independent tarpc services.

tikue added a commit that referenced this issue Oct 16, 2018
#199)

# New Crates

- crate rpc contains the core client/server request-response framework, as well as a transport trait.
- crate bincode-transport implements a transport that works almost exactly as tarpc works today (not to say it's wire-compatible).
- crate trace has some foundational types for tracing. This isn't really fleshed out yet, but it's useful for in-process log tracing, at least.

All crates are now at the top level. e.g. tarpc-plugins is now tarpc/plugins rather than tarpc/src/plugins. tarpc itself is now a *very* small code surface, as most functionality has been moved into the other more granular crates.

# New Features
- deadlines: all requests specify a deadline, and a server will stop processing a response when past its deadline.
- client cancellation propagation: when a client drops a request, the client sends a message to the server informing it to cancel its response. This means cancellations can propagate across multiple server hops.
- trace context stuff as mentioned above
- more server configuration for total connection limits, per-connection request limits, etc.

# Removals
- no more shutdown handle.  I left it out for now because of time and not being sure what the right solution is.
- all async now, no blocking stub or server interface. This helps with maintainability, and async/await makes async code much more usable. The service trait is thusly renamed Service, and the client is renamed Client.
- no built-in transport. Tarpc is now transport agnostic (see bincode-transport for transitioning existing uses).
- going along with the previous bullet, no preferred transport means no TLS support at this time. We could make a tls transport or make bincode-transport compatible with TLS.
- a lot of examples were removed because I couldn't keep up with maintaining all of them. Hopefully the ones I kept are still illustrative.
- no more plugins!

# Open Questions

1. Should client.send() return `Future<Response>` or `Future<Future<Response>>`? The former appears more ergonomic but it doesn’t allow concurrent requests with a single client handle. The latter is less ergonomic but yields back control of the client once it’s successfully sent out the request. Should we offer fns for both?
2. Should rpc service! Fns take &mut self or &self or self? The service needs to impl Clone anyway, technically we only need to clone it once per connection, and then leave it up to the user to decide if they want to clone it per RPC. In practice, everyone doing nontrivial stuff will need to clone it per RPC, I think.
3. Do the request/response structs look ok?
4. Is supporting server shutdown/lameduck important?

Fixes #178 #155 #124 #104 #83 #38
@tikue
Copy link
Collaborator

tikue commented Oct 17, 2018

The latest version is now transport agnostic! It's not generic over AsyncRead/AsyncWrite, but it is generic over Stream<Request> + Sink<Response>.

@tikue tikue closed this as completed Oct 17, 2018
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

No branches or pull requests

2 participants