Change MySQL liveness function to check for EOF

[ericnorris]

Inspired by https://github.blog/2020-05-20-three-bugs-in-the-go-mysql-driver/, this PR proposes changing the check_liveness function in pdo_mysql from an active "ping" to a non-blocking read via php_stream_eof when using mysqlnd.

The end result is the same, and it eliminates the overhead and latency of sending a MySQL command. A trade-off, however, is that the ping command would reset MySQL's wait_timeout, whereas this implementation does not. I believe there is precedent for my change from other drivers - if I understand correctly, pdo_pgsql operates this way - but I would be open to adding an explicitPDO::ping method for people that may still want that behavior for drivers that support it.

I wasn't entirely sure if implementing the underlying mysqlnd functionality solely as a macro was appropriate (although it seems simple enough), so feel free to point me at the right place to put it if necessary.

[ramsey]

cc @morozov @mbeccati, this needs review. Can one of you take a look?

[morozov]

@ramsey sorry, I'm not familiar with this part of the codebase and the API.

[mbeccati]

Thanks @ramsey.

I don't think the functionality is equivalent, or even desired tbh. Changing behaviour is also risky and I recently learned the lesson (again) the hard way.

FWIW, pdo_pgsql "consumes" any previous unread query result and tries to reconnect on error and/or if libpq has determined that the connection was broken. I don't think it counts as precedent and IIRC it is what the pgsql docs suggest.

That said, I'm not a huge mysql expert myself so I call for help from @nikic and @andrey-dnh

[nikic]

I'm not really familiar with the full implications this change would have either.

A trade-off, however, is that the ping command would reset MySQL's wait_timeout, whereas this implementation does not.

This sounds concerning to me. The purpose of check_liveness() is to make sure that persistent connections are still live prior to reuse. I think this change would make it more likely that a connection times out in the time between the liveness check and the first real use of the connection. Basically if the liveness check happens shortly before the timeout, and the use happens shortly after it.

[andreyhristov]

Just a quick note. I personally don't like the macro touching the internals (the vio) directly instead of going over a hook. This behavior doesn't fit in the way mysqlnd's API is built.

[ericnorris]

@mbeccati I don't think this is dangerous, unless I'm misunderstanding something - I know I've been repeating myself a good bit, but it lines up with exactly what GitHub ended up doing, just in a different language. Point taken on pdo_pgsql not being the same, however.

@nikic I agree that there might be more of a chance that the connection could be dead, which is why I was suggesting the potential addition of a PDO::ping method. The application could decide if it wanted to pay the penalty of sending a ping immediately after getting the persistent connection, or in the case of my other PR (#5935), it could have a near zero-cost way of checking at any point whether the connection was still alive.

@andreyhristov understood! I'm open to changing this.

All that said though, considering the pushback in the other pull request I'm going to go ahead and close this one. If anyone else feels more strongly about implementing this, I'm happy to help.