Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:111643
MIME-Version: 1.0
References: <CA+NMNO163WmNqf1qoGEXQwZYwXbjK14aoP_jPJmL2Vmpdw66OQ@mail.gmail.com>
 <tencent_F370CD37F9C8DC20AF346DF8CEC1F1FF0A06@qq.com>
In-Reply-To: <tencent_F370CD37F9C8DC20AF346DF8CEC1F1FF0A06@qq.com>
Reply-To: erictnorris@gmail.com
Date: Wed, 19 Aug 2020 23:00:24 -0400
Message-ID: <CA+NMNO2eM9MZD4rL4=j0KiqmuiwoW0Sbma+Gd27cY6Tp5pqbfw@mail.gmail.com>
To: twosee <twose@qq.com>
Cc: PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="0000000000005049bf05ad465375"
Subject: Re: [PHP-DEV] Proposal: Add PDO::checkLiveness method
From: eric.t.norris@gmail.com (Eric Norris)

--0000000000005049bf05ad465375
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

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/22982eee339c4983c87cea5d59aaf48601ad703=
0/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?


On Sat, Aug 15, 2020 at 10:38 AM twosee <twose@qq.com> wrote:

>
> > 2020=E5=B9=B48=E6=9C=8815=E6=97=A5 =E4=B8=8A=E5=8D=882:16=EF=BC=8CEric =
Norris <eric.t.norris@gmail.com> =E5=86=99=E9=81=93=EF=BC=9A
> >
> > 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 persona=
l
> 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 cal=
l
> 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 make=
s
> 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 a=
n
> 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
>
>

--0000000000005049bf05ad465375--