Closed phillip-stephens closed 1 month ago
After talking with Zakir, we can make this quite a bit simpler if we just re-use the existing connection in ExternalLookup
if the user/CLI doesn't suggest a new one. Closing this but leaving branch in case we ever want to re-visit. #445 has the new approach
Adds "name server stickiness" to lookups so Resolvers will prioritize processing queries from the same nameserver to avoid having to re-handshake TCP/TLS/HTTPS.
Changes
wireLookup
intowireLookupTCP
andwireLookupUDP
so the TCP variant could have connection infoAXFR
-AXFR
is unique in that if the user doesn't provide a name server, it first does an NS lookups for that domain. This means we should not suggest a NameServer for these lookups. It's a little gross, but I added a check inside the worker thread to discard the name server "suggestion" if we're doing AXFR. If a user specifies a name server (as opposed to our suggestion), this shouldn't be thrown awayWorkerPools
) than workers, I capped the number of pools at the--threads
count. Using nameservers > thread count will result in a performance penalty since we can't send the same name server lookups to the same workers anymore.Overview
In #431 (which this branch is based on, wanted to have this logic reviewed before merging into that to break up this larger feature), I added functionality with DoH and DoT connections that a given resolver would only re-handshake if the nameserver was different than the remote address on it's existing connection.
The issue is that in
ExternalLookup
here, if the user didn't provide a nameserver then a random one would be selected. Let's look at an example to see how this causes us to unnecessarily tear-down connections:./zdns google.com yahoo.com eBay.com --threads=2 --name-servers=1.1.1.1,8.8.8.8
--tls And let's say that after the random NS selection, this gives usIn this toy example, thread #0 would tear down it's TLS connection and re-establish one to
8.8.8.8
even though thread #1 already has a connection to8.8.8.8
.Load Imbalance
As another design consideration, all external resolvers do not behave equally.
I measured the resolution time for 7k queries and to what resolver they were sent to.
This led to the threads responsible for Google queries sitting idle while the Cloudflare ones were busy working.
Design
To address both re-using TCP connections and dealing with load imbalance, this PR implements a Priority and Global work queue.
A new
inputDeMultiplexor
chooses an external NS for each input line and passes it to the assigned priority queue. Each priority queue is for queries to a specific name-server, ex: @1.1.1.1. If the priority queue is full, then the demultiplexer will block on both the Priority/Global queue. In this way, we prioritize sending work to the threads which have a pre-existing connection to the name server, but also avoid large work imbalance issues.Similarly, threads will prefer to read from their Priority queue, before blocking on both Priority and Global queues.
Tradeoff: Work Imbalance vs. Connection Re-use
Every time a worker chooses a work item from the
Global
queue, it will have to re-handshake. However, without this Global/Priority queue split, workers with Priority on a very fast nameserver will sit idle when they could be doing work too. To showcase this, see the below experiment.I ran an experiment to check different points on this spectrum:
main
- no TCP connection re-usethis branch
- first try to pull from Priority, then either Global/PriorityAs the blocking time increases, the odds that a non-Priority thread will need to take an item from the Global queue to load-balance decreases. This decreases new TCP handshakes but increases the runtime.
3x runs per condition 7,000 domains run with
"A", "--verbosity=3", "--threads=100", "--tcp-only", "--name-servers=1.1.1.1,1.0.0.1,8.8.8.8,8.8.4.4",
Unaffected
--iterative
lookups will ignore this suggestion and chose a random root server. Since we don't support--tls
or--https
with--iterative
anyway, this isn't a concern.Performance
Using the benchmark (7k domains), edited the command to use
./zdns A --name-servers=1.0.0.1,1.1.1.1,8.8.8.8,8.8.4.4 --threads=100
(external lookups)main
branch8.31 s.
/ 14,006 packets on lab VM (varies between 8-10s)9.82 s.
/ 70,012 packets6.60 s.
/ 14004 packets10.29 s.
/ 44,226 packetsTesting
--no-recycle-sockets --tcp-only
to be sure that we're not creating persistent TCP connections if the user doesn't want that