Split send/recv reporting in bidir tests#57
Merged
Conversation
For --bidir tests, expose per-direction bytes and throughput so asymmetric-link measurements are meaningful. Unidirectional tests are unchanged: existing bytes_total / throughput_mbps are the single- direction number and the new fields are serde-skipped as None. Protocol (protocol.rs): - TestResult gains optional bytes_sent, bytes_received, throughput_send_mbps, throughput_recv_mbps - AggregateInterval gains the same fields (populated in a follow-up) Stats (stats.rs): - TestStats::total_bytes_sent() / total_bytes_received() aggregate per-stream atomics that already tracked each direction separately Server (serve.rs): - New directional_totals() helper computes per-direction totals when the test is bidir, returns all-None otherwise - Wired into both run_test (TCP) and run_quic_test (QUIC) result construction Output: - plain.rs: three-column 'Send / Recv / (Total:)' format for bidir - csv.rs: four new columns appended; empty for unidir - json.rs: automatic via serde skip_serializing_if No changes to client-side overlay — server is authoritative for both directions. Per-interval split and per-stream split deferred to follow-ups once the summary-level change lands.
TUI stats panel renders '↑ X / ↓ Y' when direction is Bidir and the server has reported per-direction throughput. Falls back to the combined throughput otherwise (older servers, unidir, early intervals before any progress). Tests added: - stats::total_bytes_sent / total_bytes_received split correctness - output_plain shows 'Send/Recv/(Total:)' for bidir, single-line for unidir - integration: bidir TestResult populates the split Options; unidir leaves them None
Two issues surfaced in review: 1. TestResult send/recv were reversed from the client's perspective. Server-local stats.total_bytes_sent() is what the server sent — which, from the client's POV that reads TestResult, is what the client *received*. Swap in directional_totals() so the wire format matches the client's expectation (bytes_sent = bytes the client sent). 2. TUI never showed the live ↑/↓ during a running test because only TestResult carried the split; AggregateInterval/TestProgress didn't. Track per-direction interval deltas in StreamStats (last_bytes_sent / last_bytes_received), extend IntervalStats, add to_aggregate_with_direction() that swaps and populates AggregateInterval's new fields for bidir intervals, thread through TestProgress and on_progress() so the TUI shows the split live.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Resolves #56.
--bidirnow reports per-direction bytes and throughput instead of just the combined sum, which was misleading on asymmetric links — users had to runxfrandxfr -Rseparately to get the numbers they actually wanted.Example output (bidir)
Plain text:
JSON adds
bytes_sent,bytes_received,throughput_send_mbps,throughput_recv_mbpsalongside the existingbytes_total/throughput_mbps.CSV gets four matching columns.
TUI shows
↑ 17.64 Gbps / ↓ 28.94 Gbpsin the throughput panel, live during the test.Design notes
Option<T>with serde defaults — older clients ignore them, older servers leave them None (client falls back to combined total)bytes_sentinTestResult/AggregateIntervalmeans "bytes the client sent" (its upload direction). Server-local counters are swapped before emission so the client reads its own direction correctly.last_bytes_sent/last_bytes_receivedinStreamStatssoAggregateIntervalcan carry real interval throughput per direction (not just end-of-test totals)StreamResultdeferred to a follow-up — MVP is aggregate onlyTest plan
cargo fmt --checkcargo clippy --all-features --all-targets -- -D warningscargo test --all-features— 93 unit + 55 integration + 18 protocol tests pass (3 new regression tests)Send(default 128KB buffer) is slower thanRecv(from server's 4MB high-speed buffer), matching expected asymmetry