Conversation
michaelsproul
left a comment
michaelsproul
left a comment
There was a problem hiding this comment.
LGTM but would like a 2nd opinion from @pawanjay176
|
Yeah. It might be worth holding this off from the next release. We can potentially merge this if the issue becomes severe after the next release. |
michaelsproul
added
blocked
RFC
AgeManning
force-pushed
the
disable-flood
branch
from
e3ba5d7 to
fd36bf5
Compare
|
what if we put this behind a CLI flag and merged it to unstable? |
Agree with this. Would be nice to toggle flood sub to compare. |
|
Yeah. I'll do that. I'll make a hidden CLI flag. We probably want to increase the mesh size a little if we are not flood publishing. |
AgeManning
added
ready-for-review
|
Hey @michaelsproul @pawanjay176 We are doing a bunch of network changes. What do you guys think about getting this in also? This disables flood publishing (because we don't really have a soln for handling it) and it increases the default mesh size to somewhat account for the lack of flood publishing. I also increased the heartbeat to 1s, to avoid some excessive gossip messaging. If you guys agree, lets get this in. If the network shows any signs of degredation we can make a patch release. |
|
Seems ok to me. I guess a patch release isn't much more work than telling everyone to add a flag like |
pawanjay176
left a comment
pawanjay176
left a comment
There was a problem hiding this comment.
Seems okay to me as well since the patch is easy if things don't work out
3 participants
This PR is aimed to improve message latency.
The network is seeing delays in message propagation. We suspect gossipsub flood publishing to be contributing to this.
A interim solution has been suggested in gossipsub: libp2p/rust-libp2p#3666 however further testing and analysis is required before moving down that path.
There can be gains in message latency in a number of areas, including improving the transport, introducing backpressure and adding more sophisticated logic around sending messages with known bandwidth limitations.
The recent change in the network subnet structure is going to push lighthouse to maintain a higher peer count. If flood publishing is correctly attributing to the message latency, I think its wise we temporarily disable it, especially as our peer count grows, whilst we explore improvements in other layers of libp2p.