quicwg / qlog

The IETF I-D documents for the qlog format
Other
84 stars 12 forks source link

Add congestion control name to recovery:parameters_set #377

Closed rmarx closed 7 months ago

rmarx commented 8 months ago

As reported by @hlandau on the mailing list:

recovery:parameters_set:

A minor point, but I suppose it may be useful to have a field for a name of a congestion control algorithm here, defined as "newreno" / text (with "newreno" being the algorithm specified in RFC 9002).

marten-seemann commented 8 months ago

This is a hard one. Of the QUIC implementations that implement NewReno, most of them tweak the algorithm described in RFC 9002 at least a little bit. So if you actually want to know what the congestion control algorithm is, you'll have to look at the NewReno as implemented in version x.y of that QUIC stack.

LPardue commented 8 months ago

Yeah in principle this sounds nice. But I don't think there is much robust interop opportunity around such a string. Marten highlighted one reason with version, other factors include other parameters of the software or environment.

I could theorise how I could use this myself to assess e.g. experiments. But then I can just do that in custom fields that I would consume myself.

rmarx commented 8 months ago

I do feel there is value in this even if you just get a general "algorithm family" out of it.

Say you're a new user of the library or a researcher and you -think- you've configured stuff to use Algorithm B instead of A, this is an easy way to check if that actually works correctly (or conversely to get an idea of what type of thing it's doing by default in the first place without digging through the code).

I do agree it shouldn't go beyond "newreno" or "bbrv1" or something like that, and I'm not even sure we need to specify any values in the qlog docs, just the field name.

LPardue commented 8 months ago

I dont buy that use case much. If you can actually configure an instance, then you should have confidence you know what you're doing, otherwise a lot of bets are off and I don't think logging helps. But if they want that capability, sticking the config in the trace description seems fine to me.

Algorithm names on their aren't useful IMO - especially if theres no standard for them.

quiche supports different algorithms and hyststart++ as different composable config.

This event already supports articulating various stuff that could be relevant but not standard if someone really wants it inside the event

this event can contain any number of unspecified fields to support different recovery approaches.

hlandau commented 8 months ago

Yeah, my intention here is definitely not any kind of "interop". But it is useful information to know the broad class of family being used IMO.

I don't think the "people should know what they're doing" argument holds water — people may be switching between different algorithms frequently for experimental purposes and thus have a large collection of QLOG traces using different algorithms.

It's not essential since people can add custom fields. But if many different implementations are going to log this, it's certainly more convenient if they use the same field name.

LPardue commented 8 months ago

We need a higher bar than it might be common and helpful. Standardising a field requires explaining what's its purpose is and how it should be used.

If I want to experiment with congestion control I will likely be playing with several parameters. A high level family name cant articulate that. Instead, I might want to write my own structured format for all the knobs that I can control. In which case the standard field would be redundant.

hlandau commented 8 months ago

After reflecting on the "philosophy of qlog" in these two comments: https://github.com/quicwg/qlog/issues/371#issuecomment-1900972178

I think I now agree and that we don't need to do this.

rmarx commented 7 months ago

Discussed during editors meeting. Closing without action; proposed solution is to use custom per-implementation fields if needed (since no 2 Reno implementations are even alike and we don't want full parameters for every possible config)