BP group forum reply triggers recount for every group forum

[boonebgorges]

Posting a reply results in bbp_update_reply() being called (on the 'bbp_new_reply' action). This function then calls bbp_update_reply_walker(), which triggers a recalc of counts for the entire forum tree.

In the case of BuddyPress groups, where all forums share a common "groups root" forum, this means that a new reply triggers a recalculation of every count in every forum.

I had a client report that the slowdown became apparent recently. I'm not sure whether there was a change in bbPress that caused this behavior to surface, or whether it's the size of the installation that's causing the problem. (The number of group forums is in the hundreds - it's not enormous.)

As far as I can see, there is no reason why the group root needs to have up-to-date counts. So I've put a truly heinous workaround in place, which prevents bbPress from running bbp_update_forum_reply_count() on the group root. I have two suggestions for bbPress:

1. Bail out of bbp_update_forum_reply_count() if it's the group root.
2. Provide a way for developers to disable the default behavior. I'm resorting to a filter on bbp_get_forum_id that returns 0 when the forum ID is the group root and bbp_update_forum_reply_count() is in the backtrace. Obviously, this is terrible. Moving the walker or bbp_update_forum_reply_count() to do_action() callbacks, or creating 'pre_' hooks in the functions, would allow devs to disable them selectively.

[tharsheblows]

What if bbp_update_forum_reply_count were replaced by bbp_bump_forum_reply_count which would only bump the counts stored in the postmeta and only for the direct ancestors?

The worry I have is that if there aren't up to date counts for the group, people will use the count repair tools more often because the counts will become noticeably off and the repair tools will time out without fixing it.

[tharsheblows]

bump rather than recalculate reply count in reply walker

[johnjamesjacoby]

Bumps would definitely help with this.

There are two reasons why counts were made overzealous:

• To maintain correctness through-out every branch in the tree that might be affected
• Lack of control over how WordPress purges post caches on children and parents

The first issue can be mitigated with bumps. The second issue should be improved in bbPress trunk for 2.6, but that needs confirming.

[thebrandonallen]

Assuming #1799 makes it into 2.6, this will be fixed.

[thebrandonallen]

Going to close as fixed, now that parts of #1799 relevant to this ticket are in trunk.

See [6036]

[boonebgorges]

Just had to wrestle with this issue again. I understand that the count bumps in #1799 will help, but I feel like it'd still be useful to have a 'bbp_pre_update_forum_reply_count' that would allow developers to disable or override the recount logic selectively. Could this be considered, especially if 2.6 isn't going to be out for a while and you'd be willing to get it into a 2.5.x release?

[netweb]

Replying to boonebgorges:

Just had to wrestle with this issue again. I understand that the count bumps in #1799 will help, but I feel like it'd still be useful to have a 'bbp_pre_update_forum_reply_count' that would allow developers to disable or override the recount logic selectively. Could this be considered, especially if 2.6 isn't going to be out for a while and you'd be willing to get it into a 2.5.x release?

Sure, have at it, reopen and patch away :)

[boonebgorges]

I had another think about how best to approach this. A pre_ filter in bbp_update_forum_reply_count() may be useful in some cases, but it'd require some acrobatics to use in my case. My root problem is the forum ancestor tree is huge, perhaps incorrectly so (botched migration? I'm not sure). By the time we're in bbp_update_forum_reply_count(), I'd have no easy way to detect whether the $forum_id being passed is one that ought to be allowed to update.

Instead, I'm going to suggest a new filter on the list of ancestor IDs in bbp_update_reply_walker(). I can grab the list using this filter, and rebuild it to exclude the branches of the ancestor tree that I want to blacklist. (You could also return an empty array if you wanted to run your own update routine.)

[johnjamesjacoby]

@boonebgorges What if this was just a built-in flag for every forum that allowed it to be exempt from the dynamic recalculating? The main wporg forums already work around this also, so it's not unheard of.

Also, I just added a bevy of no_found_rows parameters to various queries that weren't benefitting from it being there, and it made a dramatic difference on BuddyPress.org and bbPress.org. Maybe that's enough to help here too?

[johnjamesjacoby]

In the interests of getting 2.6 over the hump, I'm moving this to 2.7.

[boonebgorges]

Replying to johnjamesjacoby:

@boonebgorges What if this was just a built-in flag for every forum that allowed it to be exempt from the dynamic recalculating? The main wporg forums already work around this also, so it's not unheard of.

Oh, this is a pretty good idea. I'm not sure how/whether it'd help others, but it'd be an easy way for me to fix my problem. (It's essentially what I did in my workaround.)

Also, I just added a bevy of no_found_rows parameters to various queries that weren't benefitting from it being there, and it made a dramatic difference on BuddyPress.org and bbPress.org. Maybe that's enough to help here too?

Yeah, it'll definitely help, though it just kicks the can down the road. An out-out flag for recounts would be best. Thanks for considering!