Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Postgres: DISCARD ALL #73

Open
jakajancar opened this issue Jul 10, 2020 · 5 comments
Open

Postgres: DISCARD ALL #73

jakajancar opened this issue Jul 10, 2020 · 5 comments

Comments

@jakajancar
Copy link

Would be nice if bb8-postgres supported calling DISCARD ALL after the connection is returned to the pool, so it's guaranteed to be "pristine" on the next checkout.

Unless I'm missing something, there isn't even a "post check-in" hook in bb8 right now, only test_on_check_out. I suppose that could be used, but the best is to do it early, release the resources (e.g. UNLISTEN) and reduce the latency on checkout (assuming test_on_checkout is false).

@djc
Copy link
Owner

djc commented Jul 11, 2020

Might be nice to have a look at other pooling solutions (r2d2, deadpool, sqlx) and see if/how they support this.

(Note that previously we recommended that LISTEN queries should probably be issued on non-pooled connections, through Pool::dedicated_connection().)

@jakajancar
Copy link
Author

jakajancar commented Jul 11, 2020

PgBouncer has it configurable (server_reset_query) but it defaults to DISCARD ALL. PgBouncer is probably the gold standard when it comes to Postgres pooling.

It’s really not about the LISTEN, but all (currently 9) resources this releases. Another nice thing is that it fails if the connection is inside a transaction, so it helps automatically check that you’re closing them correctly.

I think it needs to be configurable, otherwise you loose the ability to share use prepared statements etc. Choose your own isolation level...

@djc
Copy link
Owner

djc commented Jul 14, 2020

I was referring mostly to Rust crates.

I'm unlikely to have time for this any time soon unless someone is willing to pay for it. Will review hooks for this in the connection manager trait, though.

@jakajancar
Copy link
Author

Yes, I think hooks are key rather than supporting every single possible reset query. Then one can subclass the postgres manager and do what they want.

@djc
Copy link
Owner

djc commented Dec 14, 2020

#89 added a way to customize connections at the start of the lifetime rather than the end, which should help with this.

film42 added a commit to film42/bb8 that referenced this issue Mar 14, 2021
Issue djc#73 was looking to make a "DISCARD ALL" query possible when a
connection is returned to the pool. Because async drop isn't a thing,
this isn't really possible at the moment. However, we can use the
`is_valid` method on the `ManagedConnection` trait to discard all
session state before yielding the connection.

This PR makes it possible to change the query used within the `is_valid`
method call in `bb8_postgres`.

To make configuring the validation query a little easier, I added a new
postgres connection manager builder helper type. Let me know if you're
:+1: or :-1: on that.

This also includes an example called buidler.rs that shows how someone
could use `DISCARD ALL` as a validation query. It then prints a few
things to show that session state was indeed cleared between checkouts.

```
$ cargo run --example builder
...
The current connection PID: 86445
BB8 says, "beep boop"
The current connection PID: 86445
After DISCARD ALL on checkout, BB8 says, ""
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants