Download & Extend

Performance: Slow query in ping.module ping_cron()

Project:Drupal core
Version:6.x-dev
Component:ping.module
Category:bug report
Priority:normal
Assigned:Unassigned
Status:closed (fixed)

Issue Summary

Discovered as a part of investigating using Drupal with millions of nodes:

The SQL query in ping_cron() does a full table scan of the {node} table

Comments

#1

Status:active» needs review
AttachmentSizeStatusTest resultOperations
336358-D6.patch796 bytesIgnored: Check issue status.NoneNone

#2

Status:needs review» reviewed & tested by the community

Tested, and looks good.

Ping module is not in core in D7, so this should go straight into D6.

#3

Status:reviewed & tested by the community» needs work

Is this better because changed has an index, but created does not? (I did not look that up). Also, can we have the comment capitalized and have a dot at the end?

#4

Status:needs work» reviewed & tested by the community

Both changed and created have indexes, but whatever versions of MySQL that I have tried do not make this optimization of splitting the OR clause into two separate queries to avoid the full table scan.

The order of the queries is just a heuristic to try to get the second query to be short-circuited out most of the time.

Patch file edited directly to capitalize comment and add a period

AttachmentSizeStatusTest resultOperations
336358-D6.patch797 bytesIgnored: Check issue status.NoneNone

#5

Status:reviewed & tested by the community» needs work

Why are we not using our %d placeholder in the query then?

#6

Status:needs work» needs review

@Gabor,

That was an oversight when I initially RTBC'd this. I have fixed that in this version.

AttachmentSizeStatusTest resultOperations
336358-remove-table-scan-from-ping_cron-D6.patch1.05 KBIgnored: Check issue status.NoneNone

#7

Status:needs review» reviewed & tested by the community

I did not originally use %d placeholders because the original code did not use %d placeholders.

Given that we're cleaning that up, I'd also suggest switching the double quoted strings to single quoted ones (single quotes being faster). I have edited the patch to do this and am attaching it here.

Otherwise this %d version of the patch looks good to me.

AttachmentSizeStatusTest resultOperations
336358-D6.patch1.05 KBIgnored: Check issue status.NoneNone

#8

Version:6.6» 6.x-dev
Status:reviewed & tested by the community» fixed

Thanks, committed to Drupal 6.

#9

Status:fixed» closed (fixed)

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

nobody click here