-
Notifications
You must be signed in to change notification settings - Fork 13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add events to explicitly indicate send blocking #132
Comments
As far as I could intuit, MS had events for blocked and unblocked. This seemed to be an application/ implementation event, rather than something strictly transport related. So I wonder what type of category it would fit into. |
https://github.com/microsoft/msquic/blob/main/src/inc/quic_trace.h#L44 typedef enum QUIC_FLOW_BLOCK_REASON {
QUIC_FLOW_BLOCKED_SCHEDULING = 0x01,
QUIC_FLOW_BLOCKED_PACING = 0x02,
QUIC_FLOW_BLOCKED_AMPLIFICATION_PROT = 0x04,
QUIC_FLOW_BLOCKED_CONGESTION_CONTROL = 0x08,
QUIC_FLOW_BLOCKED_CONN_FLOW_CONTROL = 0x10,
QUIC_FLOW_BLOCKED_STREAM_ID_FLOW_CONTROL= 0x20,
QUIC_FLOW_BLOCKED_STREAM_FLOW_CONTROL = 0x40,
QUIC_FLOW_BLOCKED_APP = 0x80
} QUIC_FLOW_BLOCK_REASON; The lack of any flags means it's unblocked. |
And the connection has a set of flags, as well as each stream has its own set. |
I guess my intuit was a bit off 😜 |
Leaning on the side of let folks log what they like, and focus these drafts on transport events. |
I still personally feel these would be very useful to have. I'm not sure why these wouldn't be part of "transport events" @LPardue? Seems like being blocked on e.g., flow control is a very transport-esque thing? Right now, we can somewhat deduce this information by e.g., looking at logged values for |
There's no QUIC message to tell your peer you can't send it data due to pacing or amplification prevention. So to do that in qlog here, we'd have to invent some new category of event and explain to people what it means and how to use it (produce and consume). Yes it could be interesting but I lean on punting it to be done when there's a good case about why it needs to be defined in an interoperable way in qlog. |
I still think we can live without standardizing this today. It seems a generic "application data" category of event can be defined that would work for e.g. both HTTP/2 and HTTP/3 sending on a stream could be blocked due to stream flow control or the transport congestion control. That's simple enough to write in a new spec. |
Fixes #132 (if merged). It struck me that its the "data_moved" action that can be blocked, so rather than add a separate event, just add an optional field to an existing event. This helps ensure we can represent both stream and datagram data with all relevant fields.
msquic has separate events that indicate if the sender was blocked/delayed/idle and due to which cause (e.g., pacing, flow control limited, cwnd limited, etc.).
This is quite useful for debugging. While some of this can be deduced from looking at other existing qlog events, adding some more explicit events for this could be interesting.
@nibanks: maybe you could give some pointers/links to where these events are defined for msquic for reference?
cc @LPardue
The text was updated successfully, but these errors were encountered: