clj-commons / aleph

Asynchronous streaming communication for Clojure - web server, web client, and raw TCP/UDP
http://aleph.io
MIT License
2.54k stars 241 forks source link

[dns] Extract dns-resolver-group builder code into a separate function #564

Closed alexander-yakushev closed 3 years ago

alexander-yakushev commented 3 years ago

Motivation: in certain cases, I want to use RoundRobinDnsAddressResolverGroup for DNS resolution. Some services provide a way of load balancing in the form of multiple DNS entries per URL, e.g. that's how AWS Load Balancer "balances" its own instances. The default DnsAddressResolverGroup caches just the first entry of the DNS server response and uses it throughout the cache invalidation period, leaving other returned entries unused. RR resolver caches the entire set of returned entries and goes over them on each request to the resolver.

In the first cut, I implemented the customizability via reflection which may seem clunky. The only other option I have is to pass a "constructor function" that accepts the Builder object and returns the resolver group. Please, tell me if that method would be preferred.

CC @kachayev.

kachayev commented 3 years ago

@alexander-yakushev Hey, thanks a lot for the PR! As of now, a custom dns-resolver-group could be provided directly as a configuration parameter into connection-options (as a :name-resolver). So, technically it's possible to create any resolver that suits best. I assume, that the use case here is more of "how to reuse fancy logic from the helper but only change one specific parameter?", right? Let me think a little bit about this.

alexander-yakushev commented 3 years ago

I wasn't aware that the resolver object can be directly provided! Should have checked that before :). But yes, the point still stands whether it is worth trying to extend the Clojury helper further.

kachayev commented 3 years ago

I wasn't aware that the resolver object can be directly provided

Let's be honest, the documentation for custom DNS is barely existing :)

Here's what we can do: split dns-resolver-group into 2 functions by separating dns-resolver-group-builder. So, it would be something like

(def dns-resolver-group [dns-options]
  (DnsAddressResolverGroup. (dns-resolver-group-buider b))

In you case you would just call dns-resolver-group-buider, re-attach whatever property you need utilizing Java object setters, and than pass :name-resolver built from it.

What do you think?

alexander-yakushev commented 3 years ago

That is a reasonable solution that allows reusing the builder code without going crazy on customizing the group class. Thanks for the suggestion! I've updated the PR accordingly.

kachayev commented 3 years ago

@alexander-yakushev Cool, thanks!

alexander-yakushev commented 3 years ago

Thanks for handling this!