Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:111538
Message-ID: <tencent_F370CD37F9C8DC20AF346DF8CEC1F1FF0A06@qq.com>
Content-Type: text/plain;
	charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 13.4 \(3608.80.23.2.2\))
In-Reply-To: <CA+NMNO163WmNqf1qoGEXQwZYwXbjK14aoP_jPJmL2Vmpdw66OQ@mail.gmail.com>
Date: Sat, 15 Aug 2020 22:37:59 +0800
Cc: PHP internals <internals@lists.php.net>
Content-Transfer-Encoding: quoted-printable
References: <CA+NMNO163WmNqf1qoGEXQwZYwXbjK14aoP_jPJmL2Vmpdw66OQ@mail.gmail.com>
To: erictnorris@gmail.com
Subject: Re: [PHP-DEV] Proposal: Add PDO::checkLiveness method
From: twose@qq.com (twosee)


> 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
>=20
> Hey internals,
>=20
> 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.
>=20
> 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.
>=20
> 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.
>=20
> 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.
>=20
> Thanks!
>=20
> 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