Skip to content

multi: add new config option to toggle gossip rate limiting #4810

Merged
halseth merged 3 commits into
lightningnetwork:masterfrom
Roasbeef:gossip-throttle-config
Dec 1, 2020
Merged

multi: add new config option to toggle gossip rate limiting #4810
halseth merged 3 commits into
lightningnetwork:masterfrom
Roasbeef:gossip-throttle-config

Conversation

@Roasbeef
Copy link
Copy Markdown
Member

In this PR, we add a new legacy protocol config option to allow the itests to enable/disable gossip throttling for each new node (default is no throttling in the itests). We also pull in a commit meant to restore stability to the set of itests by reducing the number of parallel itests execution instances to 1 (effectively disabling the parallelization).

Copy link
Copy Markdown
Contributor

@cfromknecht cfromknecht left a comment

Choose a reason for hiding this comment

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

changes read well to me. main question i have is do we know exactly why the current itest's updates are being ratelimited? have we confirmed that our tests actually send keep-alive updates?

Comment thread discovery/gossiper.go Outdated
Comment thread lncfg/protocol_legacy_on.go Outdated
@cfromknecht cfromknecht added this to the 0.12.0 milestone Nov 30, 2020
@Roasbeef
Copy link
Copy Markdown
Member Author

main question i have is do we know exactly why the current itest's updates are being ratelimited? have we confirmed that our tests actually send keep-alive updates?

I think mainly because our "24hr clock" in the tests is "true", and just even the update_chan_policy tests ends up doing a few updates that will have it run into the "1 update per block" limit for the set of nodes.

In this commit, we add a new option to toggle gossip rate limiting. This
new option can be useful in contexts that require near instant
propagation of gossip messages like integration tests.
@Roasbeef Roasbeef force-pushed the gossip-throttle-config branch from 7ab8001 to 447c9f2 Compare December 1, 2020 00:39
@Roasbeef Roasbeef closed this Dec 1, 2020
@Roasbeef Roasbeef reopened this Dec 1, 2020
Copy link
Copy Markdown
Contributor

@halseth halseth left a comment

Choose a reason for hiding this comment

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

Changes LGTM, we should do everything in our power to make travis green again.

However, it is all red for this PR now 😬

@halseth
Copy link
Copy Markdown
Contributor

halseth commented Dec 1, 2020

Merging this. Current itest failure seems related to #4782, looking into that separately.

@halseth halseth merged commit 8ec4697 into lightningnetwork:master Dec 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants