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

Allow configuration of persistent TCP connections #470

Merged
merged 1 commit into from
Apr 26, 2021

Conversation

edevil
Copy link
Contributor

@edevil edevil commented Apr 21, 2021

Added 2 new options to configure previously hardcoded
values: max-reuse-tcp-queries and tcp-reuse-timeout. These
allow fine-grained control over how unbound uses persistent
TCP connections to authority servers.

Some authority servers don't deal well with sending multiple
queries over the same TCP connection so configuring these
values is necessary.

Added 2 new options to configure previously hardcoded
values: max-reuse-tcp-queries and tcp-reuse-timeout. These
allow fine-grained control over how unbound uses persistent
TCP connections to authority servers.
Copy link
Member

@wcawijngaards wcawijngaards left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These options look good and can be merged.

@wcawijngaards wcawijngaards merged commit 646d6b9 into NLnetLabs:master Apr 26, 2021
wcawijngaards added a commit that referenced this pull request Apr 26, 2021
- Merge #470 from edevil: Allow configuration of persistent TCP
  connections.
@wcawijngaards
Copy link
Member

The code looks good so I merged it. From our discussion, an option for the auth tcp timeout would perhaps also be useful in this problem space and could be helpful for solutions.

@iz8mbw
Copy link

iz8mbw commented Apr 27, 2021

@edevil @wcawijngaards thanks for that!

jedisct1 added a commit to jedisct1/unbound that referenced this pull request Apr 27, 2021
* nlnet/master:
  Changelog note for PR NLnetLabs#470 - Merge NLnetLabs#470 from edevil: Allow configuration of persistent TCP   connections.
  Allow configuration of persistent TCP connections
@oerdnj
Copy link

oerdnj commented Feb 11, 2022

Hi @edevil, could you please expand on:

Some authority servers don't deal well with sending multiple queries over the same TCP connection so configuring these values is necessary.

Do you have any datapoints and can you describe the behaviour by those authoritative servers? Either here or dns-operations would be fine, I guess.

@edevil
Copy link
Contributor Author

edevil commented Feb 15, 2022

Hello @oerdnj

It has been a while since I looked at this, but what I was seeing at the time was that some servers did not respond to subsequent queries on the same TCP socket, but also did not close it. So this caused high latency because Unbound would first need to give up on the tcp socket before a successful query was sent.

@oerdnj
Copy link

oerdnj commented Feb 15, 2022

Oh, so it was a client issue and not a server issue. That's good to know, thank you, @edevil!

@edevil
Copy link
Contributor Author

edevil commented Feb 15, 2022

Well, I wouldn't consider keeping the TCP connection open and ignoring new queries on that connection a good behavior... :)

These options mitigate the issue on the client side but it seems more like a server issue.

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 this pull request may close these issues.

None yet

4 participants