Be more agressive in getting connections to peers with relevant services. #8949
Conversation
Only allow skipping relevant services until there are four outbound connections up. This avoids quickly filling up with peers lacking the relevant services when addrman has few or none of them.
|
Concept ACK. Things that come to mind:
|
My intention is that it would get four connections up, and it would continue polling addrman until it finally got something relevant. Even absent seeding, eventually it should learn something via the four that are up or via feelers. The alternative I considered to the connect-to-relevant commit was adding something to periodically disconnect one of the peers without relevant services if all outbound slots were full. I decided this would be a more invasive change. A third alternative would be to allow a feeler with relevant services to replace a peer without relevant services; this would still probably be a more invasive change (but maybe only 3 LOC, e.g. if this is a feeler, and relevant, scan outbound peers, and if you find a non-relevant one, set disconnect on it instead). I did not consider any alternatives to the DNSseed commit. I'd rather not do anything here which would complicate a 0.13 backport. |
| @@ -1467,7 +1467,11 @@ void CConnman::ThreadDNSAddressSeed() | |||
| MilliSleep(11 * 1000); | |||
|
|
|||
| LOCK(cs_vNodes); | |||
| if (vNodes.size() >= 2) { | |||
| int nRelevant = 0; | |||
| BOOST_FOREACH(CNode* pnode, vNodes) { | |||
There was a problem hiding this comment.
This is meant for backport, so no cpp11
There was a problem hiding this comment.
Only backport to 0.13 though, which does have c++11?
|
@gmaxwell What are the disadvantages of forcing dns seeding on every startup? (Or even running a re-seed every 24 hours)? |
Increasing load on the DNS seeds, as well as privacy - when the DNS seed gets a request every time it still knows you're running a node. For these reasons the number of requests to DNS seeds is minimized (see also #4559). There is an option to force querying them though. |
|
Tested both with and without the patch. I was unable to replicate the poor behaviour without the patch, but observed no ill effects with the patch, which seems logical in any case. tACK 443f8116d1c4e2fd28c4730f085db41d9088681b @rebroad DNS seeds are more of a way for new nodes to get started to join the network which once joined, they can learn about other nodes directly from the network. We shouldn't be putting too much reliance on the seeds. |
We normally prefer to connect to peers offering the relevant services. If we're not connected to enough peers with relevant services, we probably don't know about them and could use dnsseed's help.
|
I added a comment that explains why we avoid the dnsseed and switched to the range based for (for some reason I thought that was too C++11 for 0.13). Thanks all. |
|
re utACK 4630479 |
Only allow skipping relevant services until there are four outbound connections up. This avoids quickly filling up with peers lacking the relevant services when addrman has few or none of them. Github-Pull: bitcoin#8949 Rebased-From: 9583477
We normally prefer to connect to peers offering the relevant services. If we're not connected to enough peers with relevant services, we probably don't know about them and could use dnsseed's help. Github-Pull: bitcoin#8949 Rebased-From: 4630479
Currently, we make 40 requests to addrman for peers with our relevant services then give up and take a node without them. When relevant services are rare in our addrman data this means we can get all 8 peers up without them. If they happen to be reliable peers they may stay up filling our connection slots for months, even though our addrman has since been populated with plenty of relevant peers.
To avoid this this PR changes the behavior to be only willing to ignore relevant services until half the outbound connections are up.
Also, dnsseed does not fire if we have at least two connections (in or out) 11 seconds after startup. It currently doesn't perform any sanity checking of the connections-- they could be two lite-client inbound peers that will never send us addr message and our addrman might be filled with stupidity. To improve that somewhat, we check that they at least send the relevant service flags before deciding to not request dnsseeding.