Conversation
Much of this is copied from the standard wpdb class. These is a lot of overriding, so that multiple databases can be supported. Also added support for the reconnect_retries variable added in wp 3.9.
Support for connecting to db with mysqli. Also supports persistent connections. If connection fails, tries again with mysql. If connection fails, try again with mysql_connect functions.
The core version of this method would not have worked as it didn’t pass the dbh param. This is a hacked version from WP 4.5
Assign the resource connection to dbhs.
With mysqli in the mix, do correct type checking.
Check if is db object. If not, return.
|
We have this running in production now and it working great. It should be merged. @johnjamesjacoby |
|
This looks great; thank you for doing this all, and submitting upstream here! After looking at the work you've put in, I have 1 question: should we continue down the road of including these both in the same drop-in, or does it make more sense to have 2 separate files (1 for MySQL and another for MySQLi)? Obviously, some of the improvements you've made do also improve MySQL support, so dividing this back up would not be a very fun task. If we think a few years ahead, is it possible we'd need to keep dividing this up to support more flavors, and be setting ourselves up for ever-increasing complexity? |
|
So there is already a fork of hyperdb that supports mysqli. It could be found here - https://github.com/soulseekah/hyperdb-mysqli So the reason I added supported for mysqli into this and not another file was for the following reason. Imagine you have multiple pools of servers, dev, stage and production. Your dev box is php 5.6 with mysqli support but production is 5.4. With support for both you use the same plugin for multiple environments. Detecting mysqli support means that you as a dev, don't need to understand the difference, it just "works". This is how the core works with mysqli. As for supporting other database drivers like pdo, there is this for pdo. As pdo is an optional database driver for now, I am sure the use case is small. I think it is a problem we should think about later. Currently I think the wpdb class should do a better job of abstracting the code that implement the database drivers. If you could have method like query, ping, error etc. Then all a pdo drivers should do is override these methods. I am just dreaming now. I have a card up related to this - https://core.trac.wordpress.org/ticket/36242 I do need to add more detail onto it.... |
That's compelling, and makes sense. Thanks for reminding me that that is a scenario someone may find themselves in. |
|
We're starting to run this on our servers, as we need mysqli for PHP 7+ :) I think having the switch for mysql/mysqli makes sense at the moment, but should probably be revisited in the future, as mysql is considered deprecated. In the meantime, 👍 on merging. |
|
Just echoing what @rmccue said, we are using this is production for some sites in the 20 million page views a month range, and haven't run into any issues yet. |
|
I have been running a similar level of traffic haven't seen any issues related to this. I get out of memory errors sometimes, but I think this related to issues in the core and a memory leak on longer running processes. I did find one small php warning on some edge cases. I have a PR for it #8 . Otherwise, thanks for the merge. |
4 participants
Surprising hyperdb doesn't support mysqli. There are forks out that do mysqli by @soulseekah . But this fork doesn't support falling back if mysqli is not present.
This pull request tries to map as close to WP core naming, with the following methods being synced from core.
Currently testing this in our testing environment. Results looking good.
Fixes #4