djc / bb8

Full-featured async (tokio-based) postgres connection pool (like r2d2)
MIT License
753 stars 110 forks source link

dedicated_connection() does not expose underlying connection for polling message from postgres #60

Open hengchu opened 4 years ago

hengchu commented 4 years ago

The dedicated_connection() function returns M::Connection. For bb8-postgres, this type maps to the tokio_postgres::Client type. However, for receiving LISTEN notifications from postgres, we need the tokio_postgres::Connection value. Is there some way to achieve this with bb8?

Thanks!

khuey commented 4 years ago

Ugh, that's kind of nasty. Maybe we should add a method onto PostgresConnectionManager and give you a way to get that from the Pool.

djc commented 4 years ago

It seems like a weird API from tokio-postgres. If Connection is supposed to "generally be spawned off onto an executor", you have no way of getting notifications? @sfackler, is that true? Wouldn't it make more sense to expose some kind of query_raw() like API from the Client?

hengchu commented 4 years ago

Ugh, that's kind of nasty. Maybe we should add a method onto PostgresConnectionManager and give you a way to get that from the Pool.

That could work. Although this is not a high-priority issue (at least not for me...). Since we are using a dedicated connection already, right now I just use tokio_postgres::connect directly to get the underlying connection object, instead of through bb8.

It seems like a weird API from tokio-postgres. If Connection is supposed to "generally be spawned off onto an executor", you have no way of getting notifications? @sfackler, is that true? Wouldn't it make more sense to expose some kind of query_raw() like API from the Client?

There does seem to be some friction between bb8's API and tokio_postgres's notification API. Currently, you need to call poll_message on the tokio_postgres::Connection object to receive notifications. You can make this more ergonomic with futures::stream::poll_fn which just turns the connection into a notification Stream value.

khuey commented 4 years ago

It appears you have to drive the connection yourself if you want to get LISTEN notifications. Ideally tokio-postgres would split those off into a separate Stream for you ...

sfackler commented 4 years ago

The general expectation is that if you want to forward notifications to a stream, you'd write that code and then spawn it off: https://github.com/sfackler/rust-postgres/blob/master/tokio-postgres/tests/test/main.rs#L616-L619

What would a query_raw function do?

djc commented 4 years ago

Ah, that general expectation is actually reasonable. But it's different from what I read into the documentation for poll_message(): "Applications that wish to examine those messages should use this method to drive the connection rather than its Future implementation.". I guess there's no simple way to get this for a Client because there's potentially many Clients.

Maybe link that example code from the poll_message() docs?