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

The conneciton pool returns corrupt connections #139

Open
3 tasks done
importcjj opened this issue Feb 27, 2024 · 3 comments
Open
3 tasks done

The conneciton pool returns corrupt connections #139

importcjj opened this issue Feb 27, 2024 · 3 comments
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@importcjj
Copy link

importcjj commented Feb 27, 2024

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

  • I have already looked over the issue tracker 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
@importcjj importcjj added the bug Something isn't working label Feb 27, 2024
@weiznich
Copy link
Owner

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.

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

@weiznich weiznich added the help wanted Extra attention is needed label Feb 27, 2024
@importcjj
Copy link
Author

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.

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
Copy link
Owner

weiznich commented Mar 1, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants