Skip to content
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

Trigger for H3PriorityUpdated #324

Open
rmarx opened this issue Jul 6, 2023 · 4 comments
Open

Trigger for H3PriorityUpdated #324

rmarx opened this issue Jul 6, 2023 · 4 comments
Assignees

Comments

@rmarx
Copy link
Contributor

rmarx commented Jul 6, 2023

I am wondering if the new H3PriorityUpdated event (see #312) needs some specified trigger field values.

Conceptually, you can get the necessary context from other events surrounding this event (e.g., if you get a H3FrameParsed with a HEADERS containing a priority field preceding this, that's a clear indicator that's the trigger). However, there are cases where this is less clear (e.g., default initialization to a value different from the spec, local override due to configuration or other logic, merging of client and server-sent priorities for the same stream, ...)

We don't NEED to define any triggers here (trigger is always defined on event data, just not necessarily specced in qlog), but I'm wondering if some default values would be useful here @LPardue?

e.g.,

? trigger: "client_directive" / "server_directive" / "local_override" / "local_default" / "merge" / "merge_override"
@LPardue
Copy link
Member

LPardue commented Oct 21, 2023

Personally, I don't think I'd have the ability to pass through enough context to my logging library to capture the trigger. But I think I need a bit more time to work on the implementation.

@rmarx
Copy link
Contributor Author

rmarx commented Mar 1, 2024

@LPardue any update on your opinion on this? I still think this would be useful in general, even if quiche can't pass it :) and i still like the values I proposed above. Are you opposed to a PR for this?

@rmarx rmarx self-assigned this Jun 27, 2024
@LPardue
Copy link
Member

LPardue commented Jul 9, 2024

I can live with it if you want to add it. But I'd like a better description of how the "trigger" field is a part of event data; see #429

@LPardue
Copy link
Member

LPardue commented Oct 21, 2024

Thinking on this some more, I don't think the values in the OP are actually a trigger. They are a result of applying the guidance in https://www.rfc-editor.org/rfc/rfc9218.html#name-merging-client-and-server-d. In other words, how the value of new was arrived upon. The client always sends a signal, the server always has to make a decision, including ignoring the client.

So I think what maybe what you want is an optional reason field that can be used to explain how. That might just contain values `client_signal_only, client_server_merged, local_policy".

Then if you want a trigger, it should track why the update happened e.g. a client PRIORITY_UPDATE, some spontaneous server-local decision, so perhaps just client_signal, local_decision, other. For example, I have a Priority extension for changing on a byte schedule. I might extends the trigger field to have a "schedule" value, and still use a reason "local_policy".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants