sysown / proxysql

High-performance MySQL proxy with a GPL license.
http://www.proxysql.com
GNU General Public License v3.0
5.97k stars 972 forks source link

Packet out of order when fetching connection_id using binary protocol #2564

Open phoenix1337 opened 4 years ago

phoenix1337 commented 4 years ago

I am currently using PHP in combination with PDO and turned emulate prepares off so PHP uses the binary protocol to prepare statements server side.

When I execute a SELECT CONNECTION_ID(), the PDO driver is returing the following warnings:

It does not matter if I execute the query or not.

If I then prepare another statement without executing the first one, ProxySQL is terminating the client connection with the following line in the log:

MySQL_Thread.cpp:4583:process_all_sessions(): [WARNING] Closing unhealthy client connection 127.0.0.1:48084

I am using the following script to reproduce the error:

<?php

$pdo = new PDO('mysql:dbname=...;host=127.0.0.1;port=6033;charset=utf8mb4', '...', '...');
$pdo->setAttribute(PDO::ATTR_EMULATE_PREPARES, false);
$pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);

$statement1 = $pdo->prepare('SELECT CONNECTION_ID()'); // throws the warnings about wrong response size
$statement2 = $pdo->prepare('SELECT 1'); // connection is terminated (MySQL server has gone away) and an error in the log

I think it has to do with the way ProxySQL handles this statement , related to the fix in issue #1797.

https://github.com/sysown/proxysql/blob/13653c7ca49a54269fbb85bffb5f5e0b295c1b60/lib/MySQL_Session.cpp#L6215

When i execute the query with more spaces between SELECT and CONNECTION_ID, it is executing without any problem because it is handled by MariaDB instead of ProxySQL.

The version of ProxySQL i am using is proxysql 2.0.9 on CentOS Linux release 7.7.1908 (Core).

renecannao commented 4 years ago

hi @phoenix1337 . Your analysis seems correct. What do you need the CONNECTION_ID for ? Because of multiplexing, knowing the backend connection id is probably of no use.

We can fix this bug, but I want to understand if the call itself makes any sense. Thanks

phoenix1337 commented 4 years ago

Hi @renecannao

I am currently using the CONNECTION_ID for logging and analysis when debugging some errors.

I understand it is of no use to receive the CONNECTION_ID from the backend to proxysql because of multiplexing.

When i execute the query with emulate prepares turned on, I receive the connection_id from the client to proxysql if I am correct? I would expect that I receive the exect same id when emulate prepares is turned off.

renecannao commented 4 years ago

When i execute the query with emulate prepares turned on, I receive the connection_id from the client to proxysql if I am correct?

Absolutely correct.

The request makes sense, and needs to be fixed. It probably affects a lot of special queries.

We have 2 possible ways to solve this: a) never prepare this statement (and similar) and completely emulate prepare/execute/execute b) prepare this statement (to get metadata from backend), but never execute (forging a resultset)

I think option B is easier, yet not immediate for performance reason. We probably needs to extend class stmt_execute_metadata_t() to flag a PS as a "special" one if it is it a PS that should never be executed. The flag needs to be set during STMT_PREPARE. If during STMT_EXECUTE we found that the flag is set, we handle it in a similar way to MySQL_Session::handler_special_queries().

phoenix1337 commented 4 years ago

I think option A is the best option because of the performance gain like you say, but if we don't use proxysql, the query is also send to the back-end so I don't think B is a bad way to implement it.

For now we can work around this issue by disabling the query that is fetching the CONNECTION_ID. So if you want to implement A and need more time for that, it's fine with me.

You are saying it probably also affects a lot of other special queries. Which type of queries do you mean, so I can check if it also affects other queries we do. Thanks in advance.