We're getting a lot of slow transaction reports from nodewords SELECT queries (mainly from nodewords_custom, but I'll file another issue for that).

In general, why would nodewords_load_tags() be called at all, if the node that's being loaded is in teaser mode? It generates a lot of queries on list pages without any obvious benefit...or am I missing something conceptually?

Thanks for any clarification....

Comments

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Status: Active » Postponed

The nodewords are loaded during node_load(), which does not know what the desired build_mode is yet, we would have to change the loading mechanism to avoid that.

nicolash’s picture

nicolash commented

Maybe I'm looking at the wrong place, but couldn't the build mode be checked in hook_nodeapi()?

/**
 * Implements hook_nodeapi().
 */
function nodewords_nodeapi(&$node, $op, $teaser = NULL, $page = NULL) {
  switch ($op) {
    case 'load':
      if ($page) { // Only load meta tags for node detail views.
        $output['nodewords']['metatags'] = nodewords_load_tags(array(
          'type' => NODEWORDS_TYPE_NODE,
          'id' => $node->nid,
        ));
        return $output;
      }
  }
  // Rest of code omitted.
}
damienmckenna’s picture

damienmckenna
Location TN, USA
commented

Please check the API:

When node_load() is called it does not know what the build mode is going to be, $a3 and $a4 are both NULL.

nicolash’s picture

nicolash commented

Ha, thanks Damien.

And doing something hackish like

if (arg(0) == 'node') { ....

to make sure this only gets loaded on a detail view probably opens up another can of worms?

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Version: 6.x-1.12-rc1 » 6.x-1.x-dev

@NicolasH: exactly, we'll need to re-examine this later, after we have a solid 1.x release again.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Title: Why is nodewords_load_tags() called for teasers? » Rearchitect to not run any queries on node_load() unless necessary

I updated the title to be more descriptive of the core problem.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented

Would it be wrong to do a check to see if the current node is the one being loaded?

function nodewords_nodeapi(&$node, $op, $teaser = NULL, $page = NULL) {
[..]
    case 'load':
      if (arg(0) == 'node' && arg(1) == $node->nid) {
        return array(
          'nodewords' => nodewords_load_tags(NODEWORDS_TYPE_NODE, $node->nid),
        );
      }
      break;

We're already doing something similar for users:


function nodewords_user($op, &$edit, &$account, $category = NULL) {
  global $user;

  switch ($op) {
    case 'load':
      if (arg(0) == 'user' && !empty($account->uid) && variable_get('nodewords_enable_user_metatags', TRUE)) {
        $account->nodewords = nodewords_load_tags(NODEWORDS_TYPE_USER, $account->uid);
      }
      break;

Another idea would be to completely stop loading data in both hook_nodeapi() & hook_user() and just loading it via template_preprocess_page() so that the data is only ever loaded if it's appropriate for that page.

Thoughts?

damienmckenna’s picture

damienmckenna
Location TN, USA
commented

I'm thinking of going a step further and *not* doing any data loading until it gets to nodewords_preprocess_page(), and only then decide what to do..

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Title: Rearchitect to not run any queries on node_load() unless necessary » Only load data on hook_preprocess_page()
Status: Postponed » Active

Declaring my intentions.

Once that's done I'll be able to cache the results.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Version: 6.x-1.x-dev » 6.x-2.x-dev

Bumping this to v2.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Status: Active » Postponed

Marking all v2 issues as postponed while v1 is finished off.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Issue summary: View changes
Status: Postponed » Closed (won't fix)

Unfortunately this module is no longer supported, so I'm closing this issue.