Restore RequestResponse::throttled.#1726
Conversation
In contrast to the existing "throttled" approach this PR adds back- pressure to the protocol without requiring pre-existing knowledge of all nodes about their limits. It adds small, CBOR-encoded headers to the actual payload data. Extra credit messages communicate back to the sender how many more requests it is allowed to send.
Should an error in some lower layer cause a connection to be closed, our previously sent credit grant may not have reached the remote peer. Therefore, pessimistically, a credit grant is resent whenever a connection is closed. The remote ignores duplicate grants.
There was a problem hiding this comment.
What do you think about moving the codec::header module to throttled::codec? I think it is conceivable that request-response may contain other auxiliary wrappers with their own headers in the future, so separating each into their own (sub)modules seems preferable.
- Remove `ResponseSent` which was a leftover from previous attemps and issue a credit grant immediately in `send_response`. - Only resend credit grants after a connection is closed if we are still connected to this peer.
Co-authored-by: Roman Borschel <romanb@users.noreply.github.com>
Co-authored-by: Roman Borschel <romanb@users.noreply.github.com>
Co-authored-by: Roman Borschel <romanb@users.noreply.github.com>
|
As far as I know all of libp2p uses Protobuf for binary encoding. Can you explain what benefit is gained by encoding |
Mplex and yamux encode their headers in a custom format. Multi-addresses use another custom binary encoding. Peer IDs are multihashes, multistream-select uses length-prefixed and
Contrary to protocol buffers, CBOR is an IETF specification (RFC 7049) and not merely implementation defined. It is also rather lightweight w.r.t. tooling requirements. As you can see, there is no need for another compiler like
I do not agree with this consistency claim which rests on a particular way to look at the various libp2p bits and pieces. From my perspective encodings are everywhere and nothing is lost by embracing this variety. |
I stand corrected! 🙂 Thanks for the detailed explanation @twittner. I would prefer to have a single generic binary encoding scheme just for the sake of reducing complexity. At the same time this opinion is not a strong one and should not block this pull request. |
Since honest senders always assume a send budget of 1 and wait for credit afterwards, setting the default limit to a higher value can only become effective after informing the peer about it which means leaving `max_recv` at 1 and setting `next_max` to the desired value.
3 participants
In contrast to the existing "throttled" approach this PR adds back-pressure to the protocol without requiring pre-existing knowledge of all node limits. It adds small, CBOR-encoded credit grant messages which communicate back to the sender how many more requests it is allowed to send.
Alternative to #1715.