Skip to content

Conversation

@shinrich
Copy link
Member

Adding format strings to log priority weight and dependence for a transaction so we can understand changes in performance characteristics when enabling HTTP/2 priority.

bryancall
bryancall previously approved these changes Sep 23, 2019
Copy link
Contributor

@bryancall bryancall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am OK with this.

@masaori335
Copy link
Contributor

This might give us some clue of H2 priority. But I'm bit wondering of the access log is the best way for logging H2 priority information, because the weight & dependence might be changed after the HttpSM::attach_client_session() call.

@shinrich
Copy link
Member Author

True, this will not identify changes due to explicit PRIORITY frames, but in my experience I haven't seen many of them. Just getting some visibility someplace was immensely useful for understanding unexpected behavior. This is better than my previous approach of creating one off builds with extra warning messages. Open to other ideas. Could be updated to override with information from the priority frames in a later PR.

@shinrich
Copy link
Member Author

Finally pushed an update to address @masaori335's comment

@masaori335
Copy link
Contributor

[approve ci]

Copy link
Contributor

@masaori335 masaori335 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks OK - better than nothing

@bryancall
Copy link
Contributor

[approve ci]

@bryancall
Copy link
Contributor

[approve ci autest]

@bryancall bryancall merged commit 89cf5af into apache:master Oct 18, 2019
@zwoop
Copy link
Contributor

zwoop commented Oct 21, 2019

This is pending A-A-Ron's massive refactoring changes I think...

@zwoop
Copy link
Contributor

zwoop commented Oct 22, 2019

Cherry-picked to v9.0.x branch.

@zwoop zwoop modified the milestones: 10.0.0, 9.0.0 Oct 22, 2019
@zwoop zwoop modified the milestones: 9.0.0, 8.1.0 Apr 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants