search

lthibault / quic-mangos

A QUIC transport for mangos (scalability protocols) written in pure Go
Apache License 2.0
24 stars 2 forks source link

Protocol enhancements #2

Open gdamore opened 6 years ago

gdamore commented 6 years ago

I've been thinking about this, and I don't like the variable length path that gets written into the header. The primary reason is that newline delimited protocols are a pain, because you have to allow for variable length headers; what's the limit on this header length for example?

What if instead we generated a number, and used that; the value could be somewhat large, for example a 128-bit or even 256-bit checksum of the path (yielding a 16- or 32-byte value). To be honest, I think even a 64-bit number (only 8 bytes) would probably be sufficient, just taking the lower 64-bits from a sha256 or somesuch.

The disadvantage of that approach is that when a server receives a connection request for which it has no listener, it would be hard to diagnose to know what the client was attempting to connect to.

The other thing, is that I'd actually put the SP-layer protocol negotiation in the same packet, eliminating the NewConnPipe based handshake -- this would save another round trip of negotiation, since we're already exchanging data.

As it stands today, I'm not ready to endorse the current QUIC transport for mangos or NNG -- but I can see that we could make improvements where I'd be more inclined to agree.

lthibault commented 6 years ago

@gdamore Firstly, thanks for taking the time to look through this and for your feedback!

What if instead we generated a number [...] The disadvantage of that approach is that when a server receives a connection request for which it has no listener, it would be hard to diagnose to know what the client was attempting to connect to.

Just to make sure I understand, you're suggesting we hash the URL path and send that (possibly truncated) value over the wire, where it can map onto the corresponding stream. The idea is to conceptually do the same thing as we're doing now with full paths, but with some fixed-length opaque identifier.

From this, and your comment about servers receiving connection requests for which there are no listeners, I gather that it should never be necessary to derive the URL on the server side. Is this correct?

The other thing, is that I'd actually put the SP-layer protocol negotiation in the same packet, eliminating the NewConnPipe based handshake -- this would save another round trip of negotiation, since we're already exchanging data.

This definitely sounds like good design. Could you just walk me through (on a high-level) where/how this would happen in code?

As it stands today, I'm not ready to endorse the current QUIC transport for mangos or NNG -- but I can see that we could make improvements where I'd be more inclined to agree.

Not a problem. I'm happy to implement these improvements provided I can ping you once in a while for guidance 😃

gdamore commented 6 years ago

First, yes, I'm suggesting we hash the URL. The listeners don't need to know what the precise URL that a failed client was attempting to reach was, only that it did not match. It would be good for them to send back an error to the effect of "refused - nobody there" if there is no listener at the hash code that the client attempted.

(From an implementation standpoint, listeners and dialers can keep the URL around in memory, so that they can answer property requests for the URL, but the URL need never go over the wire.)

Second, about the protocol handshake. You'll need to replace the code where you use the NewConnPipe call. That call is based on TCP style handshake where the protocol number is sent by each side. That's not necessary.

Instead you can do that exchange with the URL hash. Basically send your own protocol number. The listener can refuse it if it so chooses (ideally sending back an error.) When the listener accepts the connection, if there is a message going back, it can add it's own protocol number, which the client can validate. (This isn't strictly necessary -- for example the websocket transport lacks this ability -- but I if you're already sending a frame back to the client then you might as well include it.)

lthibault commented 6 years ago

Sounds good, I'll take a stab at it ASAP (and after implementing Get/SetOpt).