Proposal: Add PDO::checkLiveness method View as Markdown

Hey Twosee,

Thanks for following up!

checkLiveness() is not cost-free, it will also cause redundant system
call overhead, In fact, it has been heavily abused. I noticed that many PHP
network programming implementations do not care about the overhead of
system calls, such as constantly using recv(MSG_PEEK) to check the
connection before new requests, this is how checkLiveness() works. This
will cause severe performance degradation because failure is always
unexpected.

I would assume that this overhead is negligible compared to the cost of
retrying a failed query - especially for the times when a query cannot be
safely retried. That being said, making the method available for folks to
use doesn't force them to use it :). An application developer can weigh the
tradeoffs for their use-case.

And it even can't guarantee that the next call is successful, the
connection may be reset after check.
What can you do if you noticed that the connection is disconnected?
phpredis checks the connection before each request and tries to reconnect
because the redis connection is almost stateless. So in phpredis, it makes
some sense to do so (phpredis will retry 10 times when the connection is
down, but this may be bad when the server is busy).

Absolutely! As I mentioned in the GitHub pull request thread, checkLiveness
does not guarantee the next call is successful, but it can tell you if
the next call will definitely fail without reconnecting. I think that
would be a worthwhile check, considering that it costs roughly a syscall to
do so. If checkLiveness returns false, your database code would simply
reconnect without potentially having to throw an exception (in the case of
a non-retryable query) and may end up being a better experience for a
hypothetical web user.

All in all, let it fail and try again, the overall performance will be
better than checking before calling it every time, so I don't think this
API is necessary.
Please don't use it, it will only make your program worse.

To be clear, I am not suggesting that developers call checkLiveness before
every query, I agree that would likely be unnecessary. Where it can be
useful though is for checking long-lived connections after some period of
inactivity, much like how PDO checks the connection state via
check_liveness before reusing a persistent connection (see
https://github.com/php/php-src/blob/22982eee339c4983c87cea5d59aaf48601ad7030/ext/pdo/pdo_dbh.c#L317-L331).
My
PR makes it so that you can do the very thing that PDO does, but
application side. I disagree that this makes your program worse, if so, why
should PDO do the same thing?

2020年8月15日 上午2:16，Eric Norris eric.t.norris@gmail.com 写道：

Hey internals,

I've submitted https://github.com/php/php-src/pull/5935 as a way to
expose
an underlying PDO driver's check_liveness function to userland scripts.
Often advice given on the internet is to issue a no-op statement (e.g.
SELECT 1 FROM dual), but this adds potentially unnecessary overhead and
latency. Using the PDO driver's check_liveness function offers a
lower-cost
way of ensuring the connection is still alive.

As noted in the PR, I am not tied to the method name, and open to any
suggestions to making the PR better - I'm mostly interested in the
underlying concept.

It appears the test I added is currently failing for pgsql. I didn't
have a
test setup for that on hand but will look into it if there is positive
feedback for this PR.

Relatedly, I've also submitted https://github.com/php/php-src/pull/5947
as
a way to potentially improve the mysqlnd driver's check_liveness
function.

Thanks!

Eric Norris

Hi, I am the author of Swoole (https://github.com/swoole/swoole-src), I
have a little network programming experience, the following is my personal
opinion:

I suggest not to check it, let it throw an exception (PHP8 has used
exceptions as the default error reporting way).

checkLiveness() is not cost-free, it will also cause redundant system call
overhead, In fact, it has been heavily abused. I noticed that many PHP
network programming implementations do not care about the overhead of
system calls, such as constantly using recv(MSG_PEEK) to check the
connection before new requests, this is how checkLiveness() works. This
will cause severe performance degradation because failure is always
unexpected.

And it even can't guarantee that the next call is successful, the
connection may be reset after check.

What can you do if you noticed that the connection is disconnected?
phpredis checks the connection before each request and tries to reconnect
because the redis connection is almost stateless. So in phpredis, it makes
some sense to do so (phpredis will retry 10 times when the connection is
down, but this may be bad when the server is busy).

But MySQL can't do this, the context of it is too complex, let it throw an
exception, catch it, and start from the beginning again is the best and
cheapest way to solve it.

All in all, let it fail and try again, the overall performance will be
better than checking before calling it every time, so I don't think this
API is necessary.

Please don't use it, it will only make your program worse.

Regards,
Twosee