Speedtest refactor #118
Speedtest refactor #118
Conversation
Currently dgram-speed-server is annoying because it terminates after each connection. This makes it accept multiple connections (like speedtest-server) which makes testing against it multiple times much less annoying. This also changes the final packet send back at the end of the test by the server to include the number of received packets so that the client can display that information at the end of the test (previously only the server displayed it).
When testing it is highly inconvenient to have the pubkey that the client needs continuously changing.
jagerman
force-pushed
the
dgram-speedtest-repeat
branch
from
5865448 to
9e74d83
Compare
There was a problem hiding this comment.
Thanks for knocking this out, this gave me an idea if you think it could be useful.
When we want to A/B test between dev and a feature branch (ex: with manual_routing), we can invoke the client datagram binary from the server binary by passing -A to the server binary. Theoretically we can now also run the datagram speed test as part of CI runs
It does seem useful, but trying to do it via a fork-exec in C++ seems overly complicated compared to doing this in shell with a pregenerated keypair: ./tests/speedtest-client --remote-pubkey IHoGeJKCHiXXcPH7oMR8Ef9LgT5UFi7Onrg54HYjGrY -HXg &
./tests/speedtest-server -HX --seed ASNFZ4mrze8BI0VniavN7wEjRWeJq83vASNFZ4mrze8Powershell can supposedly do something similar. (By complications, I don't only mean the current implementation you linked to, but also that there are a bunch of edge cases that your current implementation doesn't handle, such as using a different build directory, or trying to invoke it from somewhere other than the root project directory). But if we did have such a flag, it seems like it would be easier/nicer to rearchitect the speedtest code slightly so that it could simply run the server and client in two separate threads, and then maybe just have a single speedtest binary that has |
It felt restrictive (and a little unintuitive) to have to specify the dgram callback at the endpoint because then I can't have a different callback for individual connections or for incoming versus outgoing connections. It also feels a little weird because stream callbacks are only at the listen/connect level but not at the endpoint level. This commit allows also specifying the datagram callback to `connect()` or `listen()` -- if specified as such, it takes precedence over the one given to endpoint.
- Extract the actual server code into a dedicated object rather than being plunked into the `main` function. - Remove hash/checksum functionality. They were basically a very early data consistency test that just slowed everything down and so aren't actually useful for a speed test (that sort of testing is done via the unit tests). - Combine the datagram and stream speedtest server functionality into a single object and binary: `speedtest-server` now is a single server that works for either client type. Still TODO: - extract the client implementation into a Client class, similarly to Server. (This is a bit more involved, though, because Client has a bunch of options). - Combine current server and the two client modes into a single `speedtest` binary with `-s,--server`, `-c,--stream-client`, and `-d,--datagram-client` flags to set the mode. - Add `-C,--stream-self` and `-D,--datagram-self` flags to do a self-test: these would run both a server and client where the client connects to the server, does the test, then the program exits. (-D would be the datagram equivalent). If you specify *both* then you would first do one, then the other, then exit.
jagerman
changed the title
|
Convert to draft with related refactoring:
|
Currently dgram-speed-server is annoying because it terminates after each connection. This makes it accept multiple connections (like speedtest-server) which makes testing against it multiple times much less annoying.
This also changes the final packet send back at the end of the test by the server to include the number of received packets so that the client can display that information at the end of the test (previously only the server displayed it).