Skip to content

connectd: only charge gossip queries against the CPU budget - #9601

Merged
nGoline merged 3 commits into
ElementsProject:masterfrom
nGoline:fix/gossip-throttle-ordinary-gossip
Oct 6, 2026
Merged

nGoline merged 3 commits into
ElementsProject:masterfrom
nGoline:fix/gossip-throttle-ordinary-gossip

Conversation

@nGoline

@nGoline nGoline commented Oct 5, 2026 •

Copy link
Copy Markdown
Collaborator

This fixes CI after the gossip CPU throttle that shipped in v26.06.8. Since that change, valgrind runs fail on multi-peer offers tests (test_offer_with_private_channels_multyhop2, test_renepay.py::test_offers and test_offer_paths failed on #9582) with:

UNUSUAL connectd: That's weird: Throttling outgoing peer ...: too much CPU

The throttle is meant to bound the work of answering gossip queries, but it charged much more than that:

  • on the way in, every message connectd handles itself (pings, pongs, onion messages, custom messages, gossip forwarded to gossipd), not only query_channel_range / query_short_channel_ids;
  • on the way out, every check for pending query replies, which runs each time a peer's output queue drains, even with no query in flight;
  • the budget is split evenly across all connected peers, so on a node with many peers each share is tiny and those charges use it up.

Under valgrind those charges add up quickly, and the same thing happens on busy mainnet nodes: connectd stops reading from a peer for a second or more, which also delays its channel messages.

Commits:

  1. lightningd: add --dev-gossip-cpu-budget: sets only the CPU budget. --dev-throttle-gossip also shrinks the traffic limits, so a test using it can't tell which limit throttled a peer.
  2. tests: ordinary messages must not count against the gossip CPU budget (xfail(strict=True)): 200 pings to a node with a 10 usec budget and the normal traffic limits must not get the peer throttled.
  3. connectd: only charge gossip queries against the CPU budget: charge only reading the two query messages and answering them while one is pending (onion messages already have their own per-peer ratelimit), keep the even per-peer split with no minimum share (a minimum would let enough peers claim more than the whole budget), and log throttling once at debug. Removes the xfail.

Answering queries still counts, so test_gossip_query_channel_range_cpu_throttle still sees a flood of queries throttled.

Testing (docker):

  • the new test fails before the fix with Throttling incoming/outgoing peer ...: too much CPU on pings alone, and passes after it;
  • test_gossip.py (59 passed), test_askrene.py (37), test_xpay.py (35), all throttle tests, source checks;
  • under VALGRIND=1: test_offer_with_private_channels_multyhop2, test_renepay.py::test_offers and test_offer_paths pass.

🤖 Generated with Claude Code

@nGoline
nGoline requested a review from jaonoctus October 5, 2026 20:57
Comment thread connectd/multiplex.c Outdated
Comment thread tests/test_gossip.py
@nGoline
nGoline force-pushed the fix/gossip-throttle-ordinary-gossip branch from 92d9b52 to 9dccb6e Compare October 6, 2026 13:41

@jaonoctus jaonoctus left a comment •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see you didn't sign the commits this time, not sure if that's intentional. Other than that, lgtm

@nGoline
nGoline force-pushed the fix/gossip-throttle-ordinary-gossip branch from 9dccb6e to f0c3928 Compare October 6, 2026 15:27
nGoline and others added 3 commits October 6, 2026 15:28
--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

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The CPU throttle charged every message connectd handles itself against
a budget split evenly across all connected peers, and on the way out it
charged every check for pending query replies, which runs each time the
output queue drains.  On a node with many peers each share is tiny, so
ordinary traffic (gossip, pings, onion messages) used it up and we
stopped reading from the peer for a second or more, delaying its channel
messages too, with an UNUSUAL "Throttling ... peer ...: too much CPU".

Charge only the gossip queries the throttle exists to bound: reading
them, and answering them while one is pending.  Onion messages have
their own ratelimit.  Keep the even split, with no minimum share: a
minimum would let enough peers claim more than the whole budget between
them, and a small share now only slows that peer's queries.  Log
throttling at debug: a peer hitting its budget is what it's for.

Changelog-Fixed: connectd: ordinary gossip, pings and onion messages no longer count against the gossip query CPU budget, so busy nodes no longer throttle their peers.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@nGoline
nGoline force-pushed the fix/gossip-throttle-ordinary-gossip branch from f0c3928 to 0926d93 Compare October 6, 2026 18:42
@nGoline
nGoline merged commit 019a5e1 into ElementsProject:master Oct 6, 2026
9 checks passed
@nGoline
nGoline deleted the fix/gossip-throttle-ordinary-gossip branch October 6, 2026 18:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants