I was trying to figure out why file deletions were so slow on my site. After running SHOW PROCESSLIST a few times it became apparent that there were a lot of queries like UPDATE users SET picture='0' WHERE (picture = '98529')
that were taking multiple seconds. I traced the source of that back to user_file_delete() which tries to make sure there are no user records pointing at the file. Sadly there's no index on the picture column so this takes for ever.
I think there's two approaches the simplest it to just add the index. I've got a patch for that for 7.x. The other would be to try to look up the uid from the fid in {file_usage}. I'm not certain that would work and it seems a bit more complicated.
Comment | File | Size | Author |
---|---|---|---|
#16 | drupal-1348758-16-tests.patch | 2.07 KB | tim.plunkett |
#16 | drupal-1348758-16-combined.patch | 2.48 KB | tim.plunkett |
#8 | drupal-1348758-8.patch | 839 bytes | tim.plunkett |
#5 | user_1348758.patch | 1.12 KB | drewish |
#3 | user_1348758.patch | 825 bytes | drewish |
Comments
Comment #1
drewish CreditAttribution: drewish commentedComment #2
drewish CreditAttribution: drewish commentedHere's one for D8 but since I've got no idea where updates have gone to in D8 so it probably needs work.
Comment #3
drewish CreditAttribution: drewish commentedI guess the D8 doesn't work quite right... come on test bot!
Comment #4
moshe weitzman CreditAttribution: moshe weitzman commentedNice patch ... Needs an update function.
Comment #5
drewish CreditAttribution: drewish commentedWent ahead and rolled a patch with the update but I'm not totally sure it's necessary. Upgraded sites should get the index added in D7, new sites should have it added when the schema is defined. At least back in D6 we didn't updates from the last major version, not HEAD to HEAD updates...
Comment #6
tim.plunkettBy using !db_index_exists(), this update function can be in both D7 and D8 with no problem.
Comment #7
catchI've committed/pushed #4 to 8.x, while we can use db_index_exists() for index additions safely, there's no need for the 8.x update until we support head-to-head updates.
Moving to D7 and bumping as critical so this gets resolved before we have to re-introduce the update in 8.x though. Backport should be straightforward so tagging as novice.
Comment #8
tim.plunkettI think this is the right group.
Comment #9
marcingy CreditAttribution: marcingy commentedAssume this will come back green....
Comment #11
tim.plunkett#8: drupal-1348758-8.patch queued for re-testing.
Comment #12
tim.plunkettFYI: The failure in #10 was "Drupal installation failed" and a retest worked fine.
Comment #13
tim.plunkettNeeds tests.
Comment #14
marcingy CreditAttribution: marcingy commentedComment #15
marcingy CreditAttribution: marcingy commentedcross post
Comment #16
tim.plunkettOkay, here's an update test.
Comment #17
xjmThe upgrade path test looks great.
I also tested the patch manually:
update.php
.Comment #18
webchickWow, an upgrade path + tests + manual tests. It's like xmas in April! :D Awesome work, folks!
Committed and pushed to 7.x. Thanks! :)
Comment #19.0
(not verified) CreditAttribution: commentedClosing my a tag.