WordPress.org

Make WordPress Core

Opened 19 months ago

Closed 19 months ago

Last modified 19 months ago

#22024 closed defect (bug) (fixed)

The query cache in WP_Comment_Query:query() is not invalidated when comments are added

Reported by: ryan Owned by: ryan
Milestone: 3.5 Priority: normal
Severity: normal Version: 3.4.2
Component: Comments Keywords:
Focuses: Cc:

Description

last_changed is updated in clean_comment_cache(). clean_comment_cache() is not called from wp_insert_comment().

Tests_Comment_Query::test_get_comments_for_post() triggers the problem.

Attachments (2)

22024.diff (363 bytes) - added by ryan 19 months ago.
22024.2.diff (1.2 KB) - added by ryan 19 months ago.
Use wp_cache_incr()

Download all attachments as: .zip

Change History (14)

ryan19 months ago

comment:1 ryan19 months ago

last_changed could be changed to a wp_cache_incr() counter.

comment:2 ryan19 months ago

Previously: #14713

ryan19 months ago

Use wp_cache_incr()

comment:3 ryan19 months ago

  • Owner set to ryan
  • Resolution set to fixed
  • Status changed from new to closed

In [22080]:

Invalidate the WP_Comment_Query:query() cache when comments are added. Switch last_changed to a counter incremented via wp_cache_incr().

fixes #22024

comment:5 Ipstenu19 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

On one site, this change is causing the following error when anyone leaves a comment:

Fatal error: Call to undefined function wp_cache_incr() in /home/ipstenu/public_html/wp-includes/comment.php on line 1293

I also can reproduce the error when trying to bulk-move comments.

Fatal error: Call to undefined function wp_cache_incr() in /home/ipstenu/public_html/wp-includes/comment.php on line 1967

I svn'd up, but I cannot reproduce this on more than one site (multisite). All sites on the network, regardless of plugins, have that error.

comment:6 nacin19 months ago

You are using an object-cache.php drop-in that doesn't support wp_cache_incr() and wp_cache_decr(), added in 3.3.

Sadly, we'll probably need to avoid these functions in core for the time being, or begin to offer fallbacks (for when a drop-in doesn't add them) that in this case would call get() then set().

comment:7 follow-up: ocean9019 months ago

In 3.5 we added wp_cache_switch_to_blog() and use it, so offering fallbacks should be the right way.

comment:8 in reply to: ↑ 7 ; follow-up: nacin19 months ago

Replying to ocean90:

In 3.5 we added wp_cache_switch_to_blog() and use it, so offering fallbacks should be the right way.

Well, two different things. That's a limited-use method that does a function_exists() check, and if it doesn't exist, we do a decent amount of code. incr() and decr() are "nice-to-haves" that at worst can be replaced with a get() and set(). I'll let ryan determine what he wants to do here.

comment:9 follow-up: ryan19 months ago

First, which object cache backend is being used? It should be updated.

I guess we can add some function_exists action.

comment:10 ryan19 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In [22110]:

If wp_cache_incr() is not available fallback to get()/set(). fixes #22024

comment:11 in reply to: ↑ 8 ocean9019 months ago

Replying to nacin:

Well, two different things. That's a limited-use method that does a function_exists() check, and if it doesn't exist, we do a decent amount of code. incr() and decr() are "nice-to-haves" that at worst can be replaced with a get() and set(). I'll let ryan determine what he wants to do here.

Yeah, you are right. It was too late yesterday.

comment:12 in reply to: ↑ 9 Ipstenu19 months ago

Replying to ryan:

First, which object cache backend is being used? It should be updated.

W3 Total Cache (though it was turned off for one of my sites - I don't network activate it). I dropped him a line about this :)

Note: See TracTickets for help on using tickets.