Increase peer max count significantly #2754
Conversation
|
While I would like to see nodes accepting more peers, did you see this today: #2594 (comment) And @bladedoyle is in gitter right now talking about high cpu usage due to having too many peers. |
|
These seem more related to too many txhashset downloads. While increasing peer counts would make that more likely, we should really just throttle those to reasonable levels. I'm actually suspecting someone is being obnoxious right now. In any case, I wouldn't want to block something that'd improve the network overall because we're being stupid in allowing any txhashset download. |
|
I think we also should increase
|
|
My seed is a 2 vCPUs, 2GB machine, but I agree it'd be good to do some profiling to cut some of that CPU activity. I'm not sure I understand the part about |
|
@ignopeverell The main reason is that we propagate new headers (and the rest) to |
|
If we do this we should do review our logic for the various bits of broadcast logic. Even if we decide to connect to 250 peers, we may not want to broadcast txs (blocks, headers) to all 250 peers. |
|
@antiochp you are right, we have it for headers only https://github.com/mimblewimble/grin/blob/master/p2p/src/peers.rs#L336 and we use max for tx and compact blocks https://github.com/mimblewimble/grin/blob/master/p2p/src/peers.rs#L373 |
config/src/comments.rs
Outdated
| @@ -228,7 +228,7 @@ fn comments() -> HashMap<String, String> { | |||
| #ban_window = 10800 | |||
|
|
|||
| #maximum number of peers | |||
| #peer_max_count = 25 | |||
| #peer_max_count = 250 | |||
There was a problem hiding this comment.
Unrelated - but can we use const from code elsewhere to parameterize/templatize these values?
I haven't looked to closely into how painful this would be to do this but its a .rs file so should be possible?
|
Had a look before, but I think if we're connected to 250 we actually want to relay to 250. Alternatively, I could reduce to say 100 to start with until we have figured out CPU bottlenecks. Thoughts? |
| @@ -228,7 +228,7 @@ fn comments() -> HashMap<String, String> { | |||
| #ban_window = 10800 | |||
|
|
|||
| #maximum number of peers | |||
| #peer_max_count = 25 | |||
| #peer_max_count = 125 | |||
There was a problem hiding this comment.
Shouldn't this option be uncommented? Otherwise, it's not really a default but more like a suggestion.
There was a problem hiding this comment.
Or is the default driven by PEER_MAX_COUNT below?
There was a problem hiding this comment.
The default is driven by PEER_MAX_COUNT. Ideally config comments that are generated should be driven by that, as @antiochp mentioned, but it's a little out of scope for this PR.
antiochp
added
the
release notes
|
Just stats: 14:00:52 up 12 days, 13:42, 1 user, load average: 1.93, 2.26, 1.82 RX packets 91974348 bytes 16989604567 (15.8 GiB) 20190626 13:59:15.499 DEBUG grin_servers::grin::seed - monitor_peers: on 0.0.0.0:3414, 196 connected (176 most_work). all 4467 = 199 healthy + 5 banned + 4263 defunct |
I think several of our peer-related issues stem from a max count of peers that's much too low. At this point a node can easily support 250 peers on a low-spec machine and so I don't see a lot of reasons to have this limit so low. Having it much higher should help a lot in connecting and keeping peers.