Persistent database connections with mysqli

[blobaugh]

WordPress currently does not allow support for persistent database connections. This can be accomplished by prepending "p:" to the hostname with mysqli however with its current configuration WordPress will be confused by the ":" and think the hostname is actually a port if specified in the wp-config.php file. This patch add support for a constant that allows persistent connections to be turned on or off.

Why should this be added to core? Because persistent connections are useful :P. But really, we have seen requests for this in other tickets such as #27933. Additionally I am involved in a project where 10,000+ sites are requiring persistent db connection, not an insignificant number. Persistent db connections are also needed to ensure proper performance on IIS and Azure installations.

In short a couple lines of code is all it takes to ensure WordPress continues to work well across other platforms and project requirements.

Props to awoods and bradparbs for helping identify the issue and solution.

[blobaugh]

Adds support for persistent database connections

[awoods]

I think this is a good idea. There are some hosts, where it might not have much of an impact. But other hosts it will have a great positive impact on performance. It doesn't seem like a big deal until you need it and you can't use it. I like the idea of using by setting a constant in their wp-config.php to enable persistent connections. It's a great way to empower developers without them having to hack core.

We should ensure that a check for the mysql driver case is also covered -- for older hosts that might not be using mysqli. which is just an additional couple of lines of code.

[pento]

Thanks for the feature request, Ben!

While persistent connections can be useful under some limited circumstances, I don't think this is appropriate for core, for a couple of reasons:

• We really don't want to add new wp-config.php settings, especially for something as edge case as this.
• Persistent connections sound like a really good idea, but have bunch of gotchas that most users won't be aware of, or know how to mitigate. We don't want to encourage users to do things that will break their site, and persistent connections will do that in fairly obscure ways.

For situations like this, I'd recommend creating a DB drop-in that replaces wpdb::db_connect() with a persistent version.

[bradparbs]

Microsoft Azure recommends modifying core with a similar change for all of the WordPress installations on their. And we've seen pretty big performance increases with allowing persistant connections, due to the way the servers are set up.

I think allowing an option for the user to enable this is a good idea, even if it is an edge case.

[jtsternberg]

Replying to pento:

For situations like this, I'd recommend creating a DB drop-in that replaces wpdb::db_connect() with a persistent version.

Since azure is slowly becoming a viable option, and things like this exist ​https://wordpress.org/plugins/persistent-database-connection-updater/, it would seem we should think about a better solution. Hacking core on every upgrade? Ouch.
A) Gary, I'm guessing something like this would be a proper db drop-in replacement: ​https://gist.github.com/jtsternberg/eec4ab95e11ce9be4807
B) It seems we should be abstracting some of the functionality in wpdb::db_connect() to smaller methods. This would allow more precision in drop-in replacements. I'll submit a patch for at least that.

[jtsternberg]

While I'm still a +1 on the original patch (I think persistent connections are useful enough to warrant config override), I still think some abstraction is needed in these methods. This patch abstracts mysqli_real_connect and mysql_pconnect, for easy replacement by drop-in replacements.

[bradparbs]

+1 to Justin's latest patch.

[jtsternberg]

31018-2.diff Would simplify this: ​https://gist.github.com/jtsternberg/eec4ab95e11ce9be4807
to this: ​https://gist.github.com/jtsternberg/6e6f77f4bc27f5b0df81

[blobaugh]

Replying to pento:

• We really don't want to add new wp-config.php settings, especially for something as edge case as this.

Could you explain why this is? It seems new settings are warranted as there is no other way to modify how the database connection works.

• Persistent connections sound like a really good idea, but have bunch of gotchas that most users won't be aware of, or know how to mitigate. We don't want to encourage users to do things that will break their site, and persistent connections will do that in fairly obscure ways.

Uh huh, so does setting DB_HOST or DB_CHARSET or WP_CONTENT_DIR or any number of other settings. They can all break a user's site. That is why they are in the settings file and not exposed in the UI for users to play with. That argument does not seem to stand up to the test of other settings.

