n0-computer / quic-rpc Goto Github PK

Currently the http2 transport has an into_inner only on the SendSink, because the RecvStream never sees the raw bytes. The framing and deserialization gets done in the forwarder.

The recv side should be changed so that the stream of raw bytes::Bytes is also available for into_inner.

Also, there should be utilities to turn a stream of bytes into an AsyncRead and a sink of bytes into an AsyncWrite. The former exists for futures, but unfortunately not for tokio.

Currently the client will just try to connect forever when pointed to an incompatible server. Errors are being logged, but at a low level.

There should be a way to make this more prominent.

Either write an UDS transport from scratch, or figure out how to use quinn via an unix domain socket.

This is useful because an unix domain socket is easier to secure than a localhost network socket, and might also give a bit of a performance advantage.

Not sure if we want to expose the inner workings of the channel. I kinda like it that we don't expose too much, so we can switch from hyper to something else. (although we do expose hyper::Url in the public interface)

On a request/replay stream/sink pair for individual RPCs it should be possible to close one half while still keeping the other half open.

An example of this would be a watch call, which is a single message as request and a stream of responses. After the client makes the request it should be able to close the channel making it clear no more messages will be sent to the server. Likewise, and more important, the server would do the same: after reading the request it closes the request stream ensuring the client can no longer send any messages on the request stream.

This would be analogous to e.g. quinn::RecvStream::stop or sn2_quic::stream::ReceiveStream::stop_sending (and their send stream equivalents).

This could be done by having - on the send and recv side, a way to obtain a flume sink or stream. For some channels, like the flume channel, this is a noop.

Flume works equally well from sync and async code.

You should be able to associate a value with a connection so that it gets dropped if the connection gets dropped. Or in general the ability to do some sort of cleanup on connection loss.

requested here: https://github.com/n0-computer/iroh/pull/602/files#r1049436071

Currently there is a lot of copying stuff that is unnecessary in 99% of the time since the arriving Bytes are exactly the message most of the time, so there is no need for copying.

at 0ccd8f5

It's probably a configuration issue but it fails first with a doc test, removing the doc tests brings up a lot of compilation issues afterwards

error: could not compile `quic-rpc` (test "slow_math") due to 48 previous errors
error: could not compile `quic-rpc` (test "quinn") due to 45 previous errors

The flume version used here is out of sync with iroh, resulting in duplicate deps. Would be nice to make them match.

Currently all transports that do serialization/deserialization use hardcoded frame sizes.

Those should be made configurable, and check that the frame sizes are compatible with the maximum of the underlying transport. E.g. http2 has 16,777,215 bytes max frame size.

Currently, when writing something that is larger than the configured frame, the transport will just silently die. You will see an EarlyClose.

It would be better if the error was caught immediately on trying to send such an oversized value.

We don't do versioning. But we should have optional version enforcement. You should be able to set it up so that quic-rpc will refuse to connect to an endpoint that has a different version (e.g. git commit hash) of the protocol.

Currently, some errors like deser errors are silently dropped.

It is intentional that these errors are not forwarded to the use site - what would it do with it. But they should at least be logged somehow.

E.g. in the hyper transport the various things that can go wrong.

Connection error handling works just fine. But often the RPC user will want to send internal errors over the wire.

I don't want to conflate this with the connection error handling. So a RPC call that can produce an user visible error will return something like this: Result<Result<T, UserError>,ConnectionError>. And in principle the UserError should not be a concern of quic-rpc.

But maybe we can come up with something to make writing such things less painful. Some kind of generic serializable user error that interacts well with anyhow.

Ok, that's a pretty rubbish title. But what we really need to be able to do is:

• Signal shutdown to the server.
• Server stops accepting new connections.
• Server keeps handling existing connections.
• Once there are no more connections the server finishes, an event you can away.

The way the server is used in iroh-memstore would allow making use of this to nicely terminate. And I'm pretty sure the hyper and quinn transports allow us to implement this here.

This is kind of already done for the http2 transport, but because there was no use yet for the explicit waiting API it was not handled so needs extending.

E.g. https://docs.rs/quic-rpc/latest/quic_rpc/transport/ does not show the quinn module.

Hi!
Thank you for your great crate.

I tried to use the example with flume feature(for wasm) but can't find how to init client like in this code:

let client = quic_rpc::transport::quinn::QuinnConnection::new(
        endpoint,
        server_addr,
        "localhost".to_string(),
    );

Cheers!

The quinn throughput numbers are disappointing. s2n-quic seems to have a pretty high level API, so it should not be that hard to wrap it.

https://github.com/aws/s2n-quic

It would be probably best to have metrics for the transports, since different transports will have different metrics.

A RpcClient should not just wrap a connection, but contain logic to reconnect. Signature should be unchanged.

First, just do brute force. Eventually there would have to be some backoff.

as requested by @Arqu