PacketOutOfOrder error on connect with too many connections

[jonhoo]

Conn::new returns a PacketOutOfOrder error if too many connections are established at the same time to a MySQL database. This doesn't seem like the right error to return? You can try this with something like:

#[tokio::test(threaded_scheduler)]
async fn ooo_error() {
    use futures_util::stream::StreamExt;

    let (tx, mut rx) = tokio::sync::mpsc::unbounded_channel::<()>();
    for _ in 0..1000 {
        let tx = tx.clone();
        tokio::spawn(async move {
            let c = Conn::new(get_opts()).await.unwrap();
            let c = c.drop_query("SELECT 1").await.unwrap();
            c.disconnect().await.unwrap();
            drop(tx);
        });
    }

    // wait until all connections have been dropped
    drop(tx);
    assert_eq!(rx.next().await, None);
}

Which will panic at Conn::new(get_opts()).await.unwrap() (expected) with a PacketOutOfOrder error (unexpected).

[jonhoo]

Note that the test above will appear to pass. You'll need to run it with -- --nocapture to see the panics.

[jonhoo]

In my experience, the test above fails if the number of connections established is >= half of MySQL's max_connections. The reason for this is that when a Plain connection is upgraded to a Socket connection, there is a period of time during which both versions of the connection are present, and so the connection counts double against the connection limit.

[jonhoo]

I have a weak suspicion that this might be related to #94. @blackbeam can you reproduce this issue? It should be relatively self-contained and easy enough to debug given that you know the protocol :) Especially because it doesn't require much concurrency at all.

[jonhoo]

I just ran into this issue again today sadly, so doesn't appear to have been fixed by the resolution to #94.

[blackbeam]

Oh, this is quite sad. I'll look into it again.
Could you please, if possible, share your server and OS version, and also a database configuration?

[blackbeam]

Please note that the biggest problem for me here was to reproduce this issue (I simply couldn't).

[jonhoo]

This was on an EC2 Ubuntu machine, running latest LTS, with the latest MariaDB version available there.
I was using the stock database configuration (though note the relationship to max-connections in the first post).

[blackbeam]

Btw, which version of mysql_common is in your Cargo.lock?

[jonhoo]

This was with mysql_async = "0.23.1", and the latest mysql_common that brings in, which I think is mysql_common = "0.22.1".

[blackbeam]

I've tried to reproduce it on t2.micro instance with bionic and stock MariaDB 10.1.44.
I got "to may connections", "connection reset" and plantly of other errors but no PacketOutOfOrder.
I tried with and without --release.

Maybe I'm missing something? Here is the code:

use mysql_async::{Conn, prelude::*};

use std::future::Future;

#[tokio::main(core_threads=8)]
async fn main() {
    use futures::stream::StreamExt;

    for _ in 0..100_000 {
        let (tx, mut rx) = tokio::sync::mpsc::unbounded_channel::<()>();
        for i in 0..160 {
            let tx = tx.clone();
            tokio::spawn(async move {
                let c = Conn::new("mysql://user:password@127.0.0.1/mysql").await.unwrap();
                let c = c.drop_query("SELECT 1").await.unwrap();
                c.disconnect().await.unwrap();
                drop(tx);
            });
        }
        drop(tx);
        assert_eq!(rx.next().await, None);
    }
}

[jonhoo]

That's very weird — I don't have a good answer for you unfortunately. It could be that it only happens when there's significant concurrency, which might require more cores than a t2.micro. I'm running on a 16-core instance when I see the issue.

[blackbeam]

Hi, @jonhoo. Could you please test the following revision in your environment?

[dependencies]
mysql_async = { git = "https://github.com/blackbeam/mysql_async.git", rev = "4602469d303321ca4a307c7377b8502e8b5a77cf" }

[jonhoo]

Ooh, interesting, #4602469d303321ca4a307c7377b8502e8b5a77cf does look like a promising fix! Unfortunately I'm pretty swamped in the coming month(s) as I'm also moving cross-country, so may not be able to test this for a while. I'll add it to my backlog though!