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

redis: enable DNS lookups on redirections by default #23868

Open
rgs1 opened this issue Nov 7, 2022 · 3 comments
Open

redis: enable DNS lookups on redirections by default #23868

rgs1 opened this issue Nov 7, 2022 · 3 comments

Comments

@rgs1
Copy link
Member

rgs1 commented Nov 7, 2022

Once #23764 has been fixed and DNS lookups have been used heavily in deployments, it might be a good idea to have it on by default.

@rgs1 rgs1 added enhancement Feature requests. Not bugs or questions. triage Issue requires triage labels Nov 7, 2022
@phlax
Copy link
Member

phlax commented Nov 8, 2022

cc @mattklein123

@phlax phlax added area/redis area/dns and removed triage Issue requires triage labels Nov 8, 2022
@rgs1
Copy link
Member Author

rgs1 commented Nov 8, 2022

Pretty sure we should do this, the tricky part might be figuring out the dns_lookup_family until we get happy eyeballs support:

https://github.com/envoyproxy/envoy/blob/main/api/envoy/extensions/common/dynamic_forward_proxy/v3/dns_cache.proto#L56

I guess one way would be to have two caches (IPv4 and IPv6), and just try one after the other and then remember what succeeded for that host.

@mattklein123 mattklein123 added help wanted Needs help! and removed enhancement Feature requests. Not bugs or questions. labels Nov 8, 2022
@rgs1
Copy link
Member Author

rgs1 commented Nov 14, 2022

If the default dns cache approach ends up being more complicated than necessary, we could fallback to making the dns_cache_config field mandatory (this might be API breakage, not sure what the policy for doing that is).

But this approach is certainly less error prone than trying to build a default DNS cache and guessing the IP settings, with the only downside of requiring more configuration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants