WordPress.org

1. baptiste
   Member
   Posted 4 years ago #

   I have WP-OpenID installed and it's working. But recently during some DB instability, I saw a lot of errors like these:

   WordPress database error: [Lost connection to MySQL server during query]
   SHOW TABLES LIKE 'wp_openid_associations'

   WordPress database error: [Multiple primary key defined]
   ALTER TABLE wp_openid_associations ADD PRIMARY KEY (server_url(235),handle)

   WordPress database error: [Duplicate key name 'server_url']
   ALTER TABLE wp_openid_nonces ADD UNIQUE KEY server_url (server_url(255),timestamp,salt)

   Now - I'm guessing the plugin uses SHOW TABLES to check if new tables need to be created. I hope it's only on admin page accesses and not every page load. But the puzzling thing is this:

   ALTER TABLE wp_openid_associations ADD PRIMARY KEY (server_url(235),handle)

   The server_url field in wp_openid_associations is 255, not 235. Why would the plugin even try to ALTER it to a smaller size?

2. baptiste
   Member
   Posted 4 years ago #

   Following up - I've been really concerned that WP-OpenID has added a ton of queries to my DB on every page load. It made no sense during a DB problem that I'd see so many SHOW and ALTER commands if the plugin was properly setup.

   I've been combing through code to try and confirm this and if I traced it properly, it seems like this plugin is adding SHOW queries for little benefit. But I may be wrong.

   startup is the bootstrap routine that adds the hooks, etc as needed. I'm assuming this gets called on every page access when the plugin is initialized. It seems that way. OK. now in startup, there is a call to late_bind before the delete_user add_action.

   Looking at late_bind, there is a check to see if it's enabled, which it would be, and then there is a check of the database tables ($store->check_tables). Looking at check_tables, it adds three SHOW commands (one for each table). I know that SHOW checks aren't that intensive, but I've never seen plugins check tables on every load - they usually check a DB version variable or something. Plus, if there is any type of DB error in the SHOW, no error code check is done - it seems like the SQL return code should be checked to make sure the SHOW query succeeded before you try to recreate/alter the tables.

   Why go through all this for normal page accesses? There is a TON of code included, excessive table checking, and such for every page load if I read this right. Why not try to exclude as much as you can based on the state of the user? If a valid user hash cookie exists (is one created once an OpenID auth is successful?), is there a need to keep loading everything? Can that has just be trusted until it expires? I know an unknown user who arrives has to load most of the plugin so an OpenID check can be done.

   I just wonder given the size of this plugin if there might be ways to trim down what is loaded and initialized depending on various state checks that can be done already. This would significantly reduce the load of loading this plugin. I've noticed a slowdown for sure.

   Another option might be to create an option that allows a site owner to not load WP-OpenID on every access (I would assume for auto login) and instead still requires a user to 'login' - ie you would only load the plugin when they were in admin, or when wp-login is loaded. This way you still get the benefit of OpenID (albeit without the 'auto login' potential when your WP cookie expires), but with a much lower load impact.

3. baptiste
   Member
   Posted 4 years ago #

   Regarding the table recreate on a DB error, I wonder if something like this might be better. Here's a change I'm testing in store.php function check_tables:

   if( $retry and !$ok) {
      if ($wpdb->last_error == '') {
         $this->core->setStatus( 'database tables', false,
            'Tables not created properly. Trying to create..' );
         $this->create_tables();
         $ok = $this->check_tables( false );
      } else {
         $ok = false;
         $message .= "Database Error: " . $wpdb->last_error;
      }
   } else {
      $this->core->setStatus( 'database tables', $ok?'info':false, $message );
   }
   return $ok;

   Not a huge deal, but avoids un-necessary ALTERS. Still not sure why that ALTER has 235 in it. I mean store.php has it setup as server_url(235), but it's still being created as 255.

4. Will Norris
   Member
   Posted 4 years ago #

   yes, a lot of the DB code has been needing work for a while... I'll see if I can clean that stuff up a bit. I'll follow up in this thread with what I come up with.

Topic Closed

This topic has been closed to new replies.