Doing bulk deletion of users in a cron job, I was only getting 290-300 users deleted per run (240 seconds). It turned out that 95% of the time was being spent in comment_user(), which updates {comments} and {node_comment_statistics} on their respective uid columns. This is in an environment with no comments, but 138,000 nodes (thus {node_comment_statistics} has 138,000 rows).

The attached patch indexes the respective uid columns (I know {comments}, being empty, didn't contribute to my issue, but adding that as well seems like a no-brainer). After applying the patch and running the update process, my next cron batch deleted 3900 users.

Files: 
CommentFileSizeAuthor
#21 comment_index_update_update.patch859 bytescatch
PASSED: [[SimpleTest]]: [MySQL] 37,558 pass(es).
[ View ]
#19 indexes.diff1.18 KBmoshe weitzman
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch indexes.diff. Unable to apply patch. See the log in the details link for more information.
[ View ]
#16 comment_index_D7.patch859 bytescatch
#16 comment_index_289504_D6.patch1.37 KBcatch
#14 comment_index_289504_D6.patch1.37 KBcatch
#14 comment_index_7.patch859 bytescatch
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch comment_index_7.patch.
[ View ]
#11 comment_index_289504.patch1.54 KBmikeryan
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch comment_index_289504.patch.
[ View ]
#4 uid_index.diff1.9 KBmoshe weitzman
Passed: 11824 passes, 0 fails, 0 exceptions
[ View ]
#1 comment.install.patch1.19 KBmikeryan
Failed: Failed to apply patch.
[ View ]
comment.install.patch661 bytesmikeryan
Failed: Failed to apply patch.
[ View ]

Comments

StatusFileSize
new1.19 KB
Failed: Failed to apply patch.
[ View ]

Always forget to add my changes into the schema hook as well...

no comments? bump! any reason not to do this?

Is this applicable to D7?

Version:6.3» 7.x-dev
Assigned:Unassigned» moshe weitzman
Issue tags:+Performance
StatusFileSize
new1.9 KB
Passed: 11824 passes, 0 fails, 0 exceptions
[ View ]

Here is a D7 patch. In addition to these 2 indexes, I added foreign keys to user.uid and moved the misplaced comment_update_7005() back into the updates defgroup where it belongs. no changes to comment_update_7005()

subscribing, looks rtbc but could do with EXPLAIN output.

Here is an EXPLAIN for the delete query in comment_user_cancel() before and then after this patch.

1 SIMPLE c ALL NULL NULL NULL NULL 1 Using where
1 SIMPLE c ref comment_uid comment_uid 4 const 1 Using index

Status:Needs review» Reviewed & tested by the community

Status:Reviewed & tested by the community» Fixed

Committed to HEAD! Thanks!

Version:7.x-dev» 6.x-dev
Status:Fixed» Reviewed & tested by the community

RTBC for D6. see patch in #1

Status:Reviewed & tested by the community» Needs work

The comment update functions should have a clear indication of when they were added. Ie. before Drupal 6.0, inbetween Drupal 6.0 to 7.0. etc. See http://drupal.org/node/278592#comment-2038486

Status:Needs work» Needs review
StatusFileSize
new1.54 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch comment_index_289504.patch.
[ View ]

Rerolled with @defgroup updates-6.x-extra.

Just caught user deletions scanning 21 million rows in node_comment_statistics. This no longer applies, re-roll forthcoming.

Status:Needs review» Needs work

The last submitted patch, comment_index_289504.patch, failed testing.

Priority:Normal» Major
Status:Needs work» Needs review
Issue tags:+Needs forward port to D7
StatusFileSize
new859 bytes
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch comment_index_7.patch.
[ View ]
new1.37 KB

Here's the re-roll. The index in D7 is called 'comment_uid', updated the patch to reflect this.

Also adding a D7 patch to ensure that upgrade does not attempt to add the index twice.

Upgrading to major, spammers get deleted a lot.

Status:Needs review» Needs work

The last submitted patch, comment_index_7.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new1.37 KB
new859 bytes

la la la.

Status:Needs review» Reviewed & tested by the community

Looks like a good backport to me.

Status:Reviewed & tested by the community» Needs work

Did not apply anymore due to #336483: Performance: SELECT MAX(comment_count) FROM node_comment_statistics does full table scan changing indexes also in node comment stats. Needs a reroll.

Status:Needs work» Reviewed & tested by the community
StatusFileSize
new1.18 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch indexes.diff. Unable to apply patch. See the log in the details link for more information.
[ View ]

Rerolled and tested a new install.

Version:6.x-dev» 7.x-dev
Status:Reviewed & tested by the community» Patch (to be ported)

Committed to Drupal 6. Needs updates in Drupal 7 to fix update path.

Status:Patch (to be ported)» Needs review
Issue tags:-Needs forward port to D7+D7 upgrade path
StatusFileSize
new859 bytes
PASSED: [[SimpleTest]]: [MySQL] 37,558 pass(es).
[ View ]

Re-rolled the D7 patch.

Status:Needs review» Fixed

/me violates the cardinal rule of committing patches from needs review :\

However, it seems we need this to avoid causing problems in the upgrade path from the latest version of D6 to D7. The patch is simply wrapping a part of the upgrade path in in "if exists" check so "should" be safe (famous last words...)

Committed and pushed to 7.x. Thanks!

Status:Fixed» Closed (fixed)
Issue tags:-Performance, -D7 upgrade path

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