sfackler / rust-postgres

Native PostgreSQL driver for the Rust programming language
Apache License 2.0
3.49k stars 443 forks source link

Possibility to avoid an extra DNS lookup? #943

Closed trungda closed 1 year ago

trungda commented 2 years ago

Hi, thanks for the crate! We are using this crate heavily in prod. Nice work! I have a question regarding establishing TLS connection to a Postgres database. We are using the following code snippet to create a new connection to the db server.

let builder = ...;
let tls = postgres_openssl::MakeTlsConnector::new(builder.build());

let (client, connection) = config.connect(tls).await?;

We provide the host and port parameters in the config object. The host here could be either a raw ipaddress or the hostname of the db server. In order to use TLS, we have to provide the hostname (since it's what the server cert includes). However, our use case is a bit special, we have the hostname but we also have the ipaddress of the db server upfront.

I am wondering if we could use the ipaddress + hostname to create a TLS connection somehow without passing the hostname to config.connect() since I believe under the hood, tls.make_tls_connect` would make a DNS request to resolve the hostname but we already have the hostname resolved before trying to create the connection.

One possibility I can think of is to extend the Config struct to have an optional hostname field just so we don't have to build hostname from host(if provided). If we can do that, we can still pass the "already known" ipaddress to host and pass the hostname to the new hostname field. What do you think?

sfackler commented 2 years ago

It seems reasonable to follow psql and add the hostaddr field for the IP: https://www.postgresql.org/docs/14/libpq-connect.html#LIBPQ-PARAMKEYWORDS

trungda commented 2 years ago

Thanks. I will open a PR. How often do you release to the crate? I am asking to see if we need to pull master in our code or we can wait for the PR to be merged and released.

sfackler commented 2 years ago

I generally cut a release whenever people ask about wanting one to pull in a new feature :)

trungda commented 2 years ago

Sweet. Let me try to open a today. Thanks @sfackler

trungda commented 2 years ago

@sfackler could you help take a look at https://github.com/sfackler/rust-postgres/pull/945? Thank you.

trungda commented 1 year ago

945 is merged. We can close this now.