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

DNS client: TCP connection reuse for TCP and DNS-over-TLS #614

Closed
daareiza opened this issue Jan 3, 2018 · 1 comment
Closed

DNS client: TCP connection reuse for TCP and DNS-over-TLS #614

daareiza opened this issue Jan 3, 2018 · 1 comment

Comments

@daareiza
Copy link

daareiza commented Jan 3, 2018

Hi, while using the DNS client I have noticed a performance challenge if a TCP or DNS-over-TLS server is used to resolve queries, this compared against an UDP server (that said, 1.5x-2.0x times slower for TCP, and up to 4x times slower for DNS-over-TLS), so in the long term, there is a drastic performance drop for a heavily used DNS client (e.x.: local server proxy/forwarder).

That performance drop is mainly caused by the opening/closing of a new TCP connection for every DNS request, and the required handshake things that are inherit to the protocol (worse for a TCP TLS handshake).

So, as an improvement, it may be good to have an option to reuse an already made TCP connection and send the requests over it, reducing the handshake things over and over again, specially for a DNS-over-TLS server, where a cert is involved.

As a general idea:

  • Add a new bool flag to enable/disable the reuse of TCP connections (not valid for UDP) client.go#L30
  • Add a new client attribute with the reusable connection client.go#L30
  • On every request, check if the reusable connection is NOT nil/closed, and use that connection client.go#L160
  • If the connection is nil/closed, make the Dial call to establish a new connection client.go#L162
  • Avoid closing the connection after the message exchange finishes client.go#L167
  • Add an idle timeout to check for inactive connections and automatically close them
  • Add a handler to check/capture connection state changes (I don't know if that is possible)
@miekg
Copy link
Owner

miekg commented Nov 28, 2018

this is a difficult thing to implement and out of scope for this project.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants