diesel_async icon indicating copy to clipboard operation
diesel_async copied to clipboard

The conneciton pool returns corrupt connections

Open importcjj opened this issue 2 years ago • 3 comments

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

importcjj avatar Feb 27 '24 05:02 importcjj

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.

weiznich avatar Feb 27 '24 18:02 weiznich

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.

importcjj avatar Feb 28 '24 16:02 importcjj

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 avatar Mar 01 '24 12:03 weiznich