Support server refresh concurrency

[keatonLiu]

We can simultaneously refresh all servers instead of refresh them one by one, which can greatly improve startup speed without visible loss of accuracy

[jedisct1]

Sure, but by doing so, the latency measurements become unreliable. Not so much of an issue with a super snappy WiFi network, but this is an issue when the link is not that great.

[jedisct1]

For DoH/ODoH/DoOH servers without an IP address, most of these would also use the fallback servers, which is something we want to minimize. With sequential refreshes, as soon as a server is ready, fallback servers can be ignored.

[jedisct1]

I'm also a bit concerned about CPU/memory usage, as a lot of people are running dnscrypt-proxy on routers with limited resources. It may cause the server to not be able to start at all, or to freeze due to CPU spikes.

Maybe that should be configurable.

[keatonLiu]

I agree that it can be configurable, because for most home PC, the bandwidth and cpu cost can be ignored, while achieving significant performance improvement. If use sequential refresh, it may take several minutes to find an optimal path, compared to only a few seconds for simultaneous refresh

[lifenjoiner]

The model is: burst at a time, finish in several seconds.

I would suggest setting the Maximum Concurrency.

Besides the advantage and previous disadvantages, think about:

1. 300+ at a time could be suspicious, in security.
2. If there is a network accident for 10s, there could be no available servers.

[lifenjoiner]

@keatonLiu
It's a pity that a good idea lost contact.
I've pushed my suggestions to your update branch.

[keatonLiu]

@keatonLiu It's a pity that a good idea lost contact. I've pushed my suggestions to your update branch.

Thank you for your ideas, I'm recently busy looking for an internship, I'll check it out and update soon.

[keatonLiu]

I think if the liveServers variable is not atomic there will be a race: 6d179bb

[lifenjoiner]

Return innerLen and remove liveServers is simpler.

[keatonLiu]

Return innerLen and remove liveServers is simpler.

Sure!

[lifenjoiner]

LGTM.

[lifenjoiner]

My previous negligence is fixed.

[keatonLiu]

how about combine all the ifs together, which looks better

[lifenjoiner]

how about combine all the ifs together, which looks better

Good catch. But I'm not sure about the first 2.

[lifenjoiner]

One more thing, the change from Lock() to RLock() will allow serving client requests, on one hand increases the availability, on the other hand could break the sorted latency list.

[keatonLiu]

Do you mean that the list will be changed during output? I think RLock only allows concurrent reading, so I don't think the list will be changed during serving client requests. What's more, the sorted list is only about the initialRtt, which is only changed by calling the refresh function.

[lifenjoiner]

Amend:
... could break the sorted latency log list, if the query log is also written to the same file (stdout for example).

Not saying the serversInfo.inner.

[keatonLiu]

Amend: ... could break the sorted latency log list, if the query log is also written to the same file (stdout for example).

Not saying the serversInfo.inner.

Oh, that's very detail, I haven't thought that. Do I need to change back?

[lifenjoiner]

I'm not saying it's a bad thing. Let's see what @jedisct1 will say.

[lifenjoiner]

@keatonLiu
I reverted a commit to make this a atomic PR: One PR for one thing.
Those behavior changes are less related to this topic. They may worth a new PR.

[lifenjoiner]

Thank you! You can submit other improvements within new PRs.