search

quic-go / quic-go

A QUIC implementation in pure Go
https://quic-go.net
MIT License
9.73k stars 1.28k forks source link

Pluggable congestion control #776

Open isaachess opened 6 years ago

isaachess commented 6 years ago

Right now this package uses an implementation of TCP-cubic for congestion control. It would be nice if the code easily allowed different congestion control algos to be used.

For example, if I wanted to use a LEDBAT congestion control algorithm, if I were to write my own implementation of the SendAlgorithm interface I still don't see a way to plug that in when I create a new session.

So it would be nice, even if we do not yet have officially supported congestion control other than cubic, to have an easy way to write one and use it instead of cubic.

isaachess commented 6 years ago

The reason I'm asking this is specifically for LEDBAT. We need the ability to use cubic cc for some connections, but LEDBAT for others. So if there is already a simple way to do that with this QUIC implementation I'd be interested in that.

lucas-clemente commented 6 years ago

I think the options are either having an enum for the congestion controllers we support (which wouldn't allow it to be different on a connection basis), or adding a func (connectionID int64) SendAlgorithm to the quic.Config.

We also have plans to support BBR (#341), and for that we'll need to change the SendAlgorithm interface – so there will be breaking changes at some point.

If you plan to do benchmarks please be aware that some performance-relevant parts of our implementation are still lacking (e.g. loss recovery).

isaachess commented 6 years ago

I think having different congestion-control per-connection is not really what I need (sorry for being imprecise), but rather being able to change it per-process.

And yes, I agree having an enum for officially supported congestion controllers is a fantastic idea. I just think even now, before you get around to officially supporting a bunch of different congestion controllers, the code could be rewritten a little to make it simpler to write your own congestion controller and plug it into the session.

lucas-clemente commented 6 years ago

SGTM :) If forking is an acceptable solution for now, the relevant line you need to change is https://github.com/lucas-clemente/quic-go/blob/master/ackhandler/sent_packet_handler.go#L69.

Feel free to send PRs to simplify things wherever you think it makes sense – what specifically did you have in mind?