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: make refresh frequency adjustable #3083

Merged
merged 1 commit into from Aug 4, 2019
Merged

plugin/route53: make refresh frequency adjustable #3083

merged 1 commit into from Aug 4, 2019

Conversation

mattlqx
Copy link
Contributor

@mattlqx mattlqx commented Aug 1, 2019

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

the current update frequency for the refresh loop in the route 53 plugin is hard-coded
to 1 minute. aws rate-limits the number of api requests so less frequent record refreshes
can help when reaching those limits depending upon your individual scenarios. this pull
adds a configuration option to the route53 plugin to adjust the refresh frequency.

thanks for getting my last pull released so quickly. this is the last local change that
i have been running and would love to get it contributed back to the project.

2. Which issues (if any) are related?

Another step to solving #2353

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

Included in pull.

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

No, maintains current behavior by default.

@corbot corbot bot requested a review from yongtang August 1, 2019 16:53
@corbot
Copy link

corbot bot commented Aug 1, 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 yongtang (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.

@miekg
Copy link
Member

miekg commented Aug 1, 2019

there seems to be many issue to the frequency that allows you to fetch stuff from this API. A more user friendly approach is to figure out programmatically what the best freq. is and use that.

@mattlqx
Copy link
Contributor Author

mattlqx commented Aug 1, 2019

The Terraform approach is to make as many requests as you can and then use an exponential back-off until you've made all the requests you want. The problem there is that you don't know if you're going to be limited until you are and once you are, you can only wait and keep trying. That works fine from the standpoint of a single client, but the rate-limits are global to your whole AWS account. So too many clients without knowledge of each other trying to pull as many records as fast as possible can quickly degrade your AWS API service quality.

As an operator, I have the view of how many services I have running that need to interface with it and I need to be able to control them to prevent rate-limits from being reached in the first place to the best of my ability. This isn't a huge deal when I'm running one CoreDNS daemon configured to pull from Route 53, but it increasingly becomes an issue for every additional replica added that is configured this way.

@codecov-io
Copy link

codecov-io commented Aug 2, 2019

Codecov Report

Merging #3083 into master will increase coverage by 0.06%.
The diff coverage is 94.73%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3083      +/-   ##
==========================================
+ Coverage   55.85%   55.92%   +0.06%     
==========================================
  Files         205      205              
  Lines       10108    10123      +15     
==========================================
+ Hits         5646     5661      +15     
  Misses       4051     4051              
  Partials      411      411
Impacted Files Coverage Δ
plugin/route53/setup.go 80.95% <100%> (+3.8%) ⬆️
plugin/route53/route53.go 86.61% <75%> (+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 fc1e313...f25bed4. Read the comment docs.

plugin/route53/README.md Outdated Show resolved Hide resolved
plugin/route53/setup.go Outdated Show resolved Hide resolved
the current update frequency for the refresh loop in the route 53 plugin is hard-coded
to 1 minute. aws rate-limits the number of api requests so less frequent record refreshes
can help when reaching those limits depending upon your individual scenarios. this pull
adds a configuration option to the route53 plugin to adjust the refresh frequency.

thanks for getting my last pull released so quickly. this is the last local change that
i have been running and would love to get it contributed back to the project.

Signed-off-by: Matt Kulka <mkulka@parchment.com>
Copy link
Collaborator

@dilyevsky dilyevsky left a comment

Choose a reason for hiding this comment

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

Nice!

@dilyevsky dilyevsky merged commit 94468c4 into coredns:master Aug 4, 2019
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