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

Add config for maxUdpPacketSize #448

Closed
wants to merge 2 commits into from

Conversation

singron
Copy link

@singron singron commented Nov 6, 2020

Which problem is this PR solving?

  • The max udp packet size can differ between platforms. See Frequent EMSGSIZE with UDPSender #124 (comment)
  • In that thread, they increased the maximum packet size on the system, but sometimes it's easier to just have clients produce smaller packets if they can.

Short description of the changes

  • Make the maximum udp packet size configurable.

The tests were also failing, so there is also a small commit to fix some invalid hostnames to be invalid.

The previous hostname foo.bar.xyz successfully resolves now, which
causes these tests to fail.

RFC 2606 reserves the .invalid TLD for hostnames that are invalid, so we
should use that instead.

Signed-off-by: Eric Culp <eric@getclockwise.com>
The maximum UDP packet size isn't necessarily 65k, and the code as
written can drop lots of spans if it's any smaller. E.g. darwin has a 9k
max size by default.

Signed-off-by: Eric Culp <eric@getclockwise.com>
@codecov
Copy link

codecov bot commented Nov 6, 2020

Codecov Report

Merging #448 (72ca4be) into master (716c4c7) will decrease coverage by 0.04%.
The diff coverage is 50.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #448      +/-   ##
==========================================
- Coverage   98.72%   98.67%   -0.05%     
==========================================
  Files          50       50              
  Lines        2035     2037       +2     
  Branches      383      384       +1     
==========================================
+ Hits         2009     2010       +1     
- Misses         26       27       +1     
Impacted Files Coverage Δ
src/configuration.js 98.00% <50.00%> (-0.98%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 716c4c7...72ca4be. Read the comment docs.

@yurishkuro
Copy link
Member

Any tests that can be added? Any changes to documentation required? IIRC there's a section in the README that specifically talks about packet sizes, may be worth mentioning this new config option there.

@yurishkuro
Copy link
Member

No response from the author, closing.

@yurishkuro yurishkuro closed this Apr 28, 2021
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.

2 participants