Opened 7 years ago
Last modified 6 years ago
#3203 new defect (bug)
Performance issues when deleting a topic
Reported by: | codex-m | Owned by: | johnjamesjacoby |
---|---|---|---|
Milestone: | 2.7 | Priority: | normal |
Severity: | normal | Version: | trunk |
Component: | General - Performance | Keywords: | has-patch needs-testing dev-feedback |
Cc: | danielbachhuber |
Description
While deleting a topic in a very large database (like 4G or more) with a numerous of replies per topic. I've noticed that trashing topics will last for more than a minute or more.
With this, the server timeouts (in Nginx we got 504 gateway timeout). I've investigated this and found that when a topic is deleted, BBpress loops over replies and deletes one of them.
The issue is that when BBpress loops over replies and deletes it, it does repeated expensive queries to get_posts()
or WP_Query
without any object caching.
I fixed this by patching the slow lines in includes/forums/functions.php
with WordPress Object Cache methods. This speeds up the deletion numerous times from more than 2 minutes in some cases down to only seconds.
I'm attaching the diff of the patch based from the latest trunk so you can review and hopefully incorporate this to the coming release of BBpress.
Attachments (1)
Change History (6)
#1
@
7 years ago
- Keywords has-patch needs-testing added
- Milestone changed from Awaiting Review to 2.6
- Owner set to johnjamesjacoby
#2
@
7 years ago
Thanks for the patch! These kinds of optimizations are super useful.
I'll test, review, and make sure this is addressed in 2.6.
#3
@
6 years ago
- Keywords dev-feedback added
I'm not sure about this. I don't think object caching is helping here.
I can't find anything where the cache is invalidated in the provided patch. I guess with proper cache invalidation there will be the same amount of db queries as without object caching.
@johnjamesjacoby: To improve this, maybe it's worth looking into bbp_update_reply_walker() and bbp_trashed_reply. This runs once for every reply in the topic. Maybe while trashing a topic this should be un-hooked first:
add_action( 'trashed_post', 'bbp_trashed_reply' );
This would remove all the topic count / voice count etc. calculations which are done for each deleted reply if I'm not mistaken. bbp_recalculate_topic_engagements() runs multiple times too. But the user reply count still needs to be decreased somewhere for each deleted reply.
#4
@
6 years ago
- Milestone changed from 2.6 to 2.7
@wpdennis is correct, that unhooking these functions in some places will cause issues in others.
Probably the best thing we can do is introduce some kind of deferment API like WordPress comments has. This is too big for 2.6 now, so I think we need to bump this to 2.7 to look into further.
Implement object caching on slow lines when deleting a topic