search

n0-computer / quic-rpc

A streaming rpc system based on quic
Other
94 stars 12 forks source link

fix: add additional framing to messages #26

Closed rklaehn closed 1 year ago

rklaehn commented 1 year ago

On write, prepend a u32 containing the size of the message. On read, read until you have enough data, then deserialize.

With this we no longer rely on 1 http2 frame = 1 msg. We might as well allow http1 now...

ppodolsky commented 1 year ago

Implementations looks nice. Just want to learn if you have seen LengthDelimitedCodec, is it applicable here? By description, it is doing just the same.

rklaehn commented 1 year ago

I think a LengthDelimitedCodec is for when you have an AsyncRead and want to turn it into a stream of chunks. But in our case we already have a stream of Bytes, which just sadly does not always correspond to the chunking we want.

rklaehn commented 1 year ago

Cloning will lead to strange results when doing things that way. I think the buffer stuff needs to happen before sending via the flume channel. Will push an update.

rklaehn commented 1 year ago

@ppodolsky pushed an update. Please check it out. Sorry about this being so slow, I am not in the office atm.

ppodolsky commented 1 year ago

Ok, I'll check it today. But there were no issues with even previous commit of this PR. What is worth to look at?

rklaehn commented 1 year ago

The previous one was fine as long as you did not clone the endpoint. But if you do, it would get totally weird. Imagine a part of a message getting to one clone of the receiver, and another one getting to another one.