The conneciton pool returns corrupt connections

importcjj commented 9 months ago

Setup

Versions

Rust: rustc 1.75.0 (82e1608df 2023-12-21)
Diesel: 2.1.0
Diesel_async: 0.4.1
Database: mysql 5.7.26-log
Operating System： linux

Feature Flags

diesel: ["mysql_backend", "time"]
diesel_async: ["mysql", "deadpool"]

Problem Description

After running for a few days, some connections in the connection pool will return an error

What are you trying to accomplish?

Query records in the database

What is the expected output?

No error

What is the actual output?

Got an error: "Unexpected end of row"

Are you seeing any additional errors?

Input/output error: can't parse: buf doesn't have enough data

Steps to reproduce

It's hard to reproduce the problem, after running for a few days, some connections in the pool will return errors. I think those connections are already corrupt but still can pass the health check of the connection pool. So I did a contrast experiment, when I found the connection got corrupt, the ping2 responded with ok, but the ping responded with an error "buf doesn't have enough data"


use diesel_async::{pooled_connection::deadpool::Pool, AsyncMysqlConnection};

type MysqlPool = Pool<AsyncMysqlConnection>;

pub struct AppState {
    pub mysql_pool: MysqlPool,
}

async fn ping(State(state): State<Arc<AppState>>) -> Result<impl IntoResponse> {
    let mut conn = state.mysql_pool.get().await?;
    let x: i32 = diesel::select(1_i32.into_sql::<diesel::sql_types::Integer>())
        .first(&mut conn)
        .await?;

    Ok(Json(x))
}

async fn ping2(State(state): State<Arc<AppState>>) -> Result<impl IntoResponse> {
    let mut conn = state.mysql_pool.get().await?;
    let _ = diesel::select(1_i32.into_sql::<diesel::sql_types::Integer>())
        .execute(&mut conn)
        .await?;

    Ok(Json(())
}

Checklist

[x] I have already looked over the issue tracker for similar possible closed issues.
[x] This issue can be reproduced on Rust's stable channel. (Your issue will be closed if this is not the case)
[x] This issue can be reproduced without requiring a third party crate

weiznich commented 9 months ago

Thanks for opening this bug report. I think I would accept a PR that just changes the built-in ping function to your first variant. That should hopefully workaround that issue. Otherwise I would be interested in having a minimal reproducible example for this, as otherwise this is impossible to debug for anyone.

https://github.com/weiznich/diesel_async/blob/1ef43d2a8a4080c1e9e5c34056f4d26cbb2a85cc/src/pooled_connection/mod.rs#L330

For now you can also workaround that issue by providing your custom connection check function via this config.

importcjj commented 9 months ago

Thanks for opening this bug report. I think I would accept a PR that just changes the built-in ping function to your first variant. That should hopefully workaround that issue. Otherwise I would be interested in having a minimal reproducible example for this, as otherwise this is impossible to debug for anyone.

https://github.com/weiznich/diesel_async/blob/1ef43d2a8a4080c1e9e5c34056f4d26cbb2a85cc/src/pooled_connection/mod.rs#L330

For now you can also workaround that issue by providing your custom connection check function via this config.

Changing the built-in ping function to my first variant is not easy, since it requires the backend to support the integer type i32. The workaround way is to change the ping implementation in every certain backend.

weiznich commented 9 months ago

We can assume support for i32 for all database systems, as it is a really fundamental type. It just need to be encoded in the trait bounds for this impl.

weiznich / diesel_async