Opened 7 months ago

Last modified 4 months ago

#22301 new defect (bug)

Performance problem with Recent Comments widget

Reported by: pento Owned by:
Priority: normal Milestone: Awaiting Review
Component: Performance Version: 2.8
Severity: normal Keywords: dev-feedback
Cc: scribu, bronson@…, aaroncampbell

Description

When a comment is posted (or the status of a comment changes), the widget_recent_commentscache item is invalidated, which the Recent Comments widget uses to populate the widget content. On the next widget display, it will call get_comments() to repopulate the cache.

The problem occurs when you have a very large number of comments, the MySQL query will use the (comment_approved, comment_date_gmt) index, but if MySQL has to scan too many rows in an index, it'll switch to table scan instead. As the comment_approved column is mostly the same value, this will almost always happen. This is compounded by the query occurring on every page load until the cache is re-populated - if the query takes 60 seconds to run, there could potentially be hundreds of instances of the same query running.

So, we need a solution that either hides or eliminates how slow this query can be, and only runs it (at most) once per new comment.

After discussing this with @matt, we have a couple of ideas:

  1. Move this query to a wp_schedule_single_event() call, which has the bonus of ensuring only one is scheduled at any given time. The downside is that it may cause the cache to be outdated on a low traffic site.
  1. Keep a queue of recent comments in cache, and push a new one onto the queue when posted. This avoids the query entirely, but there would be a race condition if two comments were posted at nearly the same time - one of them could be left out of the queue entirely.

Change History (5)

  • Cc scribu added
  • Version changed from trunk to 2.8

It isn't likely that a low-traffic site is using a persistent cache, so the concern of (1) is minor. In the case of a persistent cache, we could schedule an event, yes, but we could also try to store a "lock" in the persistent cache store. Whichever process successfully sets the lock gets to run the query, and until the query is complete, the old cache gets used. Just an idea.

  • Cc bronson@… added
  • Cc aaroncampbell added
Note: See TracTickets for help on using tickets.