Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Querying DNS seeds too frequently #15434

Closed
ajtowns opened this issue Feb 18, 2019 · 2 comments · Fixed by #16939
Closed

Querying DNS seeds too frequently #15434

ajtowns opened this issue Feb 18, 2019 · 2 comments · Fixed by #16939
Labels

Comments

@ajtowns
Copy link
Contributor

ajtowns commented Feb 18, 2019

Despite being running most of the time, and having a database of tens of thousands of peers, my node seems to query the DNS seeds each time I restart it, which doesn't seem ideal from a privacy perspective?

eg from my debug.log:

2018-10-25T18:03:51Z Loaded 70360 addresses from peers.dat  172ms
2018-10-25T18:04:02Z Loading addresses from DNS seeds (could take a while)
2018-10-28T23:00:06Z Loaded 70368 addresses from peers.dat  153ms
2018-10-28T23:00:17Z Loading addresses from DNS seeds (could take a while)
2018-11-18T21:06:11Z Loaded 70033 addresses from peers.dat  1105ms
2018-11-18T21:06:22Z Loading addresses from DNS seeds (could take a while)
2018-12-19T06:37:23Z Loaded 71275 addresses from peers.dat  1953ms
2018-12-19T06:37:34Z Loading addresses from DNS seeds (could take a while)
2018-12-27T08:03:36Z Loaded 71670 addresses from peers.dat  1259ms
2018-12-27T08:03:47Z Loading addresses from DNS seeds (could take a while)
2019-02-18T08:46:48Z Loaded 70040 addresses from peers.dat  165ms
2019-02-18T08:46:59Z Loading addresses from DNS seeds (could take a while)

I think this is due to (1) many of the peers in my peers.dat not being alive, which means that (2) most outgoing connection attempts fail and stall for 5 seconds (nConnectTimeout), so (3) since the DNS seed thread waits for 11 seconds, only 3 or 4 attempts are generally made, with only 0 or 1 peers successfully connecting, so the threshold of 2 or more relevant peers isn't reached.

It might make sense to increase the DNS threads sleep time correspondingly? Something like:

-        if (!interruptNet.sleep_for(std::chrono::seconds(11)))
+        if (!interruptNet.sleep_for(std::chrono::milliseconds(nConnectTimeout * 7)))

perhaps?

@fanquake fanquake added the P2P label Feb 18, 2019
@naumenkogs
Copy link
Member

What you're saying sounds reasonable. However, I tried to apply your suggested change and it didn't change the behavior for me (but I also have less populated peers.dat).
Did you try to apply this change?

@ajtowns
Copy link
Contributor Author

ajtowns commented Feb 28, 2019

What you're saying sounds reasonable. However, I tried to apply your suggested change and it didn't change the behavior for me (but I also have less populated peers.dat).
Did you try to apply this change?

Yeah, it worked fine for me, but I didn't try very hard, so getting 2 successes out of ~7 attempts might have just been good luck? I also added a LogPrintf in ThreadOpenConnections() to report outgoing connection attempts; duplicates a -debug=net log messasge, but was less noisy for quick debugging.

I'm getting maybe 20% of outbound attempts succeeding, which if my maths isn't too far off says 8 attempts should be ~50% chance of 2 connections, 18 attempts should give 90% chance, 31 for 99%, and 40 for 99.9%. Though having now tried stopping and restarting a few times in quick succession, I'm getting higher success rates.

Might be better to run a few extra threads that just try to connect to previously seen peers at startup to get reconnected to the network faster?

sidhujag pushed a commit to syscoin/syscoin that referenced this issue May 31, 2020
96954d1 DNS seeds: don't query DNS while network is inactive (Anthony Towns)
fa5894f DNS seeds: wait for 5m instead of 11s if 1000+ peers are known (Anthony Towns)

Pull request description:

  Changes the logic for querying DNS seeds: after this PR, if there's less than 1000 entries in addrman, it will still usually query DNS seeds after 11s (unless the first few peers tried mostly succeed), but if there's more than 1000 entries it won't try DNS seeds until 5 minutes have passed without getting multiple outbound peers. (If there's 0 entries in addrman, it will still immediately query the DNS seeds). Additionally, delays querying DNS seeds while the p2p network is not active.

  Fixes bitcoin#15434

ACKs for top commit:
  fanquake:
    ACK 96954d1 - Ran some tests of different scenarios. More documentation is being added in bitcoin#19084.
  ariard:
    Tested ACK 96954d1, on Debian 9.1. Both MANY_PEERS/FEW_PEERS cases work.
  Sjors:
    tACK 96954d1 (rebased on master) on macOS 10.15.4. It found it useful to run with `-debug=addrman` and change `DNSSEEDS_DELAY_MANY_PEERS` to something lower to test the behaviour, as well as renaming `peers.dat` to test the peer threshold.
  naumenkogs:
    utACK 96954d1

Tree-SHA512: 73693db3da73bf8e76c3df9e9c82f0a7fb08049187356eac2575c4ffa455f76548dd1c86a11fc6beea8a3baf0ba020e047bebe927883c731383ec72442356005
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
4 participants
@ajtowns @fanquake @naumenkogs and others