Repository navigation
pytest: make the gossip CPU throttle tests independent of machine speed - #47
Merged
Merged
Conversation
--dev-throttle-gossip shrinks the traffic limits along with the CPU budget, so a test can't tell which one throttled a peer. This sets only the total CPU budget for answering gossip queries (in usec per second), leaving the traffic limits alone. Changelog-None (cherry picked from commit 1390018)
The CPU throttle charges every message connectd handles locally: pings, pongs, onion messages, custom messages and gossip forwarded to gossipd, not only the gossip queries it exists to bound. The per-peer share is the budget divided by the number of connected peers, so on a busy node ordinary traffic exhausts it and we stop reading from the peer. Changelog-None (cherry picked from commit 9c617d7)
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
2 participants
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_gossip_query_channel_range_cpu_throttleandtest_gossip_throttle_no_unrequested_streamintermittently fail in CI withUnable to find "Throttling outgoing peer .*: too much CPU". Both relied on ten walks of a 15000 channel gossip store exceeding--dev-throttle-gossip's CPU budget, so on a fast runner with an optimized build the throttle never triggered.--dev-gossip-cpu-budget, which sets only the CPU budget for answering gossip queries and leaves the traffic limits alone, and its test that ordinary messages don't count against that budget. That test was added upstream as an expected failure ahead of the fix, which this tree already has, so the marker is removed here.test_gossip_query_channel_range_cpu_throttleuses--dev-gossip-cpu-budget=10instead of--dev-throttle-gossip, so one walk exceeds the budget on any machine and only CPU can throttle.test_gossip_throttle_no_unrequested_streamneeds a throttle that ends quickly, which a tiny CPU budget doesn't give (the wait grows with the overage). It now throttles by traffic: a 300 channel store, whose full range reply is over--dev-throttle-gossip's 1000 bytes a second.With a 1500 channel store, so that a walk costs little, the old versions of both tests fail every run with the error CI shows (4/4) and the new ones pass (6/6); with the full stores they pass 20/20.
Changelog-None