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

Improve congestion packet hover details by taking into account event order/index #52

Open
rmarx opened this issue Jun 29, 2021 · 0 comments

Comments

@rmarx
Copy link
Member

rmarx commented Jun 29, 2021

Currently, when hovering over a packet sent in the congestion diagram, we lookup metrics like bytes_in_flight by looking at the nearest events in time (instead of actually searching the closest events in terms of order/index in the qlog list).

This is because those events can be far removed from the actual packet_sent event (or non-existing) and so we use separate lists for this (the same we use to draw the various lines for these metrics on the graph).

If the qlog implementation doesn't use fine-grained timestamps (e.g., purely at ms granularity), this can cause many packet_sent and metrics_updated events to be grouped onto the same timestamp. The hover-behaviour will then show the last data in that "timestamp bucket" instead of properly showing the metric at that packet's time.

In the attached example, this is true for e.g., packet 190 and its surroundings. There are metrics_updated events interspersed, but they all have the same timestamp, causing the bytes_in_flight to be 54000 (latest for that timestamp).

1m.zip

This could be resolved by also considering event order/index when doing the lookup, but we currently don't keep track of event indices for individual events, so that would require additional memory or an additional loop over (part of) the events...

CC @junhochoi

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

1 participant