Skip to content
This repository has been archived by the owner on Sep 12, 2024. It is now read-only.

helios-solo: skydns workaround to allow DNS responses up to 32768 bytes #1081

Merged
merged 2 commits into from
Jan 25, 2017

Conversation

gimaker
Copy link
Contributor

@gimaker gimaker commented Jan 25, 2017

TL;DR; skydns does not handle TCP well. We already worked around this
in #900. See thar PR for more context.
However, that change only fixed the problem to an extent as we still have
the same issue once the responses are >4096 bytes.

This change extends that workaround to allow us to survive responses up to
32768 bytes in size. This change does not fix the issue, but should make it
more rare.

TL;DR; skydns does not handle TCP well. We already worked around this
in #900. See thar PR for more context.
However, that change only fixed the problem to an extent as we still have
the same issue once the responses are >4096 bytes.

This change extends that workaround to allow us to survive responses up to
32768 bytes in size. This change does not fix the issue, but should make it
more rare.
@gimaker
Copy link
Contributor Author

gimaker commented Jan 25, 2017

Copy link
Contributor

@davidxia davidxia left a comment

Choose a reason for hiding this comment

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

🥇

@mattnworb
Copy link
Member

Nice work. Could you also document somewhere (for instance docs/helios_solo.md) what error messages you can look for from helios-solo or skydns when this problem occurs? For that one user in the future who encounters a giant 33kb response.

@codecov-io
Copy link

codecov-io commented Jan 25, 2017

Current coverage is 51.65% (diff: 100%)

Merging #1081 into master will increase coverage by 0.06%

@@             master      #1081   diff @@
==========================================
  Files           270        270          
  Lines         12934      12934          
  Methods           0          0          
  Messages          0          0          
  Branches       1678       1678          
==========================================
+ Hits           6672       6681     +9   
+ Misses         5759       5751     -8   
+ Partials        503        502     -1   

Powered by Codecov. Last update b74b938...e96a17c

@gimaker
Copy link
Contributor Author

gimaker commented Jan 25, 2017

@mattnworb Note added to docs.

Copy link
Member

@mattnworb mattnworb left a comment

Choose a reason for hiding this comment

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

thanks!

@davidxia
Copy link
Contributor

tests pass

@gimaker gimaker merged commit 6cb55ec into master Jan 25, 2017
@gimaker gimaker deleted the staffan/skydns-udp branch January 25, 2017 16:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants