p2p: Speed up initial connection to p2p network #15502

1. 
   ajtowns commented at 2:53 pm on February 28, 2019: member

   This patch creates four threads for connecting to the P2P network on startup. This allows more attempts based on peers.dat before the timeout for connecting to two peers hits and the DNS seeds are queried for more peers. Once an initial quorum of outbound peers are connected, these extra threads end, and long term behaviour continues unchanged.

   Fixes #15434

   Marked as draft, looking for concept acks. Second patch just dumps some statistics on how successful connections are (successful = connection established, not a VERACK actually received). I get 20%-30% on initial startup; after running a while seems to be down to ~17%.

2. fanquake added the label P2P on Feb 28, 2019
3. 
   gmaxwell commented at 3:26 pm on February 28, 2019: contributor

   I’d love to be making more connect attempts to distinct hosts at once, but I’m not super eager to add more threads: each one implies quite a few megabytes of additional memory usage (both res and virt, from the per thread stacks and the malloc pools) which are problematic on low memory systems. Perhaps it’s ultimately worth it, but have you looked into just making the connects() non-blocking and handling them asynchronously?

   Startup isn’t the only time this should be useful, it would also be nice to reconnect quickly when we’ve lost our peers due to a network interruption.

   Aside, I don’t see what in this is preventing it from trying the same netgroup multiple times concurrently, which should probably be avoided. (Perhaps by just passing in a filter to addrman to tell it to return only hosts where netgroup%threads==thread ?)

4. 
   ajtowns commented at 3:56 pm on February 28, 2019: member

   Aside, I don’t see what in this is preventing it from trying the same netgroup multiple times concurrently, which should probably be avoided. (Perhaps by just passing in a filter to addrman to tell it to return only hosts where netgroup%threads==thread ?)

   It’s putting the addresses being tried in vFastConnect which contains a bool/address pair. The bool is true if the address is ready to be tried, and false if it’s finished with and ready to be replaced. If the bool is true, the address’s group is added to setConnected when picking new addresses to try, which should prevent the same netgroup from being tried multiple times.

   (I was trying for something not too intrusive, so haven’t looked at async/non-blocking connect())

5. 
   DrahtBot commented at 9:43 pm on April 5, 2019: member

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #19084 (net: add comments on dns seed behaviour by ajtowns)
   • #19064 (refactor: Cleanup thread ctor calls by hebasto)
   • #19029 (net: Fix CThreadInterrupt::sleep_for TSan issues by hebasto)
   • #18925 (Add extra thread for scheduler, move TorControl and OpenAddedConnections to scheduler by naumenkogs)
   • #16939 (p2p: Delay querying DNS seeds by ajtowns)

   If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

6. DrahtBot added the label Needs rebase on Apr 9, 2019
7. DrahtBot removed the label Needs rebase on Apr 11, 2019
8. fanquake added the label Needs Conceptual Review on Jul 30, 2019
9. 
   fanquake commented at 6:00 am on July 30, 2019: member

   @naumenkogs Did you want to have a look here and Concept ACK or NACK ?
10. 
    naumenkogs commented at 3:23 pm on July 30, 2019: member

    I think the trade-off is straightforward, as Greg points out: bootstrap speed vs. memory usage.

    It certainly would be more attractive if we make non-blocking connections instead, but I don’t have an idea of how much effort that would require.

    If we decide to stick to the current proposal, perhaps we can pass this as an argument? Just –fast-bootstrap for now, and then we might merge it with something else (for example, with the default memory allocation for utreexo cache or whatnot) so that at the end we have high-memory mode? Let me know if this discussion happened before and there is a consensus that parameters like this is generally a bad idea :)

11. DNS seeds: wait for 5m instead of 11s if 1000+ peers are known

    Also check every 25s whether we've got enough active outbounds that DNS
    seeds aren't worth querying, and exit the thread early if so.

    c9aa069f14
12. NOMERGE: debug logging of successful/total connection attempts abc5b57656
13. speed up initial reconnection to p2p network 62686dcaf2
14. ajtowns force-pushed on Sep 23, 2019
15. 
    ajtowns commented at 11:04 am on September 23, 2019: member

    I think #16939 is enough to stop the privacy leak here, but would still be nice to connect to nodes more quickly. I think it might not be too hard to do a select() loop just for non-proxied (and non-tor) connections, so I’ll give that a go.
16. fanquake renamed this:
    Speed up initial connection to p2p network
    p2p: Speed up initial connection to p2p network
    on Sep 23, 2019
17. ajtowns added the label Up for grabs on Sep 17, 2020
18. 
    ajtowns commented at 7:23 am on September 17, 2020: member

    Dropping this for now
19. ajtowns closed this on Sep 17, 2020

    ---

20. DrahtBot locked this on Feb 15, 2022

Contributors
ajtowns gmaxwell DrahtBot fanquake naumenkogs

Labels
P2P Up for grabs Needs Conceptual Review