Performance: SELECT MAX(comment_count) FROM node_comment_statistics does full table scan
Wesley Tanaka - November 19, 2008 - 18:38
| Project: | Drupal |
| Version: | 7.x-dev |
| Component: | search.module |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs work |
| Issue tags: | Performance |
Description
An issue when running Drupal with millions of nodes:
This query in node_update_index() does a full table scan
mysql> EXPLAIN SELECT MAX(comment_count) FROM d_node_comment_statistics \G
*************************** 1. row ***************************
id: 1
select_type: SIMPLE
table: d_node_comment_statistics
type: ALL
possible_keys: NULL
key: NULL
key_len: NULL
ref: NULL
rows: 2435440
Extra:
#1
The addition of this index will optimize the query away:
ALTER TABLE node_comment_statistics ADD INDEX (comment_count);#2
Patch attached adds a hook_update_70XX() implementation which adds this index.
#3
Can we see the explain after the index addition Welsey?
#4
I'm not Wesley, but...
explain MAX(comment_count)FROM d_node_comment_statistics \G
*************************** 1. row ***************************
id: 1
select_type: SIMPLE
table: NULL
type: NULL
possible_keys: NULL
key: NULL
key_len: NULL
ref: NULL
rows: NULL
Extra: Select tables optimized away
#5
Just tagging
#6
#7
brianV, was that EXPLAIN on a table with a significant amount of nodes / comments? To me it looks like on a brand new installation, which doesnt mean much?
#8
Fireangel,
That was taken from Wesley Tanaka's page on the subject. I am providing his data, which he says is based upon a node database of 2.4million nodes.
See http://wtanaka.com/drupal/million-nodes-6 - there is an entry for this bug.
#9
Excellent! No further objections / comments here, and it's already RTBC
#10
Well, but shouldn't we update the schema definition in modules/comment/comment.install too? The index is not being created for new installs?
#11
Oups.
#12
Big time oops. Rerolled with the proper addition to comment_schema().
#13
Thanks Brian, but please remove the useless "&".
#14
Wow - I am off my game this morning...
#15
#16
Committed to CVS HEAD. Thanks!
#17
Let's backport this.
#18
Why is the update function in system.install?
#19
Looks like I am more off my game than I thought today.
There is absolutely no good reason why I put this in system.install instead of comment.install.
What's the best way to correct this? Move the hook_update_N to the correct file in another patch?