node-postgres icon indicating copy to clipboard operation
node-postgres copied to clipboard

Using hostaddr will cache the DNS lookup

Open mjgp2 opened this issue 3 years ago • 7 comments

This fixes occurrences where the IP changes for a native connection, e.g. with read replicas.

"If host is specified without hostaddr, a host name lookup occurs."

https://www.postgresql.org/docs/8.1/libpq.html#LIBPQ-CONNECT

You do not need to do the DNS lookup in javascript.

Fixes: https://github.com/brianc/node-postgres/issues/1748

mjgp2 avatar May 03 '22 17:05 mjgp2

You do not need to do the DNS lookup in javascript.

The reason it’s done in JavaScript is that it’s blocking in libpq, IIRC.

charmander avatar May 04 '22 07:05 charmander

It is blocking at within libpq (uses getaddrinfo), but it's seemingly done on a separate thread from the comment in the code, so should not block the node event loop?

https://github.com/brianc/node-libpq/blob/master/index.js#L34

FWIW we are experiencing a stale hostaddr in the connection string causing downtime on services.

mjgp2 avatar May 04 '22 08:05 mjgp2

Yah looking at this....I think the dns lookup in JS was done before I rewrote libpq bindings and made a proper background async connection worker thread. So this isn't needed now that we're running libpq's connect in node's threadpool.

Hey sorry for the headache here but CI was broken. I fixed it yesterday...would you be able to rebase onto master? Then the tests should pass and we can merge this up and get a release out! Thank you so much for contributing! ❤️

brianc avatar May 11 '22 18:05 brianc

Rebased!

Thanks so much for all your hard work, I totally <3 this library :)

mjgp2 avatar May 11 '22 18:05 mjgp2

@brianc - is the CI still broken, doesn't look like a test failure.

mjgp2 avatar May 11 '22 20:05 mjgp2

I'll try rerunning it - very weird error there I didn't on my branches or on master

brianc avatar May 12 '22 04:05 brianc

It looks like the errors in CI might be related to this PR...the other PR's I've rebased have all passed CI okay. 😢

brianc avatar May 12 '22 04:05 brianc

@brianc - I rebased, can you try rerunning?

mjgp2 avatar Oct 24 '22 09:10 mjgp2

Also, can you point me towards the settings for running unit tests against the local pg instance

mjgp2 avatar Oct 24 '22 09:10 mjgp2