For situations like this, I'd recommend creating a DB drop-in that replaces wpdb::db_connect() with a persistent version.

Yes this is one option, however it is not a good option because core changes how connections work periodically. The site owner would need to be aware that core was changing so they could hire a dev to update their drop-in file. It is not realistic for a site owner to track that. They should only have to enjoy their site. We are already seeing many instances of this happening with IIS based installs. The plugin jtsternberg mentioned goes to some length to capture this but is ultimately a bad hack and very fragile but it is currently how I have seen adding persistent connections to WordPress recommended (though I do agree it is horrible).

[pento]

Replying to blobaugh:

Could you explain why this is? It seems new settings are warranted as there is no other way to modify how the database connection works.

New settings are almost never warranted - I think the last one we added was WP_MAX_MEMORY_LIMIT, added in r17749, released in WordPress 3.2.

Uh huh, so does setting DB_HOST or DB_CHARSET or WP_CONTENT_DIR or any number of other settings. They can all break a user's site. That is why they are in the settings file and not exposed in the UI for users to play with. That argument does not seem to stand up to the test of other settings.

Of course you can break your site by messing up your other settings. The difference is that DB_PERSISTENT would allow you to break your site in an inconsistent manner, just by enabling it. Persistent connections are kind of like MySQL's query cache in this regard - they were very useful to improve performance in MySQL 4.0, and can even give performance benefits today. They'll keep giving performance benefits, right up until they explode.

In the case of persistent connections, that explosion point is when a PHP process dies without releasing all of its locks. Those locks will be held until the connection is recycled, and mysqli forces all locks to be released. In the intervening time, your site is locked with no obvious reason why. For this reason alone, I'd want to actively discourage users from enabling persistent connections, by specifically not supporting it in core.

Yes this is one option, however it is not a good option because core changes how connections work periodically. The site owner would need to be aware that core was changing so they could hire a dev to update their drop-in file. It is not realistic for a site owner to track that. They should only have to enjoy their site. We are already seeing many instances of this happening with IIS based installs. The plugin jtsternberg mentioned goes to some length to capture this but is ultimately a bad hack and very fragile but it is currently how I have seen adding persistent connections to WordPress recommended (though I do agree it is horrible).

That's the deal with DB drop-ins - they need to be maintained.

Well, that's not entirely true. When we change WPDB, we keep drop-ins in mind, so that we don't suddenly cause them to stop functioning. They really only need to be updated to take advantage of new features (see HyperDB, which functions quite well without having added mysqli support).

