Skip to content

Commit

Permalink
Replaced queries in Topic.subs.php with ones that scale better to mas…
Browse files Browse the repository at this point in the history 
…sive threads. Fixes #3341

Most topic index (board view) queries were already appropriately optimized.

The PM queries are still a mess, but this is less critical, and best addressed with personal conversations in 2.0.

Per #3230 I also removed the temporary table creation from unread replies.
  • Loading branch information
@Vekseid
Vekseid committed Sep 5, 2019
1 parent fcbd1b0 commit 9eff7c2
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 218 deletions.
20 changes: 1 addition & 19 deletions sources/controllers/Unread.controller.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -177,20 +177,7 @@ public function action_unread()
$this->_grabber->setEarliestMsg($context['showing_all_topics'] ? earliest_msg() : 0);

// @todo Add modified_time in for log_time check?
// Let's copy things out of the log_topics table, to reduce searching.
if ($modSettings['totalMessages'] > 100000 && $context['showing_all_topics'])
{
$this->_grabber->createTempTable();
}

// All unread replies with temp table
if ($context['showing_all_topics'] && $this->_grabber->hasTempTable())
{
$this->_num_topics = $this->_grabber->numUnreads(false);
$type = 'message';
}
// New posts with or without temp table
elseif ($this->_is_topics)
if ($this->_is_topics)
{
$this->_num_topics = $this->_grabber->numUnreads(empty($_SESSION['first_login']), $_SESSION['id_msg_last_visit']);
$type = 'topics';
Expand Down Expand Up @@ -243,11 +230,6 @@ public function action_unreadreplies()

$this->_grabber->setAction(Unread::UNREADREPLIES);

if ($modSettings['totalMessages'] > 100000)
{
$this->_grabber->createTempTable();
}

$this->_num_topics = $this->_grabber->numUnreads();

if ($this->_num_topics == 0)
Expand Down
72 changes: 40 additions & 32 deletions sources/subs/Topic.subs.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1598,29 +1598,30 @@ function selectMessages($topic, $start, $items_per_page, $messages = array(), $o
$db = database();

// Get the messages and stick them into an array.
$request = $db->query('', '
SELECT m.subject, COALESCE(mem.real_name, m.poster_name) AS real_name, m.poster_time, m.body, m.id_msg, m.smileys_enabled, m.id_member
FROM {db_prefix}messages AS m
LEFT JOIN {db_prefix}members AS mem ON (mem.id_member = m.id_member)
$request = $db->query('', '
SELECT m.subject, COALESCE(mem.real_name, m.poster_name) AS real_name, m.poster_time, m.body, m.id_msg, m.smileys_enabled, m.id_member FROM
(SELECT m.id_msg FROM {db_prefix}messages AS m
WHERE m.id_topic = {int:current_topic}' . (empty($messages['before']) ? '' : '
AND m.id_msg < {int:msg_before}') . (empty($messages['after']) ? '' : '
AND m.id_msg > {int:msg_after}') . (empty($messages['excluded']) ? '' : '
AND m.id_msg NOT IN ({array_int:no_split_msgs})') . (empty($messages['included']) ? '' : '
AND m.id_msg IN ({array_int:split_msgs})') . (!$only_approved ? '' : '
AND approved = {int:is_approved}') . '
ORDER BY m.id_msg DESC
LIMIT {int:start}, {int:messages_per_page}',
array(
'current_topic' => $topic,
'no_split_msgs' => !empty($messages['excluded']) ? $messages['excluded'] : array(),
'split_msgs' => !empty($messages['included']) ? $messages['included'] : array(),
'is_approved' => 1,
'start' => $start,
'messages_per_page' => $items_per_page,
'msg_before' => !empty($messages['before']) ? (int) $messages['before'] : 0,
'msg_after' => !empty($messages['after']) ? (int) $messages['after'] : 0,
)
);
LIMIT {int:start}, {int:messages_per_page}) AS o JOIN {db_prefix}messages as m ON o.id_msg=m.id_msg LEFT JOIN {db_prefix}members AS mem ON (mem.id_member = m.id_member)
ORDER BY m.id_msg DESC
',
array(
'current_topic' => $topic,
'no_split_msgs' => !empty($messages['excluded']) ? $messages['excluded'] : array(),
'split_msgs' => !empty($messages['included']) ? $messages['included'] : array(),
'is_approved' => 1,
'start' => $start,
'messages_per_page' => $items_per_page,
'msg_before' => !empty($messages['before']) ? (int) $messages['before'] : 0,
'msg_after' => !empty($messages['after']) ? (int) $messages['after'] : 0,
)
);

$messages = array();
$parser = \BBC\ParserWrapper::instance();
Expand Down Expand Up @@ -2175,21 +2176,25 @@ function getTopicsPostsAndPoster($topic, $limit, $sort)
'all_posters' => array(),
);

$request = $db->query('display_get_post_poster', '
SELECT id_msg, id_member, approved
FROM {db_prefix}messages
// When evaluating potentially huge offsets, grab the ids only, first.
// The performance impact is still significant going from three columns to one.
$request = $db->query('display_get_post_poster', '
SELECT m.id_msg, m.id_member, m.approved
FROM (SELECT id_msg FROM {db_prefix}messages
WHERE id_topic = {int:current_topic}' . (!$modSettings['postmod_active'] || allowedTo('approve_posts') ? '' : '
GROUP BY id_msg
HAVING (approved = {int:is_approved}' . ($user_info['is_guest'] ? '' : ' OR id_member = {int:current_member}') . ')') . '
ORDER BY id_msg ' . ($sort ? '' : 'DESC') . ($limit['messages_per_page'] == -1 ? '' : '
LIMIT ' . $limit['start'] . ', ' . $limit['offset']),
array(
'current_member' => $user_info['id'],
'current_topic' => $topic,
'is_approved' => 1,
'blank_id_member' => 0,
)
);
LIMIT ' . $limit['start'] . ', ' . $limit['offset']) . ') AS o JOIN {db_prefix}messages as m ON o.id_msg=m.id_msg
ORDER BY m.id_msg ' . ($sort ? '' : 'DESC'),
array(
'current_member' => $user_info['id'],
'current_topic' => $topic,
'is_approved' => 1,
'blank_id_member' => 0,
)
);

while ($row = $db->fetch_assoc($request))
{
if (!empty($row['id_member']))
Expand Down Expand Up @@ -2924,15 +2929,18 @@ function mergeableTopics($id_board, $id_topic, $approved, $offset)

// Get some topics to merge it with.
$request = $db->query('', '
SELECT t.id_topic, m.subject, m.id_member, COALESCE(mem.real_name, m.poster_name) AS poster_name
FROM {db_prefix}topics AS t
INNER JOIN {db_prefix}messages AS m ON (m.id_msg = t.id_first_msg)
LEFT JOIN {db_prefix}members AS mem ON (mem.id_member = m.id_member)
SELECT t.id_topic, m.subject, m.id_member, COALESCE(mem.real_name, m.poster_name) AS poster_name FROM
(SELECT t.id_topic FROM {db_prefix}topics AS t
WHERE t.id_board = {int:id_board}
AND t.id_topic != {int:id_topic}' . (empty($approved) ? '
AND t.approved = {int:is_approved}' : '') . '
ORDER BY t.is_sticky DESC, t.id_last_msg DESC
LIMIT {int:offset}, {int:limit}',
LIMIT {int:offset}, {int:limit}) AS o
INNER JOIN {db_prefix}topics AS t ON (o.id_topic = t.id_topic)
INNER JOIN {db_prefix}messages AS m ON (m.id_msg = t.id_first_msg)
LEFT JOIN {db_prefix}members AS mem ON (mem.id_member = m.id_member)
ORDER BY t.is_sticky DESC, t.id_last_msg DESC
',
array(
'id_board' => $id_board,
'id_topic' => $id_topic,
Expand Down
180 changes: 13 additions & 167 deletions sources/subs/Unread.class.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@ class Unread
const UNREAD = 0;
const UNREADREPLIES = 1;

/** @var bool */
private $_have_temp_table = false;
/** @var bool */
private $_ascending = false;
/** @var string */
Expand Down Expand Up @@ -130,22 +128,6 @@ public function isSortAsc()
return $this->_ascending;
}

/**
* Tries to create a temporary table according to the type of action the
* class is performing
*/
public function createTempTable()
{
if ($this->_action === self::UNREAD)
$this->_recent_log_topics_unread_tempTable();
else
{
$board = !empty($this->_query_parameters['boards'][0]) && count($this->_query_parameters['boards']) === 1 ? $this->_query_parameters['boards'][0] : 0;

$this->_unreadreplies_tempTable($board);
}
}

/**
* Sets if the data returned by the class will include a shorted version
* of the body of the last message.
Expand All @@ -162,14 +144,6 @@ public function bodyPreview($chars)
$this->_preview_bodies = (int) $chars;
}

/**
* If a temporary table has been created
*/
public function hasTempTable()
{
return $this->_have_temp_table;
}

/**
* Counts the number of unread topics or messages
*
Expand Down Expand Up @@ -384,26 +358,7 @@ private function _countRecentTopics($is_first_login, $id_msg_last_visit = 0)
*/
private function _getUnreadReplies($start, $limit, $include_avatars = false)
{
if (!empty($this->_have_temp_table))
{
$request = $this->_db->query('', '
SELECT t.id_topic
FROM {db_prefix}topics_posted_in AS t
LEFT JOIN {db_prefix}log_topics_posted_in AS lt ON (lt.id_topic = t.id_topic)
WHERE t.id_board IN ({array_int:boards})
AND COALESCE(lt.id_msg, t.id_msg) < t.id_last_msg
ORDER BY {raw:order}
LIMIT {int:offset}, {int:limit}',
array_merge($this->_query_parameters, array(
'order' => (in_array($this->_sort_query, array('t.id_last_msg', 't.id_topic')) ? $this->_sort_query : 't.sort_key') . ($this->_ascending ? '' : ' DESC'),
'offset' => $start,
'limit' => $limit,
))
);
}
else
{
$request = $this->_db->query('unread_replies', '
$request = $this->_db->query('unread_replies', '
SELECT DISTINCT t.id_topic
FROM {db_prefix}topics AS t
INNER JOIN {db_prefix}messages AS m ON (m.id_topic = t.id_topic AND m.id_member = {int:current_member})' . (strpos($this->_sort_query, 'ms.') === false ? '' : '
Expand All @@ -414,20 +369,20 @@ private function _getUnreadReplies($start, $limit, $include_avatars = false)
WHERE t.id_board IN ({array_int:boards})
AND t.id_last_msg >= {int:min_message}
AND COALESCE(lt.id_msg, lmr.id_msg, 0) < t.id_last_msg' .
($this->_post_mod ? ' AND t.approved = {int:is_approved}' : '') .
($this->_unwatch ? ' AND COALESCE(lt.unwatched, 0) != 1' : '') . '
($this->_post_mod ? ' AND t.approved = {int:is_approved}' : '') .
($this->_unwatch ? ' AND COALESCE(lt.unwatched, 0) != 1' : '') . '
ORDER BY {raw:order}
LIMIT {int:offset}, {int:limit}',
array_merge($this->_query_parameters, array(
'current_member' => $this->_user_id,
'min_message' => $this->_min_message,
'is_approved' => 1,
'order' => $this->_sort_query . ($this->_ascending ? '' : ' DESC'),
'offset' => $start,
'limit' => $limit,
))
);
}
array_merge($this->_query_parameters, array(
'current_member' => $this->_user_id,
'min_message' => $this->_min_message,
'is_approved' => 1,
'order' => $this->_sort_query . ($this->_ascending ? '' : ' DESC'),
'offset' => $start,
'limit' => $limit,
))
);

$topics = array();
while ($row = $this->_db->fetch_assoc($request))
$topics[] = $row['id_topic'];
Expand Down Expand Up @@ -507,113 +462,4 @@ private function _getUnreadReplies($start, $limit, $include_avatars = false)

return Topic_Util::prepareContext($return, true, ((int) $this->_preview_bodies) + 128);
}

/**
* Handle creation of temporary tables to track unread replies.
* The main benefit of this temporary table is not that it's faster;
* it's that it avoids locks later.
*
* @param int $board_id - id of a board if looking at the unread of a single
* board, 0 if looking at the unread of the entire forum
*/
private function _unreadreplies_tempTable($board_id)
{
$this->_db->query('', '
DROP TABLE IF EXISTS {db_prefix}topics_posted_in',
array(
)
);

$this->_db->query('', '
DROP TABLE IF EXISTS {db_prefix}log_topics_posted_in',
array(
)
);

$sortKey_joins = array(
'ms.subject' => '
INNER JOIN {db_prefix}messages AS ms ON (ms.id_msg = t.id_first_msg)',
'COALESCE(mems.real_name, ms.poster_name)' => '
INNER JOIN {db_prefix}messages AS ms ON (ms.id_msg = t.id_first_msg)
LEFT JOIN {db_prefix}members AS mems ON (mems.id_member = ms.id_member)',
);

$this->_db->skip_next_error();
$this->_have_temp_table = $this->_db->query('', '
CREATE TEMPORARY TABLE {db_prefix}topics_posted_in (
id_topic mediumint(8) unsigned NOT NULL default {string:string_zero},
id_board smallint(5) unsigned NOT NULL default {string:string_zero},
id_last_msg int(10) unsigned NOT NULL default {string:string_zero},
id_msg int(10) unsigned NOT NULL default {string:string_zero},
PRIMARY KEY (id_topic)
)
SELECT t.id_topic, t.id_board, t.id_last_msg, COALESCE(lmr.id_msg, 0) AS id_msg' . (!in_array($this->_sort_query, array('t.id_last_msg', 't.id_topic')) ? ', ' . $this->_sort_query . ' AS sort_key' : '') . '
FROM {db_prefix}messages AS m
INNER JOIN {db_prefix}topics AS t ON (t.id_topic = m.id_topic)' . ($this->_unwatch ? '
LEFT JOIN {db_prefix}log_topics AS lt ON (t.id_topic = lt.id_topic AND lt.id_member = {int:current_member})' : '') . '
LEFT JOIN {db_prefix}log_mark_read AS lmr ON (lmr.id_board = t.id_board AND lmr.id_member = {int:current_member})' . (isset($sortKey_joins[$this->_sort_query]) ? $sortKey_joins[$this->_sort_query] : '') . '
WHERE m.id_member = {int:current_member}' . (!empty($board_id) ? '
AND t.id_board = {int:current_board}' : '') . ($this->_post_mod ? '
AND t.approved = {int:is_approved}' : '') . ($this->_unwatch ? '
AND COALESCE(lt.unwatched, 0) != 1' : '') . '
GROUP BY m.id_topic',
array(
'current_board' => $board_id,
'current_member' => $this->_user_id,
'is_approved' => 1,
'string_zero' => '0',
)
) !== false;

// If that worked, create a sample of the log_topics table too.
if ($this->_have_temp_table)
{
$this->_db->skip_next_error();
$this->_have_temp_table = $this->_db->query('', '
CREATE TEMPORARY TABLE {db_prefix}log_topics_posted_in (
PRIMARY KEY (id_topic)
)
SELECT lt.id_topic, lt.id_msg
FROM {db_prefix}log_topics AS lt
INNER JOIN {db_prefix}topics_posted_in AS pi ON (pi.id_topic = lt.id_topic)
WHERE lt.id_member = {int:current_member}',
array(
'current_member' => $this->_user_id,
)
) !== false;
}
}

/**
* Creates a temporary table for logging of unread topics
*/
private function _recent_log_topics_unread_tempTable()
{
$this->_db->query('', '
DROP TABLE IF EXISTS {db_prefix}log_topics_unread',
array(
)
);

$this->_db->skip_next_error();
// Let's copy things out of the log_topics table, to reduce searching.
$this->_have_temp_table = $this->_db->query('', '
CREATE TEMPORARY TABLE {db_prefix}log_topics_unread (
PRIMARY KEY (id_topic)
)
SELECT lt.id_topic, lt.id_msg, lt.unwatched
FROM {db_prefix}topics AS t
INNER JOIN {db_prefix}log_topics AS lt ON (lt.id_topic = t.id_topic)
WHERE lt.id_member = {int:current_member}
AND t.id_board IN ({array_int:boards})' . (empty($this->_earliest_msg) ? '' : '
AND t.id_last_msg > {int:earliest_msg}') . ($this->_post_mod ? '
AND t.approved = {int:is_approved}' : '') . ($this->_unwatch ? '
AND lt.unwatched != 1' : ''),
array_merge($this->_query_parameters, array(
'current_member' => $this->_user_id,
'earliest_msg' => $this->_earliest_msg,
'is_approved' => 1,
))
) !== false;
}
}

0 comments on commit 9eff7c2

Please sign in to comment.