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

ndt7: save application level RTT samples #192

Open
bassosimone opened this issue Sep 25, 2019 · 3 comments
Open

ndt7: save application level RTT samples #192

bassosimone opened this issue Sep 25, 2019 · 3 comments
Assignees

Comments

@bassosimone
Copy link
Contributor

In #188, we have implemented the mechanism for measuring application-level pings RTT but we are not currently saving it. This should be done, to make the data actionable. We need to update the spec to mention this piece of data and change the code to save it.

@bassosimone
Copy link
Contributor Author

BTW, issue #201 seems to be a case where the RTT is actually 20-ish but BBR thinks it is much smaller, therefore, it would be beneficial to see what applevel-RTT says about the connection.

@mehtab11 mehtab11 added the P1 label Oct 7, 2019
@pboothe pboothe self-assigned this Oct 7, 2019
@darkk
Copy link
Contributor

darkk commented Jan 13, 2020

I've done some shallow refactoring to include base time.Time reference, save websocket-ping RTT and send it to client together with TCPInfo sample. Unfortunately, measuring L7 RTT while doing /upload or /download produces significantly skewed results those are hundreds milliseconds apart. That's expected at least due to TCP HOL, so I've added /ping endpoint to measure RTT cleanly.

The test produces expected difference between L7 and L4 RTT when TCP proxy is present on path. E.g. Tor Browser measures L7 / L4 RTT as 302.6 / 5.9 ms from my location in St. Petersburg to a VM in London.

It's also theoretically possible to modify /download spec to collect several L7 RTT samples before pushing the actual data to avoid HOL and gather the data from existing clients. It's unclear to me how it may affect existing logic at clients as I'm not that aware of the client code that is already rolled out.

What do you think, should it be /ping or should /download spec rather be amended?

@bassosimone
Copy link
Contributor Author

@darkk considering how other speed tests work, I think may be preferable to include an application level /ping phase to the experiment. I'm significantly less happy about modifying the expected behavior of the client during /download or /upload because that would complicate the implementation and we wanted to keep the client's implementation very easy.

darkk added a commit to censoredplanet/ndt-server that referenced this issue Jan 14, 2020
L7 ping may be significantly different from L4 pings and separate
endpoint `/ping` is used to keep spec intact till further discussion
happens.

See 3520181 and m-lab#192
@darkk darkk mentioned this issue Jan 14, 2020
2 tasks
bassosimone added a commit that referenced this issue Jan 20, 2020
* ndt7: measure and report L7 RTT based on websocket ping

L7 ping may be significantly different from L4 pings and separate
endpoint `/ping` is used to keep spec intact till further discussion
happens.

See 3520181 and #192

* ndt7: fix liveness bug in L7-RTT measurement

Also, move that measurement data into to WSInfo sub-object as it's
currently unclear if AppInfo should be extended or not. It's part of
ndt7 spec :)

TODO: squash this commit into previous one

* Fix compatibility with go1.12

* ndt7: take one L7-RTT sample right before flooding the connection

Co-authored-by: Simone Basso <bassosimone@gmail.com>
darkk added a commit to censoredplanet/ndt-server that referenced this issue Jan 20, 2020
L7 ping may be significantly different from L4 pings and separate
endpoint `/ping` is used to keep spec intact till further discussion
happens.

One L7-ping sent before `/download` test to get one sample that is not
biased by the queue of bytes.

L7 ping is logged as `WSInfo` sub-object as it's currently unclear if
AppInfo should be extended or not. It's part of ndt7 spec :)

See 3520181 and m-lab#192
satomiie pushed a commit to satomiie/performance-ndt-server that referenced this issue Apr 16, 2020
L7 ping may be significantly different from L4 pings and separate
endpoint `/ping` is used to keep spec intact till further discussion
happens.

See 3520181 and m-lab/ndt-server#192
satomiie pushed a commit to satomiie/performance-ndt-server that referenced this issue Apr 16, 2020
* ndt7: measure and report L7 RTT based on websocket ping

L7 ping may be significantly different from L4 pings and separate
endpoint `/ping` is used to keep spec intact till further discussion
happens.

See 3520181 and m-lab/ndt-server#192

* ndt7: fix liveness bug in L7-RTT measurement

Also, move that measurement data into to WSInfo sub-object as it's
currently unclear if AppInfo should be extended or not. It's part of
ndt7 spec :)

TODO: squash this commit into previous one

* Fix compatibility with go1.12

* ndt7: take one L7-RTT sample right before flooding the connection

Co-authored-by: Simone Basso <bassosimone@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants