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

[plugin/route53]: Increase ListResourceRecordSets paging size. #3073

Merged
merged 1 commit into from Jul 31, 2019

Conversation

@mattlqx
Copy link
Contributor

commented Jul 31, 2019

1. Why is this pull request needed and what does it do?

without the paging parameter set, it will default to 100 records per request. with large enough zones and potentially multiple coredns daemons configured to pull from route 53, this can quickly add up and reach aws global api rate limits (5/sec per root account). increasing paging to max can help reduce the number of requests needed to pull records for a zone without no down side that i am aware of.

2. Which issues (if any) are related?

this helps issue #2353, but probably is not a complete fix.

3. Which documentation changes (if any) need to be made?

None.

4. Does this introduce a backward incompatible change or deprecation?

No.

@corbot corbot bot requested a review from dilyevsky Jul 31, 2019

@corbot

This comment has been minimized.

Copy link

commented Jul 31, 2019

Thank you for your contribution. I've just checked the OWNERS files to find a suitable reviewer. This search was successful and I've asked dilyevsky (via plugin/route53/OWNERS) for a review.

If you have questions or suggestions for this bot, please file an issue against the miekg/dreck repository.

The bot understands the commands that are listed here.

@dilyevsky dilyevsky changed the title add paging to route 53 records api request [plugin/route53]: Increase ListResourceRecordSets paging size. Jul 31, 2019

@dilyevsky
Copy link
Collaborator

left a comment

Thanks! Not sure what's up with those presubmits...

@dilyevsky dilyevsky closed this Jul 31, 2019

@dilyevsky dilyevsky reopened this Jul 31, 2019

@dilyevsky
Copy link
Collaborator

left a comment

Oh you need to rebase to pick up circle config.

add paging to route 53 records api request
without the paging parameter set, it will default to 100 records per request. with large enough zones and
potentially multiple coredns daemons configured to pull from route 53, this can quickly add up and reach
aws global api rate limits (5/sec per root account). increasing paging to max can help reduce the number
of requests needed to pull records for a zone without no down side that i am aware of.

this helps issue #2353, but probably is not a complete fix.

Signed-off-by: Matt Kulka <mkulka@parchment.com>
@codecov-io

This comment has been minimized.

Copy link

commented Jul 31, 2019

Codecov Report

Merging #3073 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3073      +/-   ##
==========================================
+ Coverage   55.88%   55.88%   +<.01%     
==========================================
  Files         205      205              
  Lines       10118    10119       +1     
==========================================
+ Hits         5654     5655       +1     
  Misses       4053     4053              
  Partials      411      411
Impacted Files Coverage Δ
plugin/route53/route53.go 86.52% <100%> (+0.09%) ⬆️

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 cd5dceb...581d847. Read the comment docs.

@dilyevsky dilyevsky merged commit 45e17c3 into coredns:master Jul 31, 2019

4 checks passed

ci/circleci: kubernetes-tests Your tests passed on CircleCI!
Details
codecov/project 55.88% (target 50%)
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
stickler-ci No lint errors found.

@mattlqx mattlqx deleted the mattlqx:route53-paging branch Jul 31, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.