mirror of
https://github.com/hickory-dns/hickory-dns.git
synced 2026-04-25 11:15:54 +03:00
[GH-ISSUE #532] dns-over-https should reuse connections #225
Labels
No labels
blocked
breaking-change
bug
bug:critical
bug:tests
cleanup
compliance
compliance
compliance
crate:all
crate:client
crate:native-tls
crate:proto
crate:recursor
crate:resolver
crate:resolver
crate:rustls
crate:server
crate:util
dependencies
docs
duplicate
easy
easy
enhance
enhance
enhance
feature:dns-over-https
feature:dns-over-quic
feature:dns-over-tls
feature:dnsssec
feature:global_lb
feature:mdns
feature:tsig
features:edns
has workaround
ops
perf
platform:WASM
platform:android
platform:fuchsia
platform:linux
platform:macos
platform:windows
pull-request
question
test
tools
tools
trust
unclear
wontfix
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
starred/hickory-dns#225
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Originally created by @kpcyrd on GitHub (Jul 15, 2018).
Original GitHub issue: https://github.com/hickory-dns/hickory-dns/issues/532
I've used the dns-over-https resolver from master for a bit and it seems it's creating a fresh tls connection for each lookup, so each lookup takes around 300ms for me. I think trust-dns-resolver should try to keep the connection alive for as long as possible to reduce the number of roundtrips needed, as creating a tls session is expensive in a low-latency context. :)
@bluejekyll commented on GitHub (Jul 15, 2018):
Hm. Mind sharing your code (though I won’t have much time to look at it as I’ll be on a plane and vacation the next couple of weeks)? It’s possible I screwed up that implementation, but if you have the same Resolver shared across uses, it shouldn’t recreate connections, and should share them across uses.
@kpcyrd commented on GitHub (Jul 15, 2018):
I've reduced my code to a standalone program to reproduce the issue:
Using sniffglue you can observe that a new tls connection is created for each query.
@bluejekyll commented on GitHub (Jul 15, 2018):
Ok, thanks. I'll have to review the usage of h2 to see why this is happening...
@bluejekyll commented on GitHub (Aug 29, 2018):
I haven't had time to look into this, but was in the code area that may be effecting this yesterday. This boolean option in the h2 connection may be wrong, and it might be as simple as setting this to false:
github.com/bluejekyll/trust-dns@6777f648df/https/src/https_client_stream.rs (L277)@bluejekyll commented on GitHub (Oct 2, 2018):
Ok. Finally tracked this down. A little too much cloning going on. I have a patch locally that I need to clean up that resolves this.
Basically, ConnectionHandle type was cloning the connection details for a NameServer before connecting, causing all connections to connect like it was the first time every time.
@juliangehring commented on GitHub (Jan 2, 2019):
When I run @kpcyrd's example with trust-dns-resolver v0.10.2 and trust-dns-proto v0.6.1, wireshark still shows me one TLS initiatialisation (i.e. one TLS client hello) per request. Maybe I'm missing something, but is the TLS connection currently reused?
@bluejekyll commented on GitHub (Jan 2, 2019):
Over how many uses, what time period, and how many configured TLS endpints?
@juliangehring commented on GitHub (Jan 2, 2019):
I saw it consistently during 12+ runs over the last 5 days, all using the
cloudflare_httpsandcloudflare_tlsresolver config (1.0.0.1).@bluejekyll commented on GitHub (Jan 2, 2019):
Do you know the frequency of the requests? I’m wondering if the connections are being closed due to timeout. If you want, you can enable debug log lines for the proto crate and see what’s being reported.
@juliangehring commented on GitHub (Jan 3, 2019):
Here the full example (almost identical to the original one):
@juliangehring commented on GitHub (Jan 3, 2019):
It's 3 requests, one every 1.2sec (1000ms sleep + around 200ms to handle the request). Here an excerpt of a wireshark log for the example code:
The example uses the default
ResolverOptswhich specifies a timeout of 5sec.@juliangehring commented on GitHub (Jan 3, 2019):
And here the debug.log.
@bluejekyll commented on GitHub (Jan 4, 2019):
Getting back to you late on this, but I leave this here as a reference for others: The library uses the "standard" log system. with
env_loggeryou can enable all logs with this environment vairable:RUST_LOG=trust_dns=debug,trust_dns_client=debug,trust_dns_proto=debug,trust_dns_resolver=debug@bluejekyll commented on GitHub (Jan 4, 2019):
Thanks for posting the log, it does look like the stream is being closed immediately, based on this:
I'll try to look into this when I have some time. There may have been a regression recently. If so, we'll need to build a test specifically for this case to make sure that doesn't happen again.
@juliangehring commented on GitHub (Jan 4, 2019):
Thanks for looking into this. Let me know if I can provide any more details for identifying the issue.
@bluejekyll commented on GitHub (Jan 7, 2019):
Ah, I looked at your code a while, trying to figure this out, and then I realized, you're using the synchronous Resolver, and not the AsyncResolver. In a nutshell, the synchronous Resolver does not maintain a long-lived connection pool.
I'll open a separate issue to describe. In any case, I did validate that the root of this issue is resolved.