I was playing with mysql-administrator and fine-tuning my DB and looking at the connections I often see SELECT * FROM files WHERE nid = XXX(upload.module, function upload_load is selecting by a non-index column). After ALTER TABLE files ADD KEY (nid); I can't see it any more (we have 37454 files for 19143 nodes). I think this is an easy fix for 4.6.

In 4.7 there are revisions and maybe ALTER TABLE file_revisions ADD KEY (vid); would do the trick, but is PRIMARY KEY (fid,vid) enough in this case?

CommentFileSizeAuthor
#6 fix_upload_db_keys_6.txt1.48 KBpwolanin
#4 fix_upload_db_keys_1.txt1.5 KBpwolanin

Comments

magico’s picture

magico commented
Version: 4.6.8 » 4.6.9

Is this being address in the current HEAD mysql optimizations?

magico’s picture

magico commented
Version: 4.6.9 » x.y.z

Verified (in HEAD) that the table is the same, and because I've saw some code using the where clause "nid = %d" I think we should take a better look into this optimization.

mr700’s picture

mr700 commented
Version: x.y.z » 4.7.4

I would say it's a verry 'easy fix'. Some rough nubers from my tests today. I was loading series of 30 nodes nodes (~ 36000 total), most with 2, some with 1 attached file (60010 images total). Without indexes it took 5-7 seconds. After ALTER TABLE files add KEY `nid` (`nid`); ALTER TABLE file_revisions add KEY `vid` (`vid`); - always for less than a second. For large sites this can be critical.

pwolanin’s picture

pwolanin commented
Version: 4.7.4 » 5.x-dev
Status: Active » Needs review
StatusFileSize
new fix_upload_db_keys_1.txt1.5 KB

The current HEAD (5.x) version of these tables define vid as a key for file_revisions, but does not include nid as an index for the files table.

So, a quick grep turns up these 2 problematic queries in 5.x:

modules/upload/upload.module:600: return db_result(db_query('SELECT SUM(filesize) FROM {files} f INNER JOIN {node} n ON f.nid = n.nid WHERE n.uid = %d', $uid));
modules/upload/upload.module:726: $result = db_query('SELECT * FROM {files} WHERE nid = %d', $node->nid);

patch for 5.x attached which adds this additional index.

drumm’s picture

drumm
he/him
Location NY, US
commented
Status: Needs review » Needs work

Everywhere else, vid is used rather than nid.

modules/upload/upload.module:600: return db_result(db_query('SELECT SUM(filesize) FROM {files} f INNER JOIN {node} n ON f.nid = n.nid WHERE n.uid = %d', $uid));

This looks like it could easily use vid.

modules/upload/upload.module:726: $result = db_query('SELECT * FROM {files} WHERE nid = %d', $node->nid);

This might want to stay the same. It is deleting so, I'm not too worried about it.

pwolanin’s picture

pwolanin commented
Status: Needs work » Needs review
StatusFileSize
new fix_upload_db_keys_6.txt1.48 KB

The table {files} does not have vid as a column...

+----------+------------------+------+-----+---------+-------+
| Field    | Type             | Null | Key | Default | Extra |
+----------+------------------+------+-----+---------+-------+
| fid      | int(10) unsigned |      | PRI | 0       |       |
| nid      | int(10) unsigned |      |     | 0       |       |
| filename | varchar(255)     |      |     |         |       |
| filepath | varchar(255)     |      |     |         |       |
| filemime | varchar(255)     |      |     |         |       |
| filesize | int(10) unsigned |      |     | 0       |       |
+----------+------------------+------+-----+---------+-------+

patch still applies with offset- patch against current HEAD attached.

drumm’s picture

drumm
he/him
Location NY, US
commented
Status: Needs review » Fixed

Committed to HEAD.

Anonymous’s picture

(not verified) commented
Status: Fixed » Closed (fixed)