`db-cluster-config.php` does not work for non network mode

jpSimkins commented 2 years ago

I have a single "blog" website. I am NOT using "network mode". I was instructed when I bought this that this still works if I put the db-cluster-config.php in the wp-content location. I have done this and this does not work.

I have already set the define('W3TC_PRO_DEV_MODE', true); in my wp-config.php and set the license in the setting page.

I am testing on my local DEV since I am already using HyperDB to have a multi-node deployment of WordPress running in Fargate on PROD. I have a local DB cluster setup for DEV too. I need to verify this will work on DEV before I even consider putting this on PROD.

I have removed HyperDB and added the drop-in for W3TC db.php and placed the db-cluster-config.php. I can verify through debugging that I am hitting the db.php drop-in as expected. I am never hitting the db-cluster-config.php file in debug.

Support told me:

As for the DB cluster, initially, this was intended to network mode, however, it can be used in the single-blog setups, but the mentioned check-box will not be visible, however, once the dbcluster-config-sample.php file is edited and placed in /wp-content/db-cluster-config.php - it starts to work

But this seems to be missing that the db.php needs to be in place as well. Which is obvious but was not mentioned. The only way the db-cluster-config.php is loaded, is when the db.php file is present. If you remove it, it will not load db-cluster-config.php.

So I have the files in place. I added this to the db-cluster-config.php:

if (defined('DB_READ_HOST')) {
    // Overrides the databases key above
    $w3tc_dbcluster_config['databases'] = array(
        'master' => array(
            'host'     => DB_HOST,
            'user'     => DB_USER,
            'password' => DB_PASSWORD,
            'name'     => DB_NAME,
            'write'    => true,
            'read'     => false,
        ),
        'slave' => array(
            'host'     => DB_READ_HOST,
            'user'     => DB_USER,
            'password' => DB_PASSWORD,
            'name'     => DB_NAME,
            'write'    => false,
            'read'     => true,
            'timeout'  => 0.3,
        ),
    );
}

I modified this since the commented out sections replaced $w3tc_dbcluster_config which would result in the other indexes being removed. e.g. persistent, check_tcp_responsiveness, ...

I tested with original uncommitted code and my changes but it's never worked as this file is never processed.

I do have DB_READ_HOST defined but this still isn't working. This is the same config I used with HyperDB so it should work but I get nothing.

Again, when debugging, I never get to the db-cluster-config.php file.

Any suggestions? Or at least an ETA to when this will be properly supported other than coming soon?

Thanks for your time.

Note: Support has reached out to me and the issues I had with email support was due to me mistyping my email in the contact form. Therefore I removed this section as it was user error on my part.

UPDATE

I discovered that this isn't working due to W3TC_PRO not being defined. For now, I set this to true and I now get this to load the db-cluster-config.php file but it just errors out with:

PHP Fatal error: Uncaught Error: Object of class W3TC\DbCache_WpdbNew could not be converted to string in /var/www/html/wp-content/plugins/w3-total-cache/Enterprise_Dbcache_WpdbInjection_Cluster.php:873

I removed my code wrapped with DB_READ_HOST and see that even with the default setting, this is causing the same error above.

Still investigating.

UPDATE 2

The error above is due to:

Enterprise_Dbcache_WpdbInjection_Cluster.php Line: 872:

        if ( !empty( $this->wpdb_mixin->charset ) ) {
            $collation_query = "SET NAMES '$this->wpdb_mixin->charset'";
            if ( !empty( $this->wpdb_mixin->collate ) )
                $collation_query .= " COLLATE '$this->wpdb_mixin->collate'";
            mysqli_query( $this->wpdb_mixin->dbh, $collation_query );
        }

Should be using:

        if ( !empty( $this->wpdb_mixin->charset ) ) {
            $collation_query = "SET NAMES '{$this->wpdb_mixin->charset}'";
            if ( !empty( $this->wpdb_mixin->collate ) )
                $collation_query .= " COLLATE '{$this->wpdb_mixin->collate}'";
            mysqli_query( $this->wpdb_mixin->dbh, $collation_query );
        }

Since it is using object variables in a string, you need to wrap them with {}. This is due to using $this->wpdb_mixin->charset as $this->wpdb_mixin would work as is (if it were a string, but it's an object so it won't here, just an example) but since you are using another varaible within wpdb_mixin, you need to add the {}. This shouldn't matter if this is in "network mode" or not, I wonder how many people actually use this feature? It's broken as is.

