-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Implements prefix lookups for DNS TTL #4605
Implements prefix lookups for DNS TTL #4605
Conversation
This will fix hashicorp#4509 and allow forinstance lb-* to match services lb-001 or lb-service-007
@mkeeler Hello Matt, Do you think you might have a look ? Best Regards |
@pearkes Do you think it could be included? (it a quite simple patch, safe in terms of ascending compatibility and will really help having good performance for DNS usages with many many services) |
@mkeeler do you think you can review? This change is very backwards compatible and it would really help us to merge this in order to reduce significantly the load generated by DNS requests on Consul server. Kind regards |
@pearkes Do you think you might have a look? The change is pretty simple but would really help for people having large number of services. It is also very good at ascending compatibility since it simply allow the use of * not only for '*' but with any prefix. Kind regards |
This will fix hashicorp#4509 and allow forinstance lb-* to match services lb-001 or lb-service-007 Added unit tests with new TTLs prefix Fixed unstable unit tests Use Radix lookup for default key '*' on TTL lookups Added documentation for DNS wildcard matches with prefix Better documentation about DNS TTL cache mechanism
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Pierre sorry this one fell through the cracks. It seems like a solid and reasonable simple enhancement.
I don't think it will make it into today's release so no rush, but I think it's pretty much OK - I've made some minor comment and doc suggestions.
@banks Thank you for the review, all your comments resolved in next patch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great LGTM. We'll try to get this into 1.4.0!
This will fix #4509 and allow forinstance lb-* to match services lb-001 or lb-service-007.
This will fix #4509 and allow for instance
lb-*
to match serviceslb-001
orlb-service-007
When not adding
*
at the end, the existing behavior is kept, meaning strict check.