I noticed the message table is not indexed by anything other than mid (foreign key on uid and message_type, but no indexes).

Because we're often going to want to fetch messages for a given user, or fetch messages by age (which will not necessarily correlate with mid) it makes sense to index the message table by uid and by timestamp as well.

This falls in line with Drupal best practices, as for instance, the node table indexes on uid and node created time (actually, the node table indexes on every column). Makes sense to index by message_type as well.

I would say this should be a major priority from a performance standpoint.

Files: 
CommentFileSizeAuthor
#7 Screenshot 12:11:12 9:45 PM.png11.55 KBAmitaibu
#4 message_add_timestamp_index.patch653 bytesmsonnabaum
PASSED: [[SimpleTest]]: [MySQL] 69 pass(es).
[ View ]

Comments

Patch is welcome, and a benchmark even more ;)

Sure, I'll put this on my todo.

I agree that benchmarks would be nice, though considering that this is pretty basic sql query stuff, I think something would have to be seriously broken for an index on uid not to improve fetching rows by uid...

I agree. Well, then at least a patch ;)

StatusFileSize
new653 bytes
PASSED: [[SimpleTest]]: [MySQL] 69 pass(es).
[ View ]

In the interest of only adding indexes that we know we're running into issues with, here's a patch that adds one to the timestamp column.

I ran into this in Commons, where we have a query like this:

  SELECT message.mid AS mid, message.timestamp AS message_timestamp
  FROM message message
  WHERE (((message.type IN ('commons_activity_streams_comment_created', 'commons_activity_streams_node_created', 'commons_follow_user_user_followed', 'commons_wikis_wiki_updated', 'commons_q_a_question_asked')) ))
  ORDER BY message_timestamp DESC
  LIMIT 5 OFFSET 0

Before this patch:

+------+-------------+---------+------+---------------+------+---------+------+--------+-----------------------------+
| id   | select_type | table   | type | possible_keys | key  | key_len | ref  | rows   | Extra                       |
+------+-------------+---------+------+---------------+------+---------+------+--------+-----------------------------+
|    1 | SIMPLE      | message | ALL  | NULL          | NULL | NULL    | NULL | 168463 | Using where; Using filesort |
+------+-------------+---------+------+---------------+------+---------+------+--------+-----------------------------+
Filesort
+- Filter with WHERE
   +- Table scan
      rows           168463
      +- Table
         table          message

After:

+------+-------------+---------+-------+---------------+-----------+---------+------+------+-------------+
| id   | select_type | table   | type  | possible_keys | key       | key_len | ref  | rows | Extra       |
+------+-------------+---------+-------+---------------+-----------+---------+------+------+-------------+
|    1 | SIMPLE      | message | index | NULL          | timestamp | 4       | NULL |    5 | Using where |
+------+-------------+---------+-------+---------------+-----------+---------+------+------+-------------+
Filter with WHERE
+- Bookmark lookup
   +- Table
   |  table          message
   +- Index scan
      key            message->timestamp
      key_len        4
      rows           5

Query on my local machine goes from about 250ms to 0.4ms.

Status:Active» Needs review

Title:message table should add more indexesAdd an index for message timestamp
Category:support» task
Issue tags:+Performance, +commonslove

Re-titling and tagging.

StatusFileSize
new11.55 KB

After applying patch and using a query that has WHERE on the Message-type (a typical use case), mysql still doesn't use the index key

EXPLAIN SELECT message.mid AS mid, message.timestamp AS message_timestamp FROM message message WHERE message.type = "example_create_node" ORDER BY message_timestamp DESC LIMIT 5 OFFSET 0;
45 PM.png

Should we add an index for that as-well?

Interesting. That query uses the timestamp index for me, but that's probably just a difference between envs (I'm running MariaDB 5.5 locally).

And yeah, we probably should add an index to type as well.

Status:Needs review» Fixed

Add index to "type" as-well and committed, thanks.

Automatically closed -- issue fixed for 2 weeks with no activity.