Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Radical change: high-volume + WPML order fix #18

Closed
wants to merge 1 commit into from

Conversation

rmpel
Copy link

@rmpel rmpel commented Apr 13, 2018

Hello Jake Goldman and cohorts :)

I'd like to submit to you a very radical change. If you are not interested, that's ok too, we'll be maintaining a fork ourselves :)

The change:

No longer use wp_update_post in batches to update the menu_order. While this works perfectly in low-volume sites without WPML, this breaks in high-volume sites with WPML (500+ posts with translations).
It got so bad in the end that even a directly-in-database change of order 64 to 10, followed by an edit (without change) + save of the post in WP resulted in revert to order 64.

Now use a few database queries to do the same thing. this is about 30 times faster, and solves the problem of order-changes not being stored.

I realise this is a very drastic change that might break other things, but perhaps you can use it as inspiration at least. For us, this solves the problem, but we've only tested this in 1 website.
I also realise that this patch does not result in nice code (old code kept as comment), but again, it's intended as inspiration :)

Thank you for your time!

No longer use wp_update_post in batches to update the menu_order. While this works perfectly in low-volume sites without WPML, this breaks in high-volume sites with WPML (500+ posts with translations).
It got so bad in the end that even a directly-in-database change of order 64 to 10, followed by an edit (without change) + save of the post in WP resulted in revert to order 64.

Now use a few database queries to do the same thing. this is about 30 times faster, and solves the problem of order-changes not being stored.
@helen
Copy link
Contributor

helen commented Apr 13, 2018

Hi @rmpel - thanks for opening this PR. Per the contributing guidelines, could you also open a separate issue describing the problem (using WP's update functions can be slow, especially when there are a lot of actions being fired)? That way we can discuss possible solutions to time out issues and how to handle missing hooks when using alternate methods, etc. in the issue to keep all the decision-making context in one place and then if we do decide to go the route of direct queries, we can do a code review here in the PR.

@rmpel
Copy link
Author

rmpel commented Apr 13, 2018

see #19

@jeffpaul jeffpaul added this to the 2.4.0 milestone May 30, 2019
@jeffpaul jeffpaul removed the request for review from adamsilverstein February 3, 2020 18:30
@jeffpaul jeffpaul modified the milestones: 2.4.0, Future Release Feb 11, 2020
@jeffpaul jeffpaul requested a review from dkotter June 13, 2022 18:20
@jeffpaul
Copy link
Member

@dkotter tagging you here for review with the caveat that I'm considering dropping the Support Level here to Stable and such a major change like this might be more than we are likely to enact. Also noting this PR is several years old (sorry folks about that), so worth considering a couple different options before we actually get into deep code review here on any path towards merge/release. So, let me know what you think before proceeding too far on this PR.

@dkotter
Copy link
Collaborator

dkotter commented Jun 13, 2022

With how long this has been open, there will for sure need to be some refreshing of this PR. I think this addresses a legitimate potential issue but I don't think the approach taken here is one we would want to go with. I'd suggest we close this out but leave the issue open and we can always consider coming up with a proper solution for this.

@jeffpaul jeffpaul linked an issue Jun 13, 2022 that may be closed by this pull request
@jeffpaul
Copy link
Member

@rmpel apologies for the long drawn-out review here, but going to close this PR but as @dkotter noted we do want to consider a fresh, alternate approach to resolving this as part of the plugin. I appreciate your help in opening the PR and associated issue and hope we can find a way to get this to resolution and will ensure we credit you in any future merged change on the topic, thanks!

@jeffpaul jeffpaul closed this Jun 13, 2022
@jeffpaul jeffpaul removed this from the Future Release milestone Jun 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reordering fails with high volume of posts and WPML
4 participants