begin_test_transaction flaky when ran in async context

[darwin67]

Setup

Versions

• Rust: 1.51
• Diesel: 1.4.6
• Database: PostgreSQL
• Operating System Arch Linux (Linux XPS-15-7590 5.10.27-1-lts TODO #1 SMP Tue, 30 Mar 2021 13:22:29 +0000 x86_64 GNU/Linux)

Feature Flags

• diesel: postgres, r2d2, serde_json

Problem Description

begin_test_transaction gets flaky when trying to start a transaction in async context (they are not nested).
The issue is consistently showing up after around 10 tests which is same as the default pool size for r2d2.

There are no nested transactions as you will be able to see in the code example I shared,
but somehow the transaction_manager.get_transaction_depth() is returning non zero, and causing the code to panic.

I'm not sure if this is related to Diesel not supporting async or is something else so wanted to share it here and see if there are ways to resolve it.
This doesn't happen when running with #[test] and normal synced manner. e.g. see models tests in the repo below

What are you trying to accomplish?

Having tests run without flaking randomly

What is the expected output?

Tests shouldn't error out when starting a transaction in async context.

What is the actual output?

Tests failing randomly
https://gist.github.com/darwin67/335def05fe346f637664cb614f42e088

Are you seeing any additional errors?

No

Steps to reproduce

https://github.com/darwin67/async-test-issue

1. Download the repo
2. Follow README to setup the databases (async_dev, async_test), and modify the DATABASE_URL in .env to fit your local db's credentials fomat
3. Run make test

Checklist

• I have already looked over the issue tracker and the disussion forum for similar possible closed issues.

• This issue can be reproduced on Rust's stable channel. (Your issue will be
  closed if this is not the case)
• This issue can be reproduced without requiring a third party crate

[weiznich]

I would call this expected behaviour.
First of all this is not related to async code or something like that at all. It's your usage of a connection pool that causes this issue.
A connection pool is designed to keep connections as long as the transaction pool exists. If you now call begin_test_transaction() on each connection you got from a pool that can mean that you call begin_test_transaction() on a connection previously used in a other test case, where you already called begin_test_transaction() on it. To prevent getting nested transactions that way diesel checks that you called begin_test_transaction() exactly once.
So now for the how to fix your tests: Either don't use a connection pool, by creating a new connection per test or move the call to begin_test_transaction() call into the pool itself. r2d2 provides extensions points to customize the connection after creation, before the connection is available as part of the pool.
Closed as this is not an issue in diesel. Please use the forum or the gitter channel to ask follow on questions.

[darwin67]

@weiznich thanks for clarifying and sharing the customization trait for r2d2.
I added a connection customizer for test and got it to work as I expected it to.

For anyone whom encountered a similar issue as I did, here's the customizer for test connections to be used with r2d2

lazy_static! {
    static ref POOL: Pool = {
        let db_url = env::var("DATABASE_URL").expect("DATABASE_URL must be set");
        let manager = ConnectionManager::<PgConnection>::new(db_url);
        let app_env = env::var("APP_ENV").unwrap_or_else(|_| "development".to_string());
        let mut builder = r2d2::Pool::builder();

        if &app_env == "test" {
            builder = builder.connection_customizer(Box::new(TestConnectionCustomizer));
        }

        builder
            .build(manager)
            .expect("Failed to create database connection pool")
    };
}

...

#[derive(Debug, Clone, Copy)]
pub struct TestConnectionCustomizer;

impl<C, E> CustomizeConnection<C, E> for TestConnectionCustomizer
where
    C: diesel::Connection,
{
    fn on_acquire(&self, conn: &mut C) -> Result<(), E> {
        conn.begin_test_transaction()
            .expect("Failed to start test transaction");

        Ok(())
    }
}

[ralpha]

Thanks! @darwin67 I used this in combination with a manual implementation of #[rocket_contrib::database("db_name")] in Rocket.
Now I can test my Rocket endpoints very easily.
Thanks!

For people that find this in the future:
Change this: https://github.com/SergioBenitez/Rocket/blob/v0.4/contrib/codegen/src/database.rs#L96:L97
To this:

let pool = ::rocket_contrib::databases::database_config("db_name", rocket.config())
    .map(|config| {
        // More info: https://github.com/diesel-rs/diesel/issues/2733
        let manager = diesel::r2d2::ConnectionManager::new(config.url);
        r2d2::Pool::builder()
            .connection_customizer(Box::new(TestConnectionCustomizer))
            .max_size(config.pool_size)
            .build(manager)
    });