http://drupal.org/node/105639 From: andrew morton Tracker performance improvements --- modules/tracker/tracker.admin.inc | 38 ++++ modules/tracker/tracker.module | 345 +++++++++++++++++++++++++++++++++++++ modules/tracker/tracker.pages.inc | 84 +++++---- modules/tracker/tracker.test | 89 ++++++++++ 4 files changed, 516 insertions(+), 40 deletions(-) create mode 100644 modules/tracker/tracker.admin.inc diff --git modules/tracker/tracker.admin.inc modules/tracker/tracker.admin.inc new file mode 100644 index 0000000..07c41c5 --- /dev/null +++ modules/tracker/tracker.admin.inc @@ -0,0 +1,38 @@ + $max_nid)); + $form['max_nid'] = array( + '#markup' => t('Tracker has @count remaining items to index.', array('@count' => $count)), + ); + } + else { + $form['max_nid'] = array( + '#markup' => t('All content has been indexed.'), + ); + } + + $items = drupal_map_assoc(array(100, 200, 500, 1000, 2000, 5000)); + $form['tracker_batch_size'] = array( + '#type' => 'select', + '#title' => t('Batch size'), + '#description' => t('The maximum number of items indexed in each pass of a cron maintenance task. If necessary, reduce the number of items to prevent timeouts and memory errors while indexing.', array('@cron' => url('admin/reports/status'))), + '#options' => $items, + '#default_value' => variable_get('tracker_batch_size', 1000), + ); + + return system_settings_form($form); +} \ No newline at end of file diff --git modules/tracker/tracker.module modules/tracker/tracker.module index 658c3f2..353cfc6 100644 --- modules/tracker/tracker.module +++ modules/tracker/tracker.module @@ -32,14 +32,19 @@ function tracker_menu() { ); $items['tracker/all'] = array( 'title' => 'All recent posts', + 'page callback' => 'tracker_page', + 'access arguments' => array('access content'), 'type' => MENU_DEFAULT_LOCAL_TASK, + 'file' => 'tracker.pages.inc', ); $items['tracker/%user_uid_optional'] = array( 'title' => 'My recent posts', + 'page callback' => 'tracker_page', 'access callback' => '_tracker_myrecent_access', 'access arguments' => array(1), 'page arguments' => array(1), 'type' => MENU_LOCAL_TASK, + 'file' => 'tracker.pages.inc', ); $items['user/%user/track'] = array( @@ -55,10 +60,110 @@ function tracker_menu() { 'title' => 'Track posts', 'type' => MENU_DEFAULT_LOCAL_TASK, ); + + $items['admin/settings/tracker'] = array( + 'title' => 'Tracker', + 'description' => 'Settings for the tracker module', + 'page callback' => 'drupal_get_form', + 'page arguments' => array('tracker_admin_settings'), + 'access arguments' => array('administer tracker'), + 'file' => 'tracker.admin.inc', + ); + return $items; } /** + * Implement hook_perm(). + */ +function tracker_perm() { + return array( + 'administer tracker' => array( + 'title' => t('Administer tracker'), + 'description' => t('Manage tracker administration settings.'), + ), + ); +} + +/** + * Implement hook_cron(). + */ +function tracker_cron() { + $max_nid = variable_get('tracker_index_nid', 0); + $batch_size = variable_get('tracker_batch_size', 1000); + if ($max_nid > 0) { + $last_nid = FALSE; + $result = db_query_range('SELECT nid, uid, status FROM {node} WHERE nid <= :max_nid ORDER BY nid DESC', array(':max_nid' => $max_nid), 0, $batch_size); + + $count = 0; + + foreach ($result as $row) { + // Calculate the changed timestamp for this node. + $changed = _tracker_calculate_changed($row->nid); + + // Remove existing data for this node. + db_delete('tracker_node') + ->condition('nid', $row->nid) + ->execute(); + db_delete('tracker_user') + ->condition('nid', $row->nid) + ->execute(); + + // Insert the node-level data. + db_insert('tracker_node') + ->fields(array( + 'nid' => $row->nid, + 'published' => $row->status, + 'changed' => $changed, + )) + ->execute(); + + // Insert the user-level data for the node's author. + db_insert('tracker_user') + ->fields(array( + 'nid' => $row->nid, + 'published' => $row->status, + 'changed' => $changed, + 'uid' => $row->uid, + )) + ->execute(); + + $query = db_select('comment', 'c'); + // Force PostgreSQL to do an implicit cast by adding 0. + $query->addExpression('0 + :changed', 'changed', array(':changed' => $changed)); + $query->addField('c', 'status', 'published'); + $query + ->distinct() + ->fields('c', array('uid', 'nid')) + ->condition('c.nid', $row->nid) + ->condition('c.uid', $row->uid) + ->condition('c.status', COMMENT_PUBLISHED); + + // Insert the user-level data for the commenters (except if a commenter is the node's author). + $query = db_insert('tracker_user') + ->from($query) + ->execute(); + + // Note that we have indexed at least one node. + $last_nid = $row->nid; + + $count++; + } + + if ($last_nid !== FALSE) { + // Prepare a starting point for the next run. + variable_set('tracker_index_nid', $last_nid - 1); + + watchdog('tracker', t('Indexed %count nodes for tracking.', array('%count' => $count))); + } + else { + // If all nodes have been indexed, set to zero to skip future cron runs. + variable_set('tracker_index_nid', 0); + } + } +} + +/** * Access callback for tracker/%user_uid_optional */ function _tracker_myrecent_access($account) { @@ -73,3 +178,243 @@ function _tracker_user_access($account) { return user_view_access($account) && user_access('access content'); } +/** + * Implement hook_form_FORM_ID_alter(). + */ +function tracker_form_node_admin_content_alter(&$form, $form_state) { + $form['admin']['options']['submit']['#submit'][] = 'tracker_batch_node_submit'; +} + +/** + * Submit handler for node admin content page. + * + * Updates tracker indexing tables when content is bulk updated. + */ +function tracker_batch_node_submit($form, &$form_state) { + $op = $form_state['values']['operation']; + if ($op == 'publish' || $op == 'unpublish') { + $published = ($op == 'publish') ? 1 : 0; + $nids = array_filter($form_state['values']['nodes']); + db_update('tracker_node') + ->fields(array('published' => $published)) + ->condition('nid', $nids) + ->execute(); + db_update('tracker_user') + ->fields(array('published' => $published)) + ->condition('nid', $nids) + ->execute(); + } +} + +/** + * Implement hook_nodeapi_insert(). + */ +function tracker_node_insert($node, $arg = 0) { + _tracker_add($node->nid, $node->uid, $node->changed); +} + +/** + * Implement hook_nodeapi_update(). + */ +function tracker_node_update($node, $arg = 0) { + _tracker_add($node->nid, $node->uid, $node->changed); +} + +/** + * Implement hook_nodeapi_delete(). + */ +function tracker_node_delete($node, $arg = 0) { + _tracker_remove($node->nid, $node->uid, $node->changed); +} + +/** + * Implement hook_comment_insert(). + */ +function tracker_comment_insert($comment) { + $comment = (array) $comment; + if ($comment['status'] == COMMENT_PUBLISHED) { + _tracker_add($comment['nid'], $comment['uid'], $comment['timestamp']); + } + else { + _tracker_remove($comment['nid'], $comment['uid'], $comment['timestamp']); + } +} + +/** + * Implement hook_comment_update(). + */ +function tracker_comment_update($comment) { + $comment = (array) $comment; + if ($comment['status'] == COMMENT_PUBLISHED) { + _tracker_add($comment['nid'], $comment['uid'], $comment['timestamp']); + } + else { + _tracker_remove($comment['nid'], $comment['uid'], $comment['timestamp']); + } +} + +/** + * Implement hook_comment_publish(). + */ +function tracker_comment_publish($comment) { + $comment = (array) $comment; + _tracker_add($comment['nid'], $comment['uid'], $comment['timestamp']); +} + +/** + * Implement hook_comment_unpublish(). + */ +function tracker_comment_unpublish($comment) { + $comment = (array) $comment; + _tracker_remove($comment['nid'], $comment['uid'], $comment['timestamp']); +} + +/** + * Implement hook_comment_delete(). + */ +function tracker_comment_delete($comment) { + $comment = (array) $comment; + _tracker_delete($comment['nid'], $comment['uid'], $comment['timestamp']); +} + +/** + * Update indexing tables when a node is added, updated or commented on. + * + * @param $nid + * A node ID. + * @param $uid + * The node or comment author. + * @param $changed + * The node updated timestamp or comment timestamp. + */ +function _tracker_add($nid, $uid, $changed) { + $node = db_query('SELECT nid, status, uid, changed FROM {node} WHERE nid = :nid', array(':nid' => $nid))->fetchObject(); + + // Adding a comment can only increase the changed timestamp, so our + // calculation here is simple. + $changed = max($node->changed, $changed); + + // Update the node-level data. + db_merge('tracker_node') + ->key(array('nid' => $nid)) + ->fields(array( + 'changed' => $changed, + 'published' => $node->status, + )) + ->execute(); + + // Create or update the user-level data. + db_merge('tracker_user') + ->key(array( + 'nid' => $nid, + 'uid' => $uid, + )) + ->fields(array( + 'changed' => $changed, + 'published' => $node->status, + )) + ->execute(); +} + +/** + * Determine the max timestamp between $node->changed and the last comment. + * + * @param $nid + * A node ID. + * + * @return + * The $node->changed timestamp, or most recent comment timestamp, whichever + * is the greatest. + */ +function _tracker_calculate_changed($nid) { + $changed = db_query('SELECT changed FROM {node} WHERE nid = :nid', array(':nid' => $nid))->fetchField(); + $latest_comment = db_query_range('SELECT cid, timestamp FROM {comment} WHERE nid = :nid AND status = :status ORDER BY timestamp DESC', array( + ':nid' => $nid, + ':status' => COMMENT_PUBLISHED, + ), 0, 1)->fetchObject(); + if ($latest_comment && $latest_comment->timestamp > $changed) { + $changed = $latest_comment->timestamp; + } + return $changed; +} + +/** + * Clean up indexed data when nodes or comments are removed. + * + * @param $nid + * The node ID. + * @param $uid + * The author of the node or comment. + * @param $changed + * The last changed timestamp of the node. + */ +function _tracker_remove($nid, $uid = NULL, $changed = NULL) { + $node = db_query('SELECT nid, status, uid, changed FROM {node} WHERE nid = :nid', array(':nid' => $nid))->fetchObject(); + + // The user only keeps his or her subscription if both of the following are true: + // (1) The node exists. + // (2) The user is either the node author or has commented on the node. + $keep_subscription = FALSE; + + if ($node) { + // Self-authorship is one reason to keep the user's subscription. + $keep_subscription = ($node->uid == $uid); + + // Comments are a second reason to keep the user's subscription. + if (!$keep_subscription) { + // Check if the user has commented at least once on the given nid + $keep_subscription = db_query_range('SELECT COUNT(*) FROM {comments} WHERE nid = :nid AND uid = :uid AND status = 0', array( + ':nid' => $nid, + ':uid' => $uid, + ), 0, 1)->fetchField(); + } + + // If we haven't found a reason to keep the user's subscription, delete it. + if (!$keep_subscription) { + db_delete('tracker_user') + ->condition('nid', $nid) + ->condition('uid', $uid) + ->execute(); + } + + // Now we need to update the (possibly) changed timestamps for other users + // and the node itself. + + // We only need to do this if the removed item has a timestamp that equals + // or exceeds the listed changed timestamp for the node + $tracker_node = db_query('SELECT nid, changed FROM {tracker_node} WHERE nid = :nid', array(':nid' => $nid))->fetchObject(); + if ($tracker_node && $changed >= $tracker_node->changed) { + // If we're here, the item being removed is *possibly* the item that + // established the node's changed timestamp. + + // We just have to recalculate things from scratch. + $changed = _tracker_calculate_changed($nid); + + // And then we push the out the new changed timestamp to our denormalized + // tables. + db_update('tracker_node') + ->fields(array( + 'changed' => $changed, + 'published' => $node->status, + )) + ->condition('nid', $nid) + ->execute(); + db_update('tracker_node') + ->fields(array( + 'changed' => $changed, + 'published' => $node->status, + )) + ->condition('nid', $nid) + ->execute(); + } + } + else { + // If the node doesn't exist, remove everything. + db_delete('tracker_node') + ->condition('nid', $nid) + ->execute(); + db_delete('tracker_user') + ->condition('nid', $nid) + ->execute(); + } +} diff --git modules/tracker/tracker.pages.inc modules/tracker/tracker.pages.inc index 2aa7193..1e2222e 100644 --- modules/tracker/tracker.pages.inc +++ modules/tracker/tracker.pages.inc @@ -11,60 +11,64 @@ * Menu callback. Prints a listing of active nodes on the site. */ function tracker_page($account = NULL, $set_title = FALSE) { - // TODO: These queries are very expensive, see http://drupal.org/node/105639 - $query = db_select('node', 'n', array('target' => 'slave'))->extend('PagerDefault'); - $query->join('users', 'u', 'n.uid = u.uid'); - $query->join('node_comment_statistics', 'l', 'n.nid = l.nid'); - $query->addExpression('GREATEST(n.changed, l.last_comment_timestamp)', 'last_updated'); - $query - ->distinct() - ->fields('n', array('nid', 'title', 'type', 'changed', 'uid')) - ->fields('u', array('name')) - ->fields('l', array('comment_count')) - ->condition('n.status', 1) - ->orderBy('last_updated', 'DESC') - ->addTag('node_access') - ->limit(25); - if ($account) { + $query = db_select('tracker_user', 't')->extend('PagerDefault'); + $query->condition('t.uid', $account->uid); + if ($set_title) { // When viewed from user/%user/track, display the name of the user // as page title -- the tab title remains Track so this needs to be done // here and not in the menu definition. drupal_set_title($account->name); } - $query->leftJoin('comment', 'c', 'n.nid = c.nid AND (c.status = :status OR c.status IS NULL)', array(':status' => COMMENT_PUBLISHED)); - $query->condition(db_or() - ->condition('n.uid', $account->uid) - ->condition('c.uid', $account->uid) - ); } + else { + $query = db_select('tracker_node', 't')->extend('PagerDefault'); + } + + // This array acts as a placeholder for the data selected later + // while keeping the correct order. + $nodes = $query + ->addTag('node_access') + ->fields('t', array('nid', 'changed')) + ->condition('t.published', 1) + ->orderBy('t.changed', 'DESC') + ->limit(25) + ->execute() + ->fetchAllAssoc('nid'); - $result = $query->execute(); + if (!empty($nodes)) { + // Now, get the data and put into the placeholder array + $result = db_query("SELECT n.nid, n.title, n.type, n.changed, n.uid, u.name, l.comment_count FROM {node} n INNER JOIN {node_comment_statistics} l ON n.nid = l.nid INNER JOIN {users} u ON n.uid = u.uid WHERE n.nid IN (:nids)", array(':nids' => array_keys($nodes))); + foreach ($result as $node) { + $node->last_activity = $nodes[$node->nid]->changed; + $nodes[$node->nid] = $node; + } - $rows = array(); - foreach ($result as $node) { - // Determine the number of comments: - $comments = 0; - if ($node->comment_count) { - $comments = $node->comment_count; + // Finally display the data + $rows = array(); + foreach ($nodes as $node) { + // Determine the number of comments: + $comments = 0; + if ($node->comment_count) { + $comments = $node->comment_count; - if ($new = comment_num_new($node->nid)) { - $comments .= '
'; - $comments .= l(format_plural($new, '1 new', '@count new'), "node/$node->nid", array('query' => comment_new_page_count($node->comment_count, $new, $node), 'fragment' => 'new')); + if ($new = comment_num_new($node->nid)) { + $comments .= '
'; + $comments .= l(format_plural($new, '1 new', '@count new'), 'node/'. $node->nid, array('fragment' => 'new')); + } } - } - $rows[] = array( - check_plain(node_type_get_name($node->type)), - l($node->title, "node/$node->nid") . ' ' . theme('mark', node_mark($node->nid, $node->changed)), - theme('username', $node), - array('class' => array('replies'), 'data' => $comments), - t('!time ago', array('!time' => format_interval(REQUEST_TIME - $node->last_updated))) - ); + $rows[] = array( + check_plain(node_type_get_name($node->type)), + l($node->title, 'node/' . $node->nid) . ' ' . theme('mark', node_mark($node->nid, $node->changed)), + theme('username', $node), + array('class' => array('replies'), 'data' => $comments), + t('!time ago', array('!time' => format_interval(REQUEST_TIME - $node->last_activity))) + ); + } } - - if (!$rows) { + else { $rows[] = array(array('data' => t('No posts available.'), 'colspan' => '5')); } diff --git modules/tracker/tracker.test modules/tracker/tracker.test index 25f6f9c..cd77a51 100644 --- modules/tracker/tracker.test +++ modules/tracker/tracker.test @@ -137,4 +137,93 @@ class TrackerTest extends DrupalWebTestCase { $this->drupalGet('tracker'); $this->assertText('1 new', t('New comments are counted on the tracker listing pages.')); } + + /** + * Test that existing nodes are indexed by cron. + */ + function testTrackerCronIndexing() { + // Make node preview optional + variable_set('comment_preview_page', 0); + + $this->drupalLogin($this->user); + + // Create 3 nodes. + $edits = array(); + $nodes = array(); + for ($i = 1; $i <= 3; $i++) { + $edits[$i] = array( + 'comment' => 2, + 'title' => $this->randomName(), + ); + $nodes[$i] = $this->drupalCreateNode($edits[$i]); + } + + // Add a comment to the last node as other user. + $this->drupalLogin($this->other_user); + $comment = array( + 'subject' => $this->randomName(), + 'comment' => $this->randomName(20), + ); + $this->drupalPost('comment/reply/' . $nodes[3]->nid, $comment, t('Save')); + + // Start indexing backwards from node 3. + variable_set('tracker_index_nid', 3); + + // Clear the current tracker tables and rebuild them. + db_delete('tracker_node') + ->execute(); + db_delete('tracker_user') + ->execute(); + tracker_cron(); + + $this->drupalLogin($this->user); + + // Fetch the user's tracker. + $this->drupalGet('tracker/' . $this->user->uid); + + // Assert that all node titles are displayed. + foreach ($edits as $i => $edit) { + $this->assertText($edit['title'], t('Node @i is displayed on the tracker listing pages.', array('@i' => $i))); + } + $this->assertText('1 new', t('New comment is counted on the tracker listing pages.')); + $this->assertText('updated', t('Node is listed as updated')); + + // Fetch the site-wide tracker. + $this->drupalGet('tracker'); + + // Assert that all node titles are displayed. + foreach ($edits as $i => $edit) { + $this->assertText($edit['title'], t('Node @i is displayed on the tracker listing pages.', array('@i' => $i))); + } + $this->assertText('1 new', t('New comment is counted on the tracker listing pages.')); + } + + /** + * Test that publish/unpublish works at admin/content/node + */ + function testTrackerAdminUnpublish() { + + $admin_user = $this->drupalCreateUser(array('administer nodes')); + $this->drupalLogin($admin_user); + + $edit = array( + 'comment' => 2, + 'title' => $this->randomName(), + ); + $node = $this->drupalCreateNode($edit); + + // Assert that the node is displayed. + $this->drupalGet('tracker'); + $this->assertText($edit['title'], t('Node is displayed on the tracker listing pages.')); + + // Unpublish the node and ensure that it's no longer displayed. + $edit = array( + 'operation' => 'unpublish', + 'nodes[' . $node->nid . ']' => $node->nid, + ); + $this->drupalPost('admin/content', $edit, t('Update')); + + $this->drupalGet('tracker'); + $this->assertText(t('No posts available.'), t('Node is displayed on the tracker listing pages.')); + } }