heroku / heroku-buildpack-pgbouncer

Run pgbouncer in a dyno along with your application
MIT License
338 stars 136 forks source link

Use the recommended `c-ares` DNS backend instead of `evdns`? #188

Open edmorley opened 4 months ago

edmorley commented 4 months ago

The upstream PgBouncer project supports multiple DNS backends, described in the table here: https://www.pgbouncer.org/install.html

That page then goes on to say:

c-ares is the most fully-featured implementation and is recommended for most uses and binary packaging (if a sufficiently new version is available). Libevent’s built-in evdns is also suitable for many uses, with the listed restrictions. The other backends are mostly legacy options at this point and don’t receive much testing anymore.

By default, c-ares is used if it can be found. ...

However, the pgbouncer binaries used by this buildpack currently use evdns instead of c-ares since the library for the latter isn't in the Heroku base image.

The library is small, so we (the languages team) would be open to adding it to the base image to allow this buildpack to switch to the c-ares backend - if that would be of interest to the Data team?

https://packages.ubuntu.com/noble/libc-ares-dev https://packages.ubuntu.com/noble/libcares2

edmorley commented 4 months ago

@mble Any thoughts on this? 🙂

edmorley commented 4 months ago

@mble-sfdc Oh sorry realised I CCed your personal account rather than your work one.

I forgot to mention, that if this library was something the Data team did want, there would be a lag between when we add it to the base image and when it's available in Private Spaces at run time (due to the way image rollouts work there), so to be safe we'd need to wait several weeks after adding before recompiling pgbouncer in this buildpack. (So the sooner we'd add the package the better for avoiding blocking you.)

mble-sfdc commented 4 months ago

@edmorley It's probably something we could consider. If we can get it into the stack images, I think we can look at building it with c-ares support.

We've got a whole thing lined up around this buildpack to do some renovation so it would align well with it, I think.

edmorley commented 4 months ago

I've added c-ares to the run images in https://github.com/heroku/base-images/pull/307 however note that:

  1. It won't be on common runtime until the next base image update (probably next week)
  2. Even after that base image release (incl GitHub release and https://devcenter.heroku.com/changelog entry) it won't yet be on private spaces until the next AMI is generated and rolled out across all channels (up to a couple of weeks after; status can be seen via runtime-ops)

Also, I chose not to add the headers to the build image (since there don't seem to be much need for them for eg c-ares bindings used by apps themselves), so the libc-ares-dev package will need to be installed via apt-get in the Dockerfile in this repo used for the PgBouncer build itself.

edmorley commented 3 months ago

The addition of the c-ares package to the base image has propagated to all environments.

As such, it's now safe for this buildpack to start compiling builds that have that backend enabled.

Note: The build Dockerfile/scripts will need to install the headers (libc-ares-dev) for the backend to be enabled during compilation.

mble-sfdc commented 3 months ago

Fantastic, thanks @edmorley