The site now partially loads... but it's still not working properly. Seems like pagination is not working.

I have also tried commenting out my DB_READ_HOST section and have it just using the default DB setup. This shows the same issues. If I switch my branch back to HyperDB, no issues. Still looking into this. The concern now is that there isn't any error logs being written so this will be a bit tricky to isolate.

I'll see if I can identify the latest issue.

UPDATE 3

Seems like the cause of the issue in UPDATE 2 is due to $wpdb->found_posts not returning the proper value. It returns 1 when it should be showing pretty much anything else. It returns 1 when it should be 0, 6, 11, 1,200, etc. It seems to always return 1. It's funny since $wpdb->post_count is showing the value returned and $wpdb->found_posts is never changing from 1. So when $wpdb->post_count shows 6 $wpdb->found_posts still shows only 1.

This is the reason all custom post types (seems Pages works and I don't use Posts) no longer have pagination in the WP-Admin section either. This is affecting almost all results, not just custom queries (something I originally thought). The reason Posts/Pages works could be due to the way WordPress is doing them internally. Media pagination is broken too. Also, all taxonomies pagination is broken too in WP Admin.

Not sure why this is the case, again, this worked perfectly fine before. It's just the DB cluster of W3TC causing these issues (to be clear this isn't an issue with anything else).

The queries are just returning bad data so WordPress doesn't know how to paginate. This is affecting me since my home page is a paginated result.

Still looking around but it seems this is the reason for bad data. I'll see if I can figure out why this is happening. I'll pick up on this tomorrow. I'm surprised this isn't a known issue, this is completely broken.

UPDATE 4

Still looking into the issue but I did want to make note that the $w3tc_dbcluster_config['databases'] index is not used as expected.

For this example, I will just use the default setup and not have the slave added yet.

I changed my DB_PASSWORD constant to be the incorrect password and hard-coded it into the config:

    'databases' => array(
        'master' => array(
            'host'     => DB_HOST,     // If port is other than 3306, use host:port.
            'user'     => DB_USER,
            'password' => 'password',
            'name'     => DB_NAME
        )
    ),

I get the error:

We were unable to select the database.

This is not expected behavior and should be noted.

This is strange because I would expect a "Could not connect to the database" error but that is not the case here. Again, the only change is the DB_PASSWORD.

It's possible the config is not really a config and DB_PASSWORD is still used in the code. Again, I would expect a different error.

mavas84 commented 2 years ago

Hello @jpSimkins

Thank you for taking the time to report this. First, I've replied to the existing email chain and provided an explanation of how the support works for W3 Total Cache, and the fact that the last email you sent did not have the correct email address (since this is a public repository I will not share that info here as I've already shared it via email). I've also explained that the best way to reach out to the support is via the plugin in Performance>Support as you have multiple free options like "Submit a Bug Report (Free)".

As for the DB cluster issue you are experiencing, we'll make sure to run tests for this and make sure everything is covered. It would be great if you could share more details about the website and the environment as I've mentioned in the email so we can have proper insights into the use case.

I am sorry again for any inconvenience and thank you for your patience.

jpSimkins commented 2 years ago

Thank you @mavas84

Although I was working on a vanilla website, I decided to try this again. As before, I just copied my files from my other project.

This time, I installed a fresh version of WP and installed W3TC from the admin plugins management system. I activated W3TC, added the constants:

define('W3TC_PRO_DEV_MODE', true);
define('W3TC_PRO', true);

I then saved my license key in W3TC settings.

As expected, the site shows:

W3 Total Cache is currently running in Pro version Development mode.

I then added the db-cluster-config.php file but it doesn't work. Since I already figured out this requires the db.php, I added that next.

Now the site is trying to get me to reinstall WP. It's like it isn't aware of my wp-config.php file existing.

If this helps, I am using a custom wp-content directory and my wp-config.php is outside the wordpress directory. This looks like:

├── .htaccess
├── index.php
├── wordpress
│   └── ...
├── wp-config.php
└── wp-content
    ├── advanced-cache.php
    ├── cache
    ├── db-cluster-config.php
    ├── db.php
    ├── plugins
    ├── themes
    ├── upgrade
    └── w3tc-config

I will continue to look into this and see if I can find where this is happening.

Just for testing reasons, if I copy my wp-config.php into the wordpress directory, it sees my settings again and the site loads. I do not have dummy data on this site yet so I am not testing if I still experience my issues from above yet. For now, I am trying to prove it is not locating the wp-config.php file location. This also seems to only be an issue when using db-cluster-config.php. When I remove it, the site loads again. Not sure why that would matter so perhaps the wp-config.php is not really the issue here. Very strange that when I move it, it works. Just making note of this as this is a little bizarre.

