tests: compare medians in test_no_delay instead of means#9330
Open
ksedgwic wants to merge 1 commit into
Open
Conversation
test_no_delay compares mean round-trip times with a 3-standard-error margin. On loaded CI runners the trip-time distribution is heavy-tailed enough that the margin can exceed the entire effect being measured: in the ElementsProject#9329 failure the margin came to 45.3ms while the true Linux effect is one delayed-ACK quantum (40ms - the docstring's ~200ms figure is other platforms), so the assertion failed with the effect cleanly present (saving 44.6ms). The stall shifts the whole distribution by the quantum, so the median difference detects it regardless of the noise tail. Compare medians, requiring half the quantum on Linux; on platforms without a measurable stall keep the not-slower sanity check with the same slack. Fixes: ElementsProject#9329 Changelog-None
Collaborator
Author
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.
test_no_delaycompares mean round-trip times with a 3-standard-error margin. On loaded CI runners the trip-time distribution is heavy-tailed enough that the margin can exceed the entire effect being measured: in the #9329 failure the margin came to 45.3ms while the measured saving was 44.6ms - and the true Linux effect comes in 40ms delayed-ACK quanta (the old docstring's ~200ms figure is other platforms), so the assertion failed with the effect cleanly present.The Nagle stall shifts the whole distribution by whole quanta, so the median difference detects it regardless of the noise tail. Compare medians, requiring half a quantum on Linux; platforms without a measurable stall keep the not-slower sanity check with the same slack.
Measured locally on an idle machine: median trip 196.6ms with Nagle vs 113.4ms without, an 83.3ms median saving (two quanta - a payment round trip contains several stalled exchanges) against the 20ms the assertion requires.
Fixes #9329