launchbadge / sqlx

🧰 The Rust SQL Toolkit. An async, pure Rust SQL crate featuring compile-time checked queries without a DSL. Supports PostgreSQL, MySQL, and SQLite.
Apache License 2.0
12.93k stars 1.23k forks source link

Owned transactions #2312

Open Ekleog opened 1 year ago

Ekleog commented 1 year ago

Hey! First things first, I really love sqlx. It just turns out I'm trying to do weird things with it and can't figure out how to get them done with the current sqlx, but it's already great!

Is your feature request related to a problem? Please describe. I'm trying to fuzz-test an axum http server that uses sqlx for its database.

In order to get the fuzzing to have a reasonable number of iterations per second, I can't just recreate the database and run the migrations on each fuzzer run.

So I need a way to make the changes one fuzz iteration made "vanish" at the end of it. I have implemented it with a manually-called "delete everything from all tables" script, but it is ugly, and actually running it probably makes things slower than needed, because most fuzz runs probably won't write data in all tables. (Though I don't have numbers about that, because I couldn't make the alternative work)

Diesel suggests using a diesel::connection::Connection::test_transaction in order to implement tests. It's not better than sqlx in the regard of the API exposed, but let's just think of the idea. Basically, instead of running the axum server with a regular PgPool that has multiple connections, we just run it with a pseudo-pool that has a single connection, that is actually inside a transaction that will never be committed.

This is all great and nice and little girls rolling around a flower field, but then the big bad wolf comes in and asks where the grandmother is because he's the uber eats the grandmother ordered and he's not sure where the house is, and he has about thirty other kilometers to do by bike in the next fifteen minutes (what? that's not how the story goes?).

It turns out sqlx's Transaction is actually borrowed. But a PoolConnection is owned. So it is not actually possible to make one pose for the other during tests only, while keeping code that seamlessly works with regular connections for the non-test case. Oh and self_cell can't make Transaction owned because begin() is async and self_cell's builder needs a sync callback and an async block would introduce a temporary variable. Oh and I actually need two layers of Transactions, because postgresql will explode the transaction should anything go wrong with one command in it. Oh and the above means I need to actually implement on error rollback-like functionality in the fuzzer runner.

I tried doing it with self_cell. I tried doing my own pseudo-self_cell that I would know to be unsound but should codegen fine-ish, to work around the async issue. I even tried giving up all pretense and just transmuting all lifetimes to 'static to see up to where I could go.

Turns out I could get none of these to work. I had hope for the last one, but it turned out to be deadlocking. I had a single mutex. And it was using lock guards. A single thread. How. Why. Anyway.

At some point I gave up and went back to the "manual script that drops everything" solution, but I figured out you might be interested to know what I think is missing to implement whole-system transaction-based testing, in a way similar to the one suggested by diesel.

Describe the solution you'd like In addition to the Transaction type, there is a OwnedTransaction type, that takes ownership of the parent connection (either a PoolConnection or another OwnedTransaction), and is able to spit it back after a commit or rollback.

Describe alternatives you've considered See above. Not doing anything would also make sense, given how niche this use case probably is.

Additional context I think implementing this would also solve https://github.com/launchbadge/sqlx/issues/1560

Ekleog commented 1 year ago

Seems like I can never stop thinking about this. Here I was, vacating to new goals, that is automatically spinning up a postgres instance that uses RAM only so that I could fuzz my server and not completely explode my nvme.

Turns out other people did benchmarking. Testing from a transaction is apparently 3-4x faster than using TRUNCATE to delete all the data from the tables.

Well… sounds like this might be worth it after all? Though potentially with #2313 only it'd already be enough to implement that feature (based on PgPool::begin, as we'd no longer need transaction nesting), I'm not sure.

Ekleog commented 1 year ago

Night always helps thinking, and… Upon some more thought, this doesn't actually need a new type. Instead, it would be enough to add functions to Transaction that would be like (pseudo-code):

enum Parent<'p> {
    PoolConnection(PoolConnection),
    Transaction(Transaction<'p>),
}

impl<'t> Transaction<'t> {
    pub fn wrap_begin(p: Parent<'t>) -> Transaction<'t>; // should be enough to create a Transaction<'static> from a PoolConnection or another Transaction<'static>
    pub fn commit_unwrap(self) -> Option<Parent<'t>>; // or maybe without the Option and with an unwrap inside?
    pub fn rollback_unwrap(self) -> Option<Parent<'t>>;
}

Basically sqlx's internal MaybePoolConnection (already used for Pool::begin) would be replaced by Option<Parent>, and I think the implementation should be reasonably simple. Does it make sense?

TmLev commented 1 year ago

@Ekleog you may also be interested in this talk https://www.youtube.com/watch?v=_VB1fLLtZfQ

Ekleog commented 1 year ago

Very interesting talk, thank you! I'm kind of sad I didn't know of it when I was first developing the tests and coming to the same conclusion.

And it's fun, the last problem mentioned (at 10') is exactly the one this and https://github.com/launchbadge/sqlx/issues/2313 are thinking about :sweat_smile: (I hope with #2313 I could make something library-only to handle the issue, but not sure yet as I gave up in my last attempts when hitting it)