Update

I found that the issue seems to be due to Enterprise_Dbcache_WpdbInjection_Cluster specifically the initialize method.

It has:

        if ( isset( $GLOBALS['w3tc_dbcluster_config'] ) ) {
            $this->apply_configuration( $GLOBALS['w3tc_dbcluster_config'] );
        } elseif ( file_exists( WP_CONTENT_DIR . '/db-cluster-config.php' ) ) {
            // The config file resides in WP_CONTENT_DIR
            require WP_CONTENT_DIR . '/db-cluster-config.php';
        } else {
            $this->_reject_reason = 'w3tc dbcluster configuration not found, ' .
                'using single-server configuration';
            $this->next_injection->initialize();
            return;
        }

The issue is due to $GLOBALS['w3tc_dbcluster_config'] never being set. Once it loads the file, it never sets it.

I made this change:

        if ( !isset( $GLOBALS['w3tc_dbcluster_config'] ) && file_exists( WP_CONTENT_DIR . '/db-cluster-config.php' ) ) {
            // The config file resides in WP_CONTENT_DIR
            require WP_CONTENT_DIR . '/db-cluster-config.php';
        }

        if ( isset( $GLOBALS['w3tc_dbcluster_config'] ) ) {
        $this->apply_configuration( $GLOBALS['w3tc_dbcluster_config'] );
    } else {
        $this->_reject_reason = 'w3tc dbcluster configuration not found, ' .
            'using single-server configuration';
        $this->next_injection->initialize();
        return;
    }

and now I am able to use my site. The logic is still valid. Since I am including db-cluster-config.php before the appy_configuration method is called, this now works. Otherwise, appy_configuration can never be reached.

Update 2

I decided to import some data using: https://wpcom-themes.svn.automattic.com/demo/theme-unit-test-data.xml

First thing I noticed, I am still having the pagination issue I listed above in UPDATE 3 of my original post. Pagination is still broken. Even using the default theme in WordPress, the theme's pagination is broken as well. When I remove db-cluster-config.php pagination works as expected.

I have also tested UPDATE 4 from my original post and that no longer seems to be the case. Very strange but whatever, that is a good thing. This is most likely due to the change I made above.

It is interesting the UPDATE 2 from my original post is no longer causing an issue. I would still recommend my solution as that is what PHP recommends. I set a break point and am no longer getting to that code. Not sure why I was before so that explains why I am not having this issue anymore. Doesn't mean it won't cause an issue for someone else though. I'll have to see if I get to that part of the code again. For now, I will leave my breakpoint in place to see if I can ever get there as I am sure when I do, I will get the error again.

Well, that's it for today. I will look more into this issue tomorrow as UPDATE 3 is a pretty serious issue.

jpSimkins commented 2 years ago

It seems pagination is broken due to the way SELECT FOUND_ROWS() is being called. In Enterprise_Dbcache_WpdbInjection_Cluster in the query method, there is:

$this->wpdb_mixin->_last_found_rows_result = mysqli_query( $this->wpdb_mixin->dbh, "SELECT FOUND_ROWS()" );

This is wrong. You need to fetch using mysqli_num_rows. This should be:

$this->wpdb_mixin->_last_found_rows_result = mysqli_num_rows( $this->wpdb_mixin->dbh, "SELECT FOUND_ROWS()" );

Making this change now gets pagination working as expected.

I also do not see a reason to be using: $query .= "; SELECT FOUND_ROWS()"; in the same block. Not sure this is really needed. The concern is that shortly after, this is using:

        if ( function_exists( 'apply_filters' ) )
            apply_filters( 'after_query', $query );

Since the query added the ; SELECT FOUND_ROWS(). I don't think this is correct usage for that and could be a potential issue with other plugins that utilize this hook. Again, just a thought as this seems really unnecessary. I don't see any reason to append the SELECT FOUND_ROWS() to the query other than for logging? Please consider if this is necessary.

I didn't look into mssql.php as I do not use this so someone will need to check that to see if there are any issues.

jpSimkins commented 2 years ago

Submitted a PR to fix issues as this is definitely affecting anyone using it.

This fix is only fixing the db-cluster-config.php to work. When you enable DB cache, the site crashes. I am looking into that now. I may start a new ticket as this is a separate issue.

BoldGrid / w3-total-cache