Expose Doctrine support for persistent connections in PDO in Nextcloud settings

[Retidurc]

This is a very naive PR to expose Doctrine support for persistent connections in PDO doctrine/dbal#2315 in Netxcloud settings.

This setting enable the use of PDO::ATTR_PERSISTENT in the PDO driver .

This, with the various "persistent" options suggested in the docs allow for php-fpm to keep open and reuse connexion to the backend database. Not unlike a pool of connexion we can encounter in other languages.

I am personally using this modification, with a postresql database and pgsql.allow_persistent = On for a small nextcloud shared between friends and haven't seen anything weird yet. Although my use case is fairly small and problems may arise on larger instances

[PVince81]

wouldn't it work to set it to "dbdriveroptions" directly in config.php, for example:

'dbdriveroptions' => [
   PDO::ATTR_PERSISTENT => true,
]

[Retidurc]

wouldn't it work to set it to "dbdriveroptions" directly in config.php, for example:

'dbdriveroptions' => [
   PDO::ATTR_PERSISTENT => true,
]

It's more or less what my first try to tinker with PDO::ATTR_PERSISTENT looked like.

Although it works, I think it would be safer and more future-proof to expose a simple true/false flag, in turn enabling/disabling the setting in doctrine, rather than tuning the PDO driver from the nextcloud config file.

Let me explain:
PDO::ATTR_PERSISTENT may have some side effect, If there is an unpredictable error during the script execution, the connection is blocked and can not be re used. AFAIK, Doctrine dbal handles those cases when their own persistent option is enabled, they may handle more subtle cases in the future. Directly tuning the PDO driver will not enable this behavior. (and Kinda defeat the idea of using a abstraction layer)

[CarlSchwan]

I'm with this, maybe at some point we could do that by default when running php-fpm if we don't notice anything weird

[Retidurc]

I'll also update doc on the other repo for this new tunable.

Friends already using this modification on their instances reported undesirable side effects when using

mysql.max_persistent=-1
mysql.max_links=-1

or

pgsql.max_persistent = -1
pgsql.max_links = -1

AKA "use an unlimited ammount of connexion to the db" , while php-fpm is set to pm = dynamic.

Thoses are the settings we can see here in the docs

As far as I can tell, php-fpm open a new connexion for each new children, and doesn't release the open connexion when a child is terminated.

It's not Nextcloud fault, but it's something to be aware of before enabling persistent connexion to the db.

I'll take a closer look later to see if it's something coming from doctrine dbal or php itself. In the meantime, I'll try to find sane default or alternatives to the aforementioned settings.

[blizzz]

At the moment we don't set it, if not given, and doctrine sets permanent connections by default for the relevant engines:

ack ATTR_PERSISTENT 3rdparty/
3rdparty/doctrine/dbal/src/Driver/PDO/OCI/Driver.php
21:            $driverOptions[PDO::ATTR_PERSISTENT] = true;

3rdparty/doctrine/dbal/src/Driver/PDO/PgSQL/Driver.php
23:            $driverOptions[PDO::ATTR_PERSISTENT] = true;

3rdparty/doctrine/dbal/src/Driver/PDO/MySQL/Driver.php
21:            $driverOptions[PDO::ATTR_PERSISTENT] = true;

3rdparty/doctrine/dbal/src/Driver/PDO/SQLSrv/Driver.php
36:            $pdoOptions[PDO::ATTR_PERSISTENT] = true;

Applying the change would switch the normal behaviour to non-persistent connections.

[Retidurc]

At the moment we don't set it, if not given, and doctrine sets permanent connections by default for the relevant engines:

ack ATTR_PERSISTENT 3rdparty/
3rdparty/doctrine/dbal/src/Driver/PDO/OCI/Driver.php
21:            $driverOptions[PDO::ATTR_PERSISTENT] = true;

3rdparty/doctrine/dbal/src/Driver/PDO/PgSQL/Driver.php
23:            $driverOptions[PDO::ATTR_PERSISTENT] = true;

3rdparty/doctrine/dbal/src/Driver/PDO/MySQL/Driver.php
21:            $driverOptions[PDO::ATTR_PERSISTENT] = true;

3rdparty/doctrine/dbal/src/Driver/PDO/SQLSrv/Driver.php
36:            $pdoOptions[PDO::ATTR_PERSISTENT] = true;

Applying the change would switch the normal behaviour to non-persistent connections.

If you check those files, there is a if statement around all thoses lines.

grep -rin -B 2 -A1 "PDO::ATTR_PERSISTENT" .
./doctrine/dbal/src/Driver/PDO/PgSQL/Driver.php-21-
./doctrine/dbal/src/Driver/PDO/PgSQL/Driver.php-22-        if (! empty($params['persistent'])) {
./doctrine/dbal/src/Driver/PDO/PgSQL/Driver.php:23:            $driverOptions[PDO::ATTR_PERSISTENT] = true;
./doctrine/dbal/src/Driver/PDO/PgSQL/Driver.php-24-        }

./doctrine/dbal/src/Driver/PDO/MySQL/Driver.php-19-
./doctrine/dbal/src/Driver/PDO/MySQL/Driver.php-20-        if (! empty($params['persistent'])) {
./doctrine/dbal/src/Driver/PDO/MySQL/Driver.php:21:            $driverOptions[PDO::ATTR_PERSISTENT] = true;
./doctrine/dbal/src/Driver/PDO/MySQL/Driver.php-22-        }

./doctrine/dbal/src/Driver/PDO/OCI/Driver.php-19-
./doctrine/dbal/src/Driver/PDO/OCI/Driver.php-20-        if (! empty($params['persistent'])) {
./doctrine/dbal/src/Driver/PDO/OCI/Driver.php:21:            $driverOptions[PDO::ATTR_PERSISTENT] = true;
./doctrine/dbal/src/Driver/PDO/OCI/Driver.php-22-        }

./doctrine/dbal/src/Driver/PDO/SQLSrv/Driver.php-34-
./doctrine/dbal/src/Driver/PDO/SQLSrv/Driver.php-35-        if (! empty($params['persistent'])) {
./doctrine/dbal/src/Driver/PDO/SQLSrv/Driver.php:36:            $pdoOptions[PDO::ATTR_PERSISTENT] = true;
./doctrine/dbal/src/Driver/PDO/SQLSrv/Driver.php-37-        }

This is set only if "persistent" is present in the config passed to doctrine. Wich is what I'm doing there

[blizzz]

If you check those files, there is a if statement around all thoses lines.

Darn, misinterpreted 😵‍💫

[welcome]

Thanks for your first pull request and welcome to the community! Feel free to keep them coming! If you are looking for issues to tackle then have a look at this selection: https://github.com/nextcloud/server/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22

[szaimen]

I'll take a closer look later to see if it's something coming from doctrine dbal or php itself. In the meantime, I'll try to find sane default or alternatives to the aforementioned settings.

@Retidurc did you do you research on the topic?
What is your conclousion? If using that setting, does one need to set
pgsql.max_persistent = -1
pgsql.max_links = -1
to not-unlimited?