Opened 12 years ago
Last modified 8 years ago
#1925 new enhancement
Optimize `_bbp_last_active_time` to use `post_modified` instead
Reported by: | vibol | Owned by: | |
---|---|---|---|
Milestone: | 2.7 | Priority: | normal |
Severity: | normal | Version: | 2.1.2 |
Component: | General - Performance | Keywords: | early |
Cc: | vibol, georgemamadashvili@…, wordpress@…, jared@…, stephen@…, nashwan.doaqan@…, erick@…, pippin@…, scott@… |
Description
Background ticket: http://bbpress.trac.wordpress.org/ticket/1885
Background Conversation:
Vibol: For the first query, last_active_time now looks to be the culprit. It's a postmeta lookup with a sort.
JJJ: Meta queries are always going to be a slow like this. There isn't an available index we can use here to avoid the meta query. We *could* hack in repurposing post_date as the freshness time, but we lose the actual date the topic was posted. It would take some rearchitecting to move the original post creation date to postmeta, to make the index available.
Problem: The _bbp_last_active_time query is doing a full table scan and sort against the postmeta table when retrieving forum index pages. The slowdown is linear with size of table scan. Here's an example slow query:
# Time: 120804 19:52:17 # Query_time: 21.460766 Lock_time: 0.000051 Rows_sent: 25 Rows_examined: 313770 SET timestamp=1344127937; SELECT SQL_CALC_FOUND_ROWS wp_posts.* FROM wp_posts INNER JOIN wp_postmeta ON (wp_posts.ID = wp_postmeta.post_id) WHERE 1=1 AND wp_posts.post_parent = 17 AND wp_posts.post_type = 'topic' AND (wp_posts.post_status = 'publish' OR wp_posts.post_status = 'closed') AND (wp_postmeta.meta_key = '_bbp_last_active_time' ) GROUP BY wp_posts.ID ORDER BY wp_postmeta.meta_value DESC LIMIT 0, 25;
Expected behavior: Should be fast if the data is indexed, but the postmeta table does not have indexes against it.
Guidance: I'm looking for someone to fix this, or, I am happy to put in the time to fix it with guidance on desired approach. This is a non-trivial issue, so someone with more experience in WP/BBP is ideal.
Attachments (18)
Change History (101)
#3
@
12 years ago
As I dig through this, it seems like there are more issues that crop up. See the following set of queries that happen from forum view to thread reply.
# Query_time: 14.640429 Lock_time: 0.000133 Rows_sent: 25 Rows_examined: 313770 SET timestamp=1345373401; SELECT SQL_CALC_FOUND_ROWS wp_posts.ID FROM wp_posts INNER JOIN wp_postmeta ON (wp_posts.ID = wp_postmeta.post_id) WHERE 1=1 AND wp_posts.post_parent = 17 AND wp_posts.post_type = 'topic' AND (wp_posts.post_status = 'publish' OR wp_posts.post_status = 'closed') AND (wp_postmeta.meta_key = '_bbp_last_active_time' ) GROUP BY wp_posts.ID ORDER BY wp_postmeta.meta_value DESC LIMIT 0, 25; # Time: 120819 5:51:22 # Query_time: 11.424189 Lock_time: 0.000054 Rows_sent: 1 Rows_examined: 1623849 SET timestamp=1345373482; SELECT COUNT( DISTINCT post_author ) FROM wp_posts WHERE ( post_parent = 1623928 AND post_status = 'publish' AND post_type = 'reply' ) OR ( ID = 1623928 AND post_type = 'topic' ); # Time: 120819 5:51:50 # Query_time: 27.115751 Lock_time: 0.000070 Rows_sent: 55996 Rows_examined: 56000 SET timestamp=1345373510; SELECT ID FROM wp_posts WHERE post_parent = 17 AND post_status IN ( 'publish', 'closed' ) AND post_type = 'topic' ORDER BY ID DESC;
The last one I'll have to try to attach because it's too big to fit in this field.
#4
@
12 years ago
WP posts have four date fields. post_date, post_date_gmt, post_modified and post_modified_gmt. Feels hacky but we could use the post_modified or post_modified_gmt in the forums and topics records for the last_active_time. Even without index, not having to join against postmeta will increase the performance a lot
#6
@
12 years ago
Sadly, since all of the core database columns are used by WordPress already, reusing a column for our own purposes is probably dangerous, and will take more work to override the way WordPress naturally writes to those columns than it's probably worth.
post_modified *almost* makes sense, but we should be fully aware of what WordPress core uses that for before hijacking it. If it's used for comparing revisions, we're out of luck; if it's not used at all, we're at risk of it being deprecated and disappearing completely (like post_category).
#8
@
12 years ago
Query times are the average of 20 runs. First couple of runs ignored as to get MySQL "hot".
DB Size:
forums 5
topics 29208
replies 95755
Main query now (ordering by meta_value):
SELECT SQL_CALC_FOUND_ROWS wp_posts.ID FROM wp_posts INNER JOIN wp_postmeta ON ( wp_posts.ID = wp_postmeta.post_id ) WHERE 1 = 1 AND wp_posts.post_parent = 4 AND wp_posts.post_type = 'topic' AND ( wp_posts.post_status = 'publish' OR wp_posts.post_status = 'closed' OR wp_posts.post_status = 'private' ) AND ( wp_postmeta.meta_key = '_bbp_last_active_time' ) GROUP BY wp_posts.ID ORDER BY wp_postmeta.meta_value DESC LIMIT 0, 15
Avg. time: 162ms
Main query without the join:
SELECT SQL_CALC_FOUND_ROWS wp_posts.ID FROM wp_posts WHERE 1 = 1 AND wp_posts.post_parent = 4 AND wp_posts.post_type = 'topic' AND ( wp_posts.post_status = 'publish' OR wp_posts.post_status = 'closed' OR wp_posts.post_status = 'private' ) GROUP BY wp_posts.ID ORDER BY wp_posts.post_modified DESC LIMIT 0, 15
Avg. time: 14.9ms
I'm cleaning the patch to switch all uses of _bbp_last_active_time meta to post_modified. Will post asap.
#9
follow-ups:
↓ 13
↓ 19
@
12 years ago
First pass
1) The batch update script in tools.php is not working. Need to fix.
2) I need to change the importer in includes/admin/converters/bbPress1.php, but I don't have a clue how. Need to research how it works. (Never played with the importer)
#10
@
12 years ago
Finally swinging back around to this. MZAWeb, are you using your changes in production?
Another field that has come to my attention is menu_order. It's primarily used for menu items, but is seldom used for Posts. There are already many plugins that use it to allow users to manually order their Pages and Posts. It's an int(11) so it should be able to fit a timestamp until about 2033.
In either case (post_modified or menu_order), altering the index on the table to add the newly referenced field should help speed things up as well since post_modified and menu_order are not in the indices.
I'd be curious to hear your thoughts.
#11
@
12 years ago
An update.
First off, thank you MZAWeb for contributing your patch. The patch makes a significant difference in performance for my use case, enough so that I intend to use it in production.
The only concern would be curiosity into where WordPress updates last_modified--I believe it does this if a Post is edited (in this case, a Topic). I'm still mulling over the idea of using menu_order, simply because that is a field that is seldom changed by WordPress.
I'll put up changes I made to MZAWeb's patch that prevents an infinite recursion when a Forum is created. I'll also include an index that I've added to further speed up the query.
#13
in reply to:
↑ 9
@
12 years ago
- Cc stephen@… added
Replying to MZAWeb:
2) I need to change the importer in includes/admin/converters/bbPress1.php, but I don't have a clue how. Need to research how it works. (Never played with the importer)
This part should be pretty straightforward (for me), I'll test your patch and add a patch for the importers here next week.
#14
follow-up:
↓ 15
@
12 years ago
- Cc nashwan.doaqan@… added
I really like this ticket it will help to get a better performance , I read your conversation about using last_modified or menu_order but It seems not a good idea for me , And you know why ! ...
There are some other options we can use :
1 - Create a new table "wp_forum_data" like an example ,
with an indexed columns for the most needed things .
2 - Extend the wp_posts table columns .
#15
in reply to:
↑ 14
;
follow-up:
↓ 16
@
12 years ago
Replying to alex-ye:
I really like this ticket it will help to get a better performance , I read your conversation about using last_modified or menu_order but It seems not a good idea for me , And you know why ! ...
Actually, I'm not sure why it doesn't seem a good idea for you. Care to elaborate? I'm on the fence myself and would love to hear arguments.
There are some other options we can use :
1 - Create a new table "wp_forum_data" like an example ,
with an indexed columns for the most needed things .
2 - Extend the wp_posts table columns .
I totally get the arguments in favor of that, I really do. But I'm quite sure adding a new table or field it's not gonna happen.
#16
in reply to:
↑ 15
@
12 years ago
Replying to MZAWeb:
Care to elaborate? I'm on the fence myself and would love to hear arguments.
The main arguments is :
1 - WordPress is still use the post_modified, post_modified_gmt and menu_order , So using this fields by bbPress in another meaning can cause many bugs as my expectation .
2 - _bbp_last_active_time is not the only meta value which make SQL queries slow , bbPress use many meta fields , and it may be add more in the future
3 - What if the keymaster want to know the really last modified forum ?! , last_active_time and post_modified have a confuse meaning ! , This is not logical to me
I totally get the arguments in favor of that, I really do. But I'm quite sure adding a new table or field it's not gonna happen.
I think adding a new table can be good idea if we have many meta to index , but can you explain why you refuse to add a new column ?
#17
@
11 years ago
- Milestone changed from 2.4 to 2.5
If someone wants to patch this up to use post_modified, I'd love to get this going for 2.5.
#19
in reply to:
↑ 9
@
11 years ago
Replying to MZAWeb:
2) I need to change the importer in includes/admin/converters/bbPress1.php, but I don't have a clue how. Need to research how it works. (Never played with the importer)
Attached 1925.bbPress1.php.diff which removes that entire field mapping as we already map to post_modified
a couple of lines up...
This would also need to be removed from all importers (phpBB, vBulletin etc) and is implemented identically in each importer, quick and easy once we are ready to commit this ticket.
#20
follow-up:
↓ 29
@
11 years ago
- Keywords has-patch added
Hi! it's the bugist formally known as cnorris23. I'm including a more comprehensive patch. The current patch doesn't work because wp_insert_post(), and by extension, wp_update_post(), doesn't allow you to pass your own post_modified(_gmt) times. In order to do this, I've hooked into wp_insert_post_data, to essentially reset the post_modified(_gmt) times back to the times in the current post object. Also, at this time, since a true post edit time was mentioned early in the ticket, a _bbp_last_edit_time_gmt post meta key will be added with what should have been the new post_modified_gmt time. The other thing this patch does, is retain, and continue to add/update the _bbp_last_active_time for some back compat.
Things to note. Please do not run this on a production server, with production data. It won't break things per se, but your topics/forums definitely won't be properly sorted. This leads me to my final note. I have patches for both tools.php and the importers. The tools.php can use a little more work and cleanup, and I need to write a few queries to run during the upgrade routine. Since the queries being run in tools.php are little beasts, the same will go for the upgrade routine, I want to see how this patch is going to land before I spend too much time taming.
#21
@
11 years ago
- Milestone changed from 2.5 to 2.6
This patch is a great start. Any thoughts on what a database upgrade routine would look like?
#22
@
11 years ago
- Component changed from General to Performance Improvements
- Keywords early added; dev-feedback removed
#23
@
11 years ago
- Summary changed from Performance: Optimize _bbp_last_active_time to use an indexed field for very large deployments to Optimize `_bbp_last_active_time` to use `post_modified` instead
#24
in reply to:
↑ 2
@
11 years ago
Replying to johnjamesjacoby:
... maybe, try a subquery using the ID and post_parent indexes, but that only works if we can guarantee that newer replies will always have a higher ID. This *should* be the case, but it's hard to know for sure given the way the importer works, and how other forum software stores its data.
For the importers this 'should' always be the case. The way the current importer works is it will import all the forums, then topics, then replies so in theory it would be a rare case but not foolproof.
In theory the above would also mitigate against how 'other forum software' stores it's data as we would only be importing the data based on the way the current importer imports it's data.
What does cause an issue is the built in WordPress WXR Export/Import, it includes the current post_id during export and upon import if that post_id is not used it will use that existing post_id to assign to the freshly imported post.
Test Scenario:
- Import an existing forum into my WP install bbPress/Import test site
- As this site has done a couple of imports, currently the post ID increment in wp_posts is ~400,000 and in wp_postmeta 2,000,000
- When I import a new forum, topic or reply it will get the next post_id available 400,001, 400,002
- When I export those using WP's WXR Export the post_id is included in the export 400,001, 400,002
- During the WXR import if the post_id in the target installation is available it will be used eg 400,001, 400,002 etc, etc
- When new posts, pages, forums, topics are created they will be assigned post_id 2, 3, 4.... right up to the existing post_id's 400,001, 400,002 and then continue from the next available post_id.
Coming shortly, testing notes from patches attached to this ticket.
#25
@
11 years ago
Will update my testing notes and fresh patches tomorrow with a clear head...
Anything I write at the moment is more confusing than helpful.
#26
@
11 years ago
Some data from each patch.
http://sqlfiddle.com/#!2/7898a/1 1925-mzaweb-wp_posts.sql
1925-mzaweb.png
SELECT * FROM wp_posts WHERE ID = 90581 OR post_parent = 90581
http://sqlfiddle.com/#!2/e6a05/1 1925-thebrandonallen-wp_posts.sql
1925-thebrandonallen.png
SELECT * FROM wp_posts WHERE ID = 42628 OR post_parent = 42628
#27
@
11 years ago
Oops, forgot wp_postmeta
data in the above:
- 1925-thebrandonallen wp_postmeta
42628 | _bbp_last_active_time | 2013-12-08 21:06:09 |
42628 | _bbp_last_edit_time_gmt | 2013-12-08 10:09:36 |
- 1925-mzaweb wp_postmeta
90581 | _bbp_last_active_time | 2013-12-08 19:44:30 |
#28
follow-up:
↓ 33
@
11 years ago
1925-MZAWeb
- This does 95% of what is needed and the only issue I see is when you edit a topic the topic freshness is set to the edited topic time.
- This kind of puts us back where we started from in that we have used the
post_date
,post_date_gmt
,post_modified
&post_modified_gmt
but if we were to use awp_postmeta
field to keep the topic freshness accurate we are then joining thewp_posts
&wp_postmeta
again.
Could we have the topic freshness read from post_modified
if there are no revisions to the topic, if there are revsions and replies then freshness is calculated from the post_modified
field of the last reply?
1925-thebrandonallen
- All the revision times (
post_date
,post_date_gmt
,post_modified
&post_modified_gmt
) are set to the latest topic freshness timestamp. As we want to keep the topic (and reply) edited revisions these timestamps need to remain accurate.
- Both functions
bbp_fix_post_modified
&bbp_update_post_modified_helper
are manipulatingpost_modified
&post_modified_gmt
fields with checks to only updateforum
ortopic
post types (I looked but could not find a way to stop this occurring to the revisions)
- As
_bbp_last_edit_time_gmt
is the only place (currently based on the above) the true last edit time of a topic there is no way to recalculate this time as a Repair Tool inincludes/admin/tools.php
if things get out of sync.
- Looking at
wp_post_update
(/wp-includes/post.php#L2972) it useswp_insert_post
(/wp-includes/post.php#L2812) and if it is an 'update' (rather than new post) it automatically adds the current time to thepost_modified
&post_modified_gmt
db fields.
Can the functions bbp_fix_post_modified
& bbp_update_post_modified_helper
be modified so that all the revisions don't end up setting all the timestamps to the most recent topic freshness?
#29
in reply to:
↑ 20
@
11 years ago
Replying to thebrandonallen:
Also, at this time, since a true post edit time was mentioned early in the ticket, a _bbp_last_edit_time_gmt post meta key will be added with what should have been the new post_modified_gmt time.
This is not needed as the value is stored in the post_date
of the latest post revision of the topic/reply, in this case topic post ID 90581's latest revision is post id 90610
.
#31
@
11 years ago
That's it for now, head is swimming in data... Any thoughts, comments or corrections to my understanding of the logic here would be greatly appreciated.
#32
@
11 years ago
One more note, if you use the 'Recent Topics' widget with the view 'Popular' we will again be joining the wp_posts
& wp_postmeta
tables as this is calculated via wp_postmeta
._bbp_reply_count
.
#33
in reply to:
↑ 28
;
follow-up:
↓ 34
@
11 years ago
Replying to netweb:
- Good catch. It didn't do that at one point. Took some tracking down, but I found the issue. I'll get patch up tonight hopefully. Gotta remove some debug code.
- bbp_update_post_modified_helper was stamping out the revision dates. Just needed to do a stricter post_type check
- This is true. It's a side effect of trying to use post_modified. I thought about splitting post_modified and post_modified_gmt making one the freshness and leaving the other the as the true modified time, but that would have been incredibly confusing.
- This is correct as well. However, my goal was to keep in line with the current system. The way it's done now is we create a last_active time when a reply/topic is created, then propagate that time up the chain to the parent topic/forum. Solely using wp_update_post would work, but we wouldn't be able to specify a specific time as we do now, and we run the risk of having times that are out of sync, and not representative of the relative child element. Here's an example. With our current system (preserved in my patch) a reply is created at 2013-12-09 10:10:10, and the topic/forum last active meta values will be the same, 2013-12-09 10:10:10. Using wp_update_post could very easily result in that same reply resulting in a topic last active of 2013-12-09 10:10:11, and forum last active of 2013-12-09 10:10:12. On a busy forum on a shared host (a very likely scenario), the disparity could be much worse. For most, this difference will be negligible, but I figured it was better to maintain the consistent outcome we have currently. Obviously, wp_update_post would greatly simplifies the code.
Replying to netweb:
This is not needed as the value is stored in the post_date of the latest post revision of the topic/reply, in this case topic post ID 90581's latest revision is post id 90610.
I originally thought the same thing, but then it struck me that revisions are optional. You can disable them in bbPress, which removes the option to save a revision in the topic/forum edit forms, or they can be disabled system wide via a WP filter or constant. As such, we can't depend on a revision to be available to use, hence, _bbp_last_edit_time_gmt.
Thanks for reviewing the ticket!
#34
in reply to:
↑ 33
@
11 years ago
Replying to thebrandonallen:
- This is correct as well. However, my goal was to keep in line with the current system. The way it's done now is we create a last_active time when a reply/topic is created, then propagate that time up the chain to the parent topic/forum. Solely using wp_update_post would work, but we wouldn't be able to specify a specific time as we do now, and we run the risk of having times that are out of sync, and not representative of the relative child element. Here's an example. With our current system (preserved in my patch) a reply is created at 2013-12-09 10:10:10, and the topic/forum last active meta values will be the same, 2013-12-09 10:10:10. Using wp_update_post could very easily result in that same reply resulting in a topic last active of 2013-12-09 10:10:11, and forum last active of 2013-12-09 10:10:12. On a busy forum on a shared host (a very likely scenario), the disparity could be much worse. For most, this difference will be negligible, but I figured it was better to maintain the consistent outcome we have currently. Obviously, wp_update_post would greatly simplifies the code.
We don't need to apply a specific time as WordPress handles this natively:
- A new post is created using
wp_insert_post
and the current timestamp is set inpost_date
,post_date_gmt
,post_modified
&post_modified_gmt
- An updated/edited post uses
wp_update_post
and the originalpost_date
&post_date_gmt
times are kept withpost_modified
&post_modified_gmt
updated with the current timestamp.
I'll come back to the issue of possible last active meta conflicts _bbp_last_active_time
below.
- bbp_update_post_modified_helper was stamping out the revision dates. Just needed to do a stricter post_type check
Yes, I thought that was the case.
I originally thought the same thing, but then it struck me that revisions are optional. You can disable them in bbPress, which removes the option to save a revision in the topic/forum edit forms, or they can be disabled system wide via a WP filter or constant. As such, we can't depend on a revision to be available to use, hence, _bbp_last_edit_time_gmt.
We are still good here and don't need _bbp_last_edit_time_gmt
- WordPress Revisions disable
define('WP_POST_REVISIONS', false );
- bbPress Revisions disable
_bbp_allow_revisions
=0
In both these cases post_modified
& post_modified_gmt
dates are updated to the correct time of the last topic edit.
Coming back to 'Topic Freshness' / _bbp_last_active_time
and function bbp_get_topic_last_active_time
(/includes/topics/template.php#L1791)
!#diff Index: template.php =================================================================== --- template.php (revision 5220) +++ template.php (working copy) @@ -1791,15 +1789,14 @@ function bbp_get_topic_last_active_time( $topic_id = 0 ) { $topic_id = bbp_get_topic_id( $topic_id ); - // Try to get the most accurate freshness time possible - $last_active = get_post_meta( $topic_id, '_bbp_last_active_time', true ); - if ( empty( $last_active ) ) { - $reply_id = bbp_get_topic_last_reply_id( $topic_id ); - if ( !empty( $reply_id ) ) { - $last_active = get_post_field( 'post_date', $reply_id ); - } else { - $last_active = get_post_field( 'post_date', $topic_id ); - } + // Check the topic for the last reply id + $reply_id = bbp_get_topic_last_reply_id( $topic_id ); + + // If we have a reply use that timestamp, if not use the topic timestamp + if ( !empty( $reply_id ) ) { + $last_active = get_post_field( 'post_date', $reply_id ); + } else { + $last_active = get_post_field( 'post_date', $topic_id ); } $last_active = !empty( $last_active ) ? bbp_get_time_since( bbp_convert_date( $last_active ) ) : '';
Rather than go getting the _bbp_last_active_time
we can use the native post time stamps as they are accurate and just use the existing method to calculate the freshness based on if the topic has a reply use the last reply post_date
, if no replies use the topic post_date
This works for all combinations of revisions...
Patch incoming....
#35
@
11 years ago
- Keywords needs-testing added
_bbp_last_active_time
is kept for back compat, most likely will be found in use with custom views.
I am messing about with the tools.php
at the moment trying to work out if we need a database upgrade routine is needed or not and I will add the converters to this also.
I will also run a couple of SQL queries as per @MZAWeb's previous tests.
p.s. There are a few extra PHPDoc updates in the includes/forums/functions.php
section.
#36
@
11 years ago
- Summary changed from Optimize `_bbp_last_active_time` to use `post_modified` instead to Optimize `_bbp_last_active_time` to use `post_date` instead
Boom! Patch no longer uses post_modified
and now uses post_date
In 1925.05.diff we now use post_date
and let WordPress handle 'all the things'
- Topics and Replies are ordered by
post_date
- Stickies not in original topic query ordered by
post_date
orderASC
- Forum, Topic & Reply revision dates are handled by default WordPress revision handling
#37
@
11 years ago
DB Size:
forums 15
topics ~20,000
replies ~30,000
1. The forum in this query has only ~1,000 topics
2. I'd hardly call my local MySQL optimized either, out of the box config.
3. Query times are the average of 20 runs. First couple of runs ignored as to get MySQL "hot".
SELECT SQL_CALC_FOUND_ROWS wp_posts.ID FROM wp_posts INNER JOIN wp_postmeta ON ( wp_posts.ID = wp_postmeta.post_id ) WHERE 1 = 1 AND wp_posts.post_parent = 42642 AND wp_posts.post_type = 'topic' AND ( wp_posts.post_status = 'publish' OR wp_posts.post_status = 'closed' OR wp_posts.post_status = 'private' ) AND ( wp_postmeta.meta_key = '_bbp_last_active_time' ) GROUP BY wp_posts.ID ORDER BY wp_postmeta.meta_value DESC LIMIT 0, 15
Average query time: 374.8ms
SELECT SQL_CALC_FOUND_ROWS wp_posts.ID FROM wp_posts WHERE 1 = 1 AND wp_posts.post_parent = 42642 AND wp_posts.post_type = 'topic' AND ( wp_posts.post_status = 'publish' OR wp_posts.post_status = 'closed' OR wp_posts.post_status = 'private' ) GROUP BY wp_posts.ID ORDER BY wp_posts.post_date DESC LIMIT 0, 15
Average query time: 12.1ms
#38
follow-up:
↓ 39
@
11 years ago
Hi, thank you all for putting time and energy into this issue. Please let me know when I should spend time to build a test environment and run these patches against them. Also, it just occurred to me that I never uploaded my adapted patch from long ago. I have it running in production now and it's been working for many months. I made some changes in there that may still be applicable for the Tools section. However, my patch is against 2.2.4 and last_modified. See attached.
#39
in reply to:
↑ 38
@
11 years ago
Replying to vibol:
Hi, thank you all for putting time and energy into this issue. Please let me know when I should spend time to build a test environment and run these patches against them.
Now would be great and extremely helpful to get another set of eyes on the data :)
I am still digging through the logic and confirming 'existing topics' order but initial thoughts are everything is fine and no database upgrade routine will be needed.
The quickest way I see would be to make a copy of your current database and setup a fresh install, apply this patch and then compare both performance and the logic to make sure all the things are ordered correctly and there are no issues with topic or reply edits and revisions.
Replying to vibol:
Also, it just occurred to me that I never uploaded my adapted patch from long ago. I have it running in production now and it's been working for many months. I made some changes in there that may still be applicable for the Tools section. However, my patch is against 2.2.4 and last_modified.
At a quick glance the patch is very similar rather than use post_modified
we apply the same logic but use post_date
which would be very similar performance.
Any calls to update post_modified
manually are no longer required (nor did they work unless a direct SQL write made the modifications) and as this is part of WordPress CORE we now just let WordPress' wp_insert_post
and wp_update_post
do it's thing.
#41
follow-up:
↓ 49
@
11 years ago
Topics are reporting the correct freshness, but sorting by topic freshness (the default method) does not work. Topics are sorted by the date they were originally posted, not by the time of the most recent reply.
#45
follow-up:
↓ 61
@
11 years ago
Thanks johnjamesjacoby. This fixes the problem with inconsistent times I found and mentioned earlier. Just waiting to see what netweb comes up with to fix the sorting by last active.
#46
@
11 years ago
Above patch switches back to post_modified
to avoid issues with ordering forums/topics/replies by their original creation date, which should still be possible to do.
#47
@
11 years ago
This brings us back to my patch. wp_update_post does not allow you to pass a custom time to post_modified, and post_modified_gmt.
http://core.trac.wordpress.org/browser/trunk/src/wp-includes/post.php#L2819
#48
@
11 years ago
I've just realized there is a way to do it using wp_update_post. This should get rid of the need for the bbp_update_(forum/topic)_post_modified() and bbp_update_post_modified_helper() functions. It should also fix the issue of revision times being set to last active time. Will get my thoughts in patch form later this evening.
#49
in reply to:
↑ 41
@
11 years ago
Awesome 1925.patch patch :)
1925.2.patch is a refresh with changes:
- Less the
bbp_update_topic()
r5222 commit - Forum freshness
bbp_get_forum_last_active_time
fixed
Just one last issue to fix as $topic_id post_modified
currently gets refreshed when a topic is edited.
Gone looking at WP's revisions code to see if we can check for a revision before firing wp_update_post
in bbp_update_forum_last_active_time
#50
@
11 years ago
In 1925.converters.patch updates all the bbPress importers:
- Removes
_bbp_last_active_time
topic date import from all importers - Remaps topic date field mapping using to
post_modified
(Vanilla, vBulletin3, vBulletin & phpBB)
#51
follow-up:
↓ 52
@
11 years ago
The above patch 1925.post_modified.01.diff makes some improvements to my previous patch.
- I've updated to using wp_update_post as this seems to be the desired approach. It's definitely easier to maintain.
- bbp_fix_post_modified sticks around, for two reasons. 1) In order for the custom post_modified(_gmt) times that we pass to wp_update_post to actually be used, we need to make sure the are set. 2) this is a good place to set the _bbp_last_edit_time_gmt. This allows us to have an edit time that can function as the post_modified time that we are losing. Plus, it's needed to fix revision times. I have updated the logic, so we are no longer potentially making calls the db.
- Introduce bbp_fix_revision_times. Revisions use the post_modified(_gmt) from the post they are revising to set the post date times to set the corresponding post_date(_gmt) fields. Since we're using post_modified for fresheness, this is throwing off the revision times. We pull _bbp_last_edit_time_gmt, and use it as a starting point to rebuild the proper date values.
- Right before I posted this, I realized that replies were creating post revisions, since we're using wp_update_post now. I've added in the revisions suspension logic from the edit topic handler to stop this.
@
11 years ago
post_modified - with wp_update_post, allows us to pass our own post_modified(_gmt) times, fixes issue with revisions
#52
in reply to:
↑ 51
@
11 years ago
- Keywords commit added; has-patch needs-testing removed
- Type changed from defect to enhancement
Thanks so much for that. It seems to work really well. I refreshed the patch to add a revision check for forums, just in case they ever do get revisions.
This is ready to commit whenever we have a database upgrade routine.
#53
follow-up:
↓ 54
@
11 years ago
We should make sure that reply revision date meta works the same as topics do, to keep everything consistent.
#54
in reply to:
↑ 53
@
11 years ago
Replying to johnjamesjacoby:
We should make sure that reply revision date meta works the same as topics do, to keep everything consistent.
When I figured out the issue with topics, I double checked replies, and there didn't seem to be any issues, but more eyes would be good. Mine were tired.
#55
@
11 years ago
Testing 1925.3.patch now...
Looks good so far with all the
- 'freshness' times accurate for template notices/lists including new topics/replies added.
- Topic edited content and time is good
- Reply edited content and time is good
- Move a topic via front/back end is good
- Split topic times look good
- Wishing we had PHPUnit tests
Issues:
- Reply Revisions
- An edited reply works as expected, it does not bump the freshness and the edited content IS correct though:
- Reply post revisions are not created as posts in
wp_posts
- Reply revisions log
_bbp_revision_log
is not created inwp_postmeta
- All the functions for the above are there, it just doesn't happen, needs more investigation.
- Merge Topic (Related #2329)
- Merge 'TopicB 2:00pm' topic into 'TopicA 1:00pm'
- 'TopicA' freshness is now 2:00pm
- Merge 'TopicC 3:00pm' topic into 'TopicA 1:00pm'
- 'TopicA 1:00pm' topic freshness remains at 2:00pm
- This is a wider topic merge issue than this ticket specifically I think.
Importers
- As per the attached patch above where I removed
_bbp_last_active_time
from each importer as this should only be the last known edit/update time stamp of the topic id. Just need to think about the logic of each forum software a bit before writing a patch.
Tools // Repair topic revisions last edit time _bbp_last_edit_time_gmt
I think this SQL is good, another set of eyes to confirm
INSERT INTO `$wpdb->postmeta` (`post_id`, `meta_key`, `meta_value`)
SELECT topic.post_parent, '_bbp_last_edit_time_gmt ', topic.post_date_gmt FROM wp_posts AS topic WHERE topic.post_parent =90702 ORDER BY topic.ID DESC LIMIT 1
Database Upgrade
- ToDO - Still thinking about this ;)
Performance Average query time: 13.2ms
SELECT SQL_CALC_FOUND_ROWS wp_posts.ID FROM wp_posts WHERE 1 =1 AND wp_posts.post_parent =42642 AND wp_posts.post_type = 'topic' AND ( wp_posts.post_status = 'publish' OR wp_posts.post_status = 'closed' OR wp_posts.post_status = 'private' OR wp_posts.post_status = 'hidden' ) ORDER BY wp_posts.post_modified DESC LIMIT 0 , 15
#56
follow-up:
↓ 57
@
11 years ago
BLOCKER: Current itteration breaks ALL the importers
When importing we use wp_insert_post
to insert each forum, topic, reply post resulting in:
WordPress database error: [Incorrect datetime value: '' for column 'post_date_gmt' at row 1]
We need to remove the two filters when doing an import and them back once finished.
remove_filter( 'wp_insert_post_data', 'bbp_fix_post_modified');
remove_filter( 'wp_insert_post_data', 'bbp_fix_revision_times');
#57
in reply to:
↑ 56
@
11 years ago
Replying to netweb:
BLOCKER: Current itteration breaks ALL the importers
When importing we use
wp_insert_post
to insert each forum, topic, reply post resulting in:
WordPress database error: [Incorrect datetime value: '' for column 'post_date_gmt' at row 1]
We need to remove the two filters when doing an import and them back once finished.
remove_filter( 'wp_insert_post_data', 'bbp_fix_post_modified');
remove_filter( 'wp_insert_post_data', 'bbp_fix_revision_times');
I wasn't able to reproduce this. In my testing, the imported forum/topic/reply never made it far enough through the 'if' logic to even have their dates adjusted. bbp_fix_revision_times is the only function the messes with post_date_gmt, but, since we don't insert revisions, the logic that messes with post_date_gmt never runs. I only have bbPress 1 data to run import tests, but I was never able to get the db error to occur.
#59
follow-up:
↓ 62
@
11 years ago
- Keywords commit removed
Taking 1925.3.patch for another spin...
The importer now seems to work with this, not sure why it didn't work previously.
Here is a copy of my standard phpBB SQL Database ntwb_phpbbv3.zip (66kb) if someone else could test this to eliminate my setups out of the equation that would be grand.
bbPress Importer Settings: Select Platform phpBB
| Database Name ntwb_phpbbv3
| Table Prefix phpbb_
(The remaining settings are your own primary database ip address, username, password etc)
Creating a forum in the back end fails, technically it is created, but admin doesn't come back, you are left with a blank white screen.
Fatal error: Maximum function nesting level of '100' reached, aborting! in C:\xampp\htdocs\wp-includes\functions.php on line 2837
Replying to johnjamesjacoby:
post_modified *almost* makes sense, but we should be fully aware of what WordPress core uses that for before hijacking it. If it's used for comparing revisions, we're out of luck; if it's not used at all, we're at risk of it being deprecated and disappearing completely (like post_category).
Has anyone tested post/page revisions with this? I have now :(
Blocker: 1925.3.patch breaks post revisions
Repro:
- Deactivate bbPress
- Create a WordPress post and publish
- Edit post and update
- Make another edit and see post revisions below post body
- Activate bbPress
- Edit the same post again, no new revisions are created
- The post content is updated reflecting the edit changes
- No new revision post is created in
wp_posts
- Deactivate bbPress, edit post and a new revision is displayed
- Check the
wp_posts
table and a new revision now exists
Replying to netweb:
Issues:
- Reply Revisions
- An edited reply works as expected, it does not bump the freshness and the edited content IS correct though:
- Reply post revisions are not created as posts in
wp_posts
- Reply revisions log
_bbp_revision_log
is not created inwp_postmeta
- All the functions for the above are there, it just doesn't happen, needs more investigation.
Reply Revisions work as expected using /trunk
and do NOT work with 1925.3.patch
This ticket was mentioned in IRC in #bbpress-dev by netweb. View the logs.
11 years ago
#61
in reply to:
↑ 45
@
11 years ago
Replying to thebrandonallen:
Just waiting to see what netweb comes up with to fix the sorting by last active.
I found a solution for this through changing the ORDER BY
in bbp_has_topics()
and performance is ~4x
Current: Query took 0.0675 sec (897 topics and 1,836 replies)
SELECT SQL_CALC_FOUND_ROWS wp_posts.ID FROM wp_posts INNER JOIN wp_postmeta ON (wp_posts.ID = wp_postmeta.post_id) WHERE 1=1 AND wp_posts.post_parent = 96479 AND wp_posts.post_type = 'topic' AND (wp_posts.post_status = 'publish' OR wp_posts.post_status = 'closed' OR wp_posts.post_status = 'private' OR wp_posts.post_status = 'hidden') AND (wp_postmeta.meta_key = '_bbp_last_active_time' ) GROUP BY wp_posts.ID ORDER BY wp_postmeta.meta_value DESC LIMIT 0, 15
Solution: Query took 0.0173 sec (897 topics and 1,836 replies)
SELECT SQL_CALC_FOUND_ROWS wp_posts.ID FROM wp_posts WHERE 1=1 AND wp_posts.post_parent = 96479 AND wp_posts.post_type = 'topic' AND (wp_posts.post_status = 'publish' OR wp_posts.post_status = 'closed' OR wp_posts.post_status = 'private' OR wp_posts.post_status = 'hidden') ORDER BY COALESCE( ( SELECT MAX( post_date ) FROM wp_posts r1 WHERE r1.post_parent = wp_posts.id ) , wp_posts.post_date ) DESC LIMIT 0, 15
The function uses /Plugin_API/Filter_Reference/posts_orderby as this type of custom SQL cannot be parsed directly by WP_Query's 'ORDERBY' parameter
This was tested alongside 1925.05.diff.
The function as it stands changes the "ORDER BY" for EVERY WP_Query
so we would need to use apply_filter
and remove_filter
before/after bbp_has_topics
$bbp->topic_query
function bbp_topic_freshness_override($orderby_statement) {
$orderby_statement = "COALESCE( ( SELECT MAX( post_date ) FROM wp_posts r1 WHERE r1.post_parent = wp_posts.id ) , wp_posts.post_date ) DESC";
return $orderby_statement;
}
add_filter('posts_orderby', 'bbp_topic_freshness_override');
It has some larger ramifications for bbp_has_topics()
though as the custom 'ORDER BY' can not be changed by end users eg. bbp_has_topics( array( 'orderby' => 'title', 'order' => 'ASC' ) )
To keep current back compat these would need to be moved into a new function using /Plugin_API/Filter_Reference/posts_clauses (including 'posts_orderby').
eg. (idea, not tested ;))
function bbp_topic_freshness_override($orderby_statement) {
if( $bbp->topic_query['orderby'] == 'date' ) {
$orderby_statement = "COALESCE( ( SELECT MAX( post_date ) FROM wp_posts r1 WHERE r1.post_parent = wp_posts.id ) , wp_posts.post_date ) DESC";
}
if( $bbp->sticky_query ) {
$orderby_statement = "wp_posts.post_date ASC";
}
if( $bbp->topic_query['orderby'] == 'title' ) {
$orderby_statement = "wp_posts.post_title DESC";
}
if( $bbp->topic_query['orderby'] == 'etc' ) {
$orderby_statement = "etc etc etc";
}
return $orderby_statement;
}
apply__filter('posts_orderby', 'bbp_topic_freshness_override');
It has some benefits also that a similar function could be used for forums topic freshness also.
I haven't taken to rewrite bbp_has_topics()
for the above 'proof on concept' but everyone's thoughts on this method would be greatly appreciated :)
#62
in reply to:
↑ 59
@
11 years ago
Replying to netweb:
The importer now seems to work with this, not sure why it didn't work previously.
Here is a copy of my standard phpBB SQL Database ntwb_phpbbv3.zip (66kb) if someone else could test this to eliminate my setups out of the equation that would be grand.
Thanks for the test data! I gave it a go, and had no problems as well.
Creating a forum in the back end fails, technically it is created, but admin doesn't come back, you are left with a blank white screen.
Fatal error: Maximum function nesting level of '100' reached, aborting! in C:\xampp\htdocs\wp-includes\functions.php on line 2837
A fun little infinite loop was being created between bbp_update_forum_last_active_time() and BBP_Forums_Admin::attributes_metabox_save().
Has anyone tested post/page revisions with this? I have now :(
Blocker: 1925.3.patch breaks post revisions
I wasn't able to reproduce this. Whether bbPress was disabled, or active and patched/unpactched for post_modified, post revisions were properly created.
Replying to netweb:
Issues:
- Reply Revisions
- An edited reply works as expected, it does not bump the freshness and the edited content IS correct though:
- Reply post revisions are not created as posts in
wp_posts
- Reply revisions log
_bbp_revision_log
is not created inwp_postmeta
- All the functions for the above are there, it just doesn't happen, needs more investigation.
Reply Revisions work as expected using
/trunk
and do NOT work with1925.3.patch
I wasn't able to reproduce this either. Post revisions were created in the wp_posts table, and _bbp_revision_log was correctly updated.
#63
follow-up:
↓ 65
@
11 years ago
To be clear, we cannot use post_date
and should be using post_modified
. post_date
is reserved for the creation date, which is still important to bbPress and WordPress.
#64
@
11 years ago
- Summary changed from Optimize `_bbp_last_active_time` to use `post_date` instead to Optimize `_bbp_last_active_time` to use `post_modified` instead
Attached patch brings a few changes/additions. Most notably, is the fix for the infinite loop netweb stumbled upon. The other changes are just logic improvements and PHPdoc cleanup. I've also included both the importer updates, the tools.php updates, and a possible upgrade routine in this patch. This should, hopefully, make it easier to make sure all the necessary code is there during testing.
As far as the upgrade routine goes. I've intentionally not included the required db bump to make the upgrader run automatically. Wanted to make it just a little bit harder for people to add this to a production server. It definitely needs some stress testing. The queries shouldn't be too taxing, but my test install only has about ~5000 combined forums/topics/replies.
#65
in reply to:
↑ 63
@
11 years ago
Replying to johnjamesjacoby:
To be clear, we cannot use
post_date
and should be usingpost_modified
.post_date
is reserved for the creation date, which is still important to bbPress and WordPress.
It looks like I have not described myself well in my comments above, so here goes, per the original ticket:
Replying to vibol:
JJJ: Meta queries are always going to be a slow like this. There isn't an available index we can use here to avoid the meta query. We *could* hack in repurposing
post_datepost_modified
as the freshness time..
I understand the need to use post_modified
for 'THIS' solution with the proposed ideas and patches by @MZAWeb & @thebrandonallen
The patch I added with the comment 'Boom' (1925.05.diff) was because I had thought I had all the parts of the puzzle solved and there would be no need need for bbPress to use _bbp_last_active_time
or the need for bbPress modify any of the four post date fields in wp_posts
, the problem was I did not have all the parts to this idea and topics were sorted by topic creation date rather than 'freshness'.
Per my latest #comment:61 I believe I now have 'all the parts' for this solution to work perfectly. It is extremely similar this: (Note: The repair tools currently use the 'highest reply ID' to calculate the last reply.)
Replying to johnjamesjacoby:
We could also, maybe, try a subquery using the ID and post_parent indexes, but that only works if we can guarantee that newer replies will always have a higher ID. This *should* be the case, but it's hard to know for sure given the way the importer works, and how other forum software stores its data.
The key to this was the SQL COALESCE
query, it acts kind of like an inner join of the wp_posts
table to Return the first non-NULL argument
(MySQL Docs). This lets us for example get a list of topics and include the last reply id and/or date in that single query.
This allows us to include MAX ( 'post_date')
/ MAX ('ID')
/ MAX( 'reply'.'ID' )
type queries in the current bbp_has_forums
/ bbp_has_topics
queries. These new queries are near identical to the current repair tool queries that are used to rebuild these fields in wp_postmeta
. As we can actually include them in bbPress' primary queries we gain significant performance improvements and no longer need to store this data in a wp_postmeta.bbp_last_active_*
field (or wp_posts.post_modified
) field.
My 'proposed solution' enables bbPress to no longer need to store, use or query these wp_postmeta
fields:
(Each of following can be included in existing bbp_has_forums
, bbp_has_topics
queries as a single $WPDB
query of the wp_posts
table.)
_bbp_last_active_time
_bbp_last_active_id
_bbp_last_reply_id
_bbp_last_topic_id
Possibly also: (I haven't gone completely down this rabbit hole)
_bbp_forum_id
_bbp_topic_id
_bbp_reply_id
_bbp_reply_to
_bbp_status
The following will need to be retained in wp_postmeta
:
_bbp_anonymous_email
,_bbp_anonymous_name
&_bbp_anonymous_website
_bbp_author_ip
_bbp_forum_subforum_count
_bbp_forum_type
_bbp_group_ids
_bbp_reply_count
,_bbp_reply_count_hidden
,_bbp_total_reply_count
_bbp_topic_count
,_bbp_topic_count_hidden
,_bbp_total_topic_count
_bbp_voice_count
_bbp_revision_log
The following repair tools would no longer be required:
(bbPress would be using WP's native and original untouched timestamp and date wp_posts
fields)
- Recalculate the parent topic for each post
- Recalculate the parent forum for each post
- Recalculate last activity in each topic and forum
There is a fair amount of code and testing required for all of the improvements above so I would think if we removed _bbp_last_active_time
and _bbp_last_active_id
for starters we can then go from there.
In summary, is what I have outlined above good idea? I believe it is.
- Comparable performance to the
post_modified
solution. - We are not modifying WordPress Core table database field for our own purpose
- Future proof if WordPress Core 'did' decide to do 'something' in the future with
post_modified
- For starters two less bbPress
wp_postmeta
fields
Am I bonkers? crazy? nuts? If I'm not I'll upload a patch, if I am lets go with the post_modified
solution :)
#66
@
11 years ago
You're not bonkers if it works ;), but I think the only way to see if it works, is to see the code, and see it in action.
#67
follow-up:
↓ 81
@
11 years ago
'Proof of Concept' patch ;)
Some things to note:
- function
bbp_topic_freshness_override
is pretty hacky and does what is needed for Proof of Concept
- Only works for a 'single forum' list of topics sorted by latest reply (
bbp_has_topics
)- (Latest replies
wp_posts.post_date
or the topicswp_posts.post_date
if the topic has no replies)
- (Latest replies
- Other conditions will need some different SQL queries eg search, widgets etc
- Still a bit to do here to test all the things and implement the same on the 'forum' side for 'last active topic'
- Support for legacy uses of custom forum, topic or reply 'orderby' and 'order' ASC/DESC
- eg
bbp_has_topics( array( 'orderby' => 'title', 'order' => 'ASC' ) )
- eg
#68
@
11 years ago
- Cc scott@… added
I'm seeing major slow downs on two specific areas:
# Time: 140212 17:48:21 # Query_time: 2.377407 Lock_time: 0.000123 Rows_sent: 15 Rows_examined: 494974 SET timestamp=1392227301; SELECT SQL_CALC_FOUND_ROWS wp_2_posts.ID FROM wp_2_posts INNER JOIN wp_2_postmeta ON (wp_2_posts.ID = wp_2_postmeta.post_id) WHERE 1=1 AND wp_2_posts.post_parent = 3846935 AND wp_2_posts.post_type = 'topic' AND (wp_2_posts.post_status = 'publish' OR wp_2_posts.post_status = 'closed') AND (wp_2_postmeta.meta_key = '_bbp_last_active_time' ) GROUP BY wp_2_posts.ID ORDER BY wp_2_postmeta.meta_value DESC LIMIT 0, 15 /* In [/wp-content/plugins/bbpress/includes/topics/template.php:203] */;
And:
# Time: 140212 17:31:07 # Query_time: 8.857887 Lock_time: 0.005828 Rows_sent: 1 Rows_examined: 1603732 SET timestamp=1392226267; SELECT COUNT(ID) FROM wp_2_posts WHERE post_parent IN ( 8811487,8811483,8811482,…,8717184,8708196 ) AND post_status = 'publish' AND post_type = 'reply'; /* In [/wp-content/plugins/bbpress/includes/forums/functions.php:1507] */;
This specific database has millions of topics/replies, with a pretty heavy meta table (bbPress meta stuff, nothing custom).
#69
@
11 years ago
Thanks Scott for the additional data. This ticket is geared towards addressing those two issues specifically, amongst several others. Stephen's been doing a great job at experimenting here, and we'll continue to update with any progress that's made.
#70
@
11 years ago
Is there any specific patches submitted here that someone as desperate as me can look towards for immediate gains?
#71
@
11 years ago
I my testing 1925.4.diff is stable, but it's vastly different than what netweb is currently exploring. You want do irreparable harm to your database, but it will mean the co-opting of the post_modified(_gmt) fields in for topics and forums. If netweb's approach gets the green light over the post_modified route, you can create/and run a query that resets your post_modified(_gmt) fields back to their original values if the data accuracy of those fields is important to you. I know you know this, but, please, please, pretty please test it before you go live :). If you decide to test it you'll want to set the upgrade routine to compare at a lower db version than what you currently have installed, but higher than the db version for the last upgrade routine. I intentionally made a barrier to entry on that one. Minor, but intentional.
netweb's 1925.alt.01.diff looks promising, but it, currently, only covers topic sorting.
#72
@
11 years ago
So I took care of a couple of the slow queries in a temporary way. The first ignores the reply counting for forums themselves (we don't care to show reply numbers). The second handles # of voices counting for a topic, in which we simply just want to get the current value from _bbp_voice_count and return that, instead of trying to calculate it over and over each time a new topic/reply is added.
function my_bbp_optimize_queries( $query ) { if ( false !== strpos( $query, 'SELECT COUNT(ID) FROM ' ) && false !== strpos( $query, ' ) AND post_status = \'publish\' AND post_type = \'reply\'' ) ) { $query = 'SELECT "0" AS `the_count`'; } elseif ( false !== strpos( $query, 'SELECT COUNT( DISTINCT post_author ) FROM ' ) && false !== strpos( $query, ' AND post_status = \'publish\' AND post_type = \'reply\' ) OR ( ID = ' ) && false !== strpos( $query, ' AND post_type = \'topic\' )' ) ) { preg_match( '/ AND post_status = \'publish\' AND post_type = \'reply\' \) OR \( ID = (\d+) AND post_type = \'topic\' \)/', $query, $topic ); $count = (int) get_post_meta( $topic[ 1 ], '_bbp_voice_count', true ); if ( empty( $count ) ) { $count = 1; } $query = 'SELECT ' . $count . ' AS `the_count`'; } return $query; } add_filter( 'query', 'my_bbp_optimize_queries' );
#73
@
11 years ago
FYI, my solution has some flaws for voice count, it could be better, but you can still see what I was after.
This ticket was mentioned in IRC in #bbpress-dev by netweb. View the logs.
11 years ago
#75
@
10 years ago
I just bumped into Scott Reilly's (coffee2code) Last Contacted plugin and this uses a custom orderby coalesce statement.
The following two functions are a good reference of the proposed implementation of adding and removing the add_filter( 'posts_orderby'...
filter conditionally
- https://plugins.trac.wordpress.org/browser/last-contacted/trunk/last-contacted.php#L690
- https://plugins.trac.wordpress.org/browser/last-contacted/trunk/last-contacted.php#L772
I'll take a closer look once we have some unit tests added to test this ticket.
This ticket was mentioned in IRC in #bbpress-dev by netweb. View the logs.
10 years ago
This ticket was mentioned in Slack in #bbpress by thebrandonallen. View the logs.
10 years ago
This ticket was mentioned in Slack in #bbpress by netweb. View the logs.
9 years ago
#80
@
9 years ago
Hi there,
I've been following this issue for a while now, and understandably, I see this might take a bit more time. I'm working with a bbPress site with ~55K topics and ~275K replies. We started to feel the weight of this a few months ago, but now it's really noticable.
Out of all the patches here, is there one recommended for an interim fix? Something that is more in line with what is planned to be the official solution. Our forums are only getting bigger so I'm seriously considering patching it myself. It would be great to know where's the best place to start.
Thanks!
#81
in reply to:
↑ 67
@
9 years ago
Any other progress here? I'd be happy to help with any testing.
@netweb and @thebrandonallen, I've been doing some testing with these patches in a local environment working with data copied from our production site. Before any modifications, my average query time is right at 2s
Average query times:
1925.alt.01.diff - 950ms
1925.4.diff - 280ms
For the time being I'm going to setup some short lived page caching on my forum index, but I've got to make a call for production soon and I'm leaning towards biting the bullet and running the db migration for 1925.4.diff
#82
@
9 years ago
I don't know that I can recommend either at the moment. While the results are impressive, after some discussion, I don't think we're going to take the approach in 1925.4.diff. The 1925.alt.01.diff is more proof-of-concept at the moment, but is likely the approach we'll be taking. Until fairly recently, we haven't had the framework to test patch like this. So, that being said, knowing it hasn't been thoroughly tested, if you wanted to use it, you would need to make sure you change all the references to wp_posts
to match your actual table name. Even, better would be to make it a reference to wpdb::posts
. For now, I would skip the patch, and use an opcache and/or object caching PHP extension, or some sort of page caching as you mentioned.
#83
@
8 years ago
Via https://bbpress.org/forums/topic/bbpress-for-huge-forums-capable/page/2/#post-176909
I have a website that is 1 yo but already have 27 million posts and is only online because of the 1925.4.diff that bbpress staff/helpers provided here:
Even with the patch… I have some trouble with long threads using too much cpu… so I’m always closing threads because of that.
I also got in trouble with high cpu load when we have too many topics under the same forum… because of this problem.. from time to time I create new forums and make the old one a “museum”.
My CPU consumption usually varies from 8 to 30. 40-50 on peak days.
Without doing this “hacks” my website can’t survive… because the cpu consumption goes up to 500 until everything crashes.
So... We're sort of stuck. The approach you quoted from our previous conversation is the only available index in the posts table that makes sense to try to use. We could also, maybe, try a subquery using the ID and post_parent indexes, but that only works if we can guarantee that newer replies will always have a higher ID. This *should* be the case, but it's hard to know for sure given the way the importer works, and how other forum software stores its data.
It's a problem not easily solved due to our limited schema and a lack of indexes on the postmeta table. Happy that you're around looking into it too.