Memory exhaustion on posts with thousands of comments

[pento]

The recent changes to WP_Comment_Query::get_comments() passes an array of all commend IDs to _prime_comment_caches(). This loads all of those comments, and tries to cache them.

On posts with thousands of comments, this causes memory exhaustion - we're seeing this on WP.com posts with (approximately) more than 10,000 comments.

This behaviour is a regression in 4.4 - previously, there were no memory exhaustion errors on these posts.

[boonebgorges]

Do you have more details about when this is happening? Is it on a normal comment page load, as when viewing a single post? Or does it occur only in the context of custom calls to get_comments() or WP_Comment_Query?

[pento]

It seems to be happening on normal comment page load. We've just hacked in a hard limit for now, but users are noticing that their massive comment threads aren't completely loading anymore. :-)

function wpcom_limit_comment_query( &$obj ) {
	if ( isset( $obj->query_vars['number'] ) && $obj->query_vars['number'] === '' )
		$obj->query_vars['number'] = '10000';
}
add_action( 'pre_get_comments', 'wpcom_limit_comment_query' );

[boonebgorges]

I'm assuming that comment pagination is enabled on these sites? Can you share posts_per_page and page_comments settings?

[pento]

Yep, comment pagination is enabled.

comments_per_page: 30
posts_per_page: 25
page_comments: 1

[boonebgorges]

For the record, the purpose of #8071 was to *prevent* this from happening.

I've just run some preliminary tests, and I've confirmed that on a vanilla installation, only the proper comments from the current comment-page are being loaded into the cache. More specifically:

• In comments_template(), the $comment_args['number'] var is being set from get_option( 'comments_per_page' )
• This value is then passed to WP_Comment_Query
• which is respected in WP_Comment_Query->get_comment_ids()
• which determines what gets passed to _prime_comment_caches()

@pento Could you be a dear and help to debug where things are going astray on wordpress.com? Some things to check:

• The logic that comments_template() uses to determine comments_per_page has changed a bit in 4.4. Have these changes been merged properly to .com? :-D
• Are there any callbacks (probably on 'pre_get_comments' or 'parse_comment_query') doing anything weird with the default query vars?

Without more information, it's hard to tell whether the regression is due to an oversight in core, or a plugin doing something wonky.

[boonebgorges]

Another thing to check:

• How does the theme initialize the comments loop? What I would call the "standard" technique is: (a) call comments_template() in the single-item template(s), then (b) in comments.php,if ( have_comments() ) { wp_list_comments(); }. But themes sometimes have other ways of doing it, and it could be that these "other ways" do not properly inherit the changes in 4.4

[pento]

Ah, I see. It looks like we haven't merged some relevant bits from comments_template().

Let's try that and we'll see what happens.

[pento]

Merging all these bits is proving to be a bit tricky. :-)

I'm fairly certain the changes to comments_template() will fix this, so I'm removing the blocker severity. Should hopefully be able to close it with a definite answer by tomorrow.

[boonebgorges]

Merging all these bits is proving to be a bit tricky. :-)

From ​https://wordpress.org/news/2008/12/coltrane/:

We heard how tired you were of doing upgrades for yourself and your friends, so now WordPress includes a built-in upgrade that will automatically notify you of new releases, and when you’re ready it will download them, install them, and upgrade your blog with a single click.

You guys should check that out for wordpress.com!!!1!

[pento]

Sounds pretty gimmicky to me. I wouldn't trust these WordPress developers to not break everything.

[afercia]

Replying to pento:

Sounds pretty gimmicky to me. I wouldn't trust these WordPress developers to not break everything.

+1 :D

[wonderboymusic]

@pento is fired

[rachelbaker]

Closing this ticket. @pento Please re-open if there still is something actionable here.