Regarding 31018-2.diff​, this will create severe maintenance problems - even if we were to switch these functions to protected, instead of public (so that plugins don't start calling them), we'd still need to support them for DB drop-ins, like the one being suggested. If we were to move to a DB driver system, for example, we'd likely be stuck with these driver-specific functions on the global API.

This ticket comes down entirely to where we want the maintenance burden to rest - on core, to maintain support for persistent connections, or on a DB drop-in, to maintain feature parity with core. Given the extremely limited audience of this feature, it seems to me that core isn't the right place for this.

[MrGregWaugh]

Enable pass through 'p:' from DB_HOST to mysqli_real_connect.

[MrGregWaugh]

So I'm going to make one more stab at this. I agree with most here that while persistent database connections are generally not needed, there are specific cases where they are of great benefit, the example of using ClearDB being one. Seeing "solutions" like the "persistent-database-connection-updater" make me really sad.

The only thing really preventing one from being able to use valid mysqli host syntax in wp-config.php for persistent connections (prepending a 'p:') is the port_or_socket() code in wp-db.php, which according to the comments "duplicates how mysql_connect detects a port and/or socket file". My proposed solution is to enable that to correctly pass through a 'p:' from DB_HOST to mysqli_real_connect.

For example, you would be able to do:

define('DB_HOST', 'p:db-host.expensive-connection.com');

Note: this patch is intended to enable this functionality with absolutely minimal changes to the core code.

Patch_31018-3 (raw)

Here is the complete code segment, from wp-db.php line 1431

			// mysqli_real_connect doesn't support the host param including a port or socket
			// like mysql_connect does. This duplicates how mysql_connect detects a port and/or socket file.
			$port = null;
			$socket = null;
			$host = $this->dbhost;
			$pre_host = '';
			// If DB_HOST begins with a 'p:', allow it to be passed to mysqli_real_connect().
			// mysqli supports persistent connections starting with PHP 5.3.0.
			if (version_compare( phpversion(), '5.3.0', '>=' ) && 0 === strpos( $host, 'p:' )) {
				$host = substr( $host, 2 );
				$pre_host = 'p:';
			}
			$port_or_socket = strstr( $host, ':' );
			if ( ! empty( $port_or_socket ) ) {
				$host = substr( $host, 0, strpos( $host, ':' ) );
				$port_or_socket = substr( $port_or_socket, 1 );
				if ( 0 !== strpos( $port_or_socket, '/' ) ) {
					$port = intval( $port_or_socket );
					$maybe_socket = strstr( $port_or_socket, ':' );
					if ( ! empty( $maybe_socket ) ) {
						$socket = substr( $maybe_socket, 1 );
					}
				} else {
					$socket = $port_or_socket;
				}
			}
			$host = $pre_host . $host;

			if ( WP_DEBUG ) {
				mysqli_real_connect( $this->dbh, $host, $this->dbuser, $this->dbpassword, null, $port, $socket, $client_flags );
			} else {
				@mysqli_real_connect( $this->dbh, $host, $this->dbuser, $this->dbpassword, null, $port, $socket, $client_flags );
			}

[blobaugh]

I think this is a good solution. It avoids the creation of a new constant and still allows advanced users/dev the ability to have their persistent connections. Thanks @MrGregWaugh

[lisota]

+1 for this solution.

WordPress needs an option to support persistent MySQL connections for folks that need it and know what they are doing. There are valid cases to use persistent connections to MySQL, and hosters out there who recommend its use.

Supporting persistent connections should not require rewriting core files, namely wp-db.php, which are easily overwritten by core updates.

The fix is a trivial pass-thru of the "p:" parameter. Anyone sophisticated enough to add that parameter to the DB_HOST does not need hand holding.

[pento]

31018-3.diff​ is a direction I like.

I'd like to see support for ext/mysql added to the patch before we commit to anything, though.

If someone feels like picking it up, dusting it off, and adding some unit tests, I think we might be onto something.

[hr1232]

Has there been any development towards solving this?

I have to ask myself, why is something so ugly like this still in there?

<?php
$port_or_socket = strstr( $host, ':' );

mysql_connect is long gone and mysqli_connect takes port and socket as two different parameters. Why not make them available as two different configuration parameters in wp-config.php, just like any other modern PHP app does? If a parameter is not set, it defaults to null. If both are set, mysqli decides what to use (preferably socket the connection).

Sorting socket- and tcp-based conenctions by looking if the hostname contains a colon is a very dirty thing. especially if you doen't even check if whatever comes after the colon is a valid port number...

Using mysqli_connect correctly (i.e. without that ugly piece of history) would not only solve the problem of persistent connections not being available in WordPress, it would also shorten the core code by a few unnecessary, ugly lines....

[diddledani]

Refresh of 3108-3

[diddledani]

3108-4.diff is a refreshed patch with a few extra niceties (the define for DB_PERSISTENT_CONNECTION) and work to implement ext/mysql support, though this might be possible to drop for the patch now that PHP5.6 is minimum requirement with a view to moving to 7.0+ minimum soon.

[ShadowXVII]

I bought up the same issue with the WP Performance team.
I'll cross link here:

​https://github.com/WordPress/performance/issues/403