Async DNS resolution

[DzigaV]

According to the docs:

AsyncConnection.connect() may still block on DNS name resolution. To avoid that you should set the hostaddr connection parameter.

The resolve_hostaddr_async() is an experimental solution to help to do that. Feedback about the feature is welcome!

Is there a reason why you're not using asyncio.loop.getaddrinfo?

[dvarrazzo]

Uhm... none that I remember. Maybe I just failed to google for it. However, that function is just the blocking function called in an executor: I tried the same approach in the async-dns branch a while ago. By the look of that branch, I got stuck investigating some resource leak. See also #69.

Note that the algorithm to replace hostaddr in a connection string is not trivial; it is already implemented and can be ported to asyncio.loop.getaddrinfo though.

The current implementation in psycopg._dns.resolve_hostaddr_async() is not used by default because it uses the dnspython external package, which I didn't want to add as a hard dependency yet, and because that implementation doesn't use /etc/hosts for resolution, so it cannot be considered a full or transparent replacement.

So, all in all, as things were when I stopped with the async-dns branch, were:

• -1: reliance on an external library
• -1: non transparent sync/async behaviour w.r.t. /etc/hosts (dnspython shortcoming)
• -1: resource leaks
• -0.5: complex algorithm, feeling more a policy than a mechanism, so added as utility function

I guess, if asyncio.loop.getaddrinfo proves reliable, we only remain with a -0.5 here and we could be brave and release the algorithm to the wild. Users are made for testing, aren't they?

[DzigaV]

Note that the algorithm to replace hostaddr in a connection string is not trivial;

Yes, I wouldn't want to touch this with a 10-foot pole.

I agree that this is tricky from a library perspective. I like the way Tornado handles this, by providing a pluggable interface where one can provide your own Resolver class. Although to be honest, just switching out dnspython for loop.getaddrinfo would work for me.

I suppose I can import resolve_hostaddr_async from _dns, provide my own async_resolver instance, use it in a subclass of AsyncConnection and provide that to AsyncConnectionPool, but it seems a little bit... involved 🙂

[dvarrazzo]

it seems a little bit... involved

Yes, it is tricky to inject a policy into a library. The _get_connection_params subclass hook is there to gather use cases, such as yours, and figure out if there is a better design to solve real use cases. So, feedback and suggestions are welcome.

[DzigaV]

it seems a little bit... involved

Yes, it is tricky to inject a policy into a library. The _get_connection_params subclass hook is there to gather use cases, such as yours, and figure out if there is a better design to solve real use cases. So, feedback and suggestions are welcome.

Easy for me to say, but my feedback is philosophical rather than technical: psycopg should be async without caveats.

From a practical standpoint, I don't want the pool blocking my application for any reason, as it goes about its business. I wouldn't want to work around this with caching either, since DNS-based failover would be hindered.

Therefore the non-trivial monster method is a necessary evil, provided it can work without an external library, hence my suggestion today. I don't think the dnspython solution works for all the reasons you state.

[dvarrazzo]

Therefore the non-trivial monster method is a necessary evil, provided it can work without an external library, hence my suggestion today.

I appreciate it and agree with you: _dns tools and related hooks are a work bench where to explore with a territory that is not totally familiar to me, but the final aim is to provide async resolution out of the box.