Yoast / wordpress-seo

Yoast SEO for WordPress
https://yoast.com/wordpress/plugins/seo/
Other
1.78k stars 894 forks source link

let users choose to disable bulk title editor #837

Closed lisaleague closed 9 years ago

lisaleague commented 10 years ago

The database query can be inefficient for large tables and can require a VPS with high resources allocated, which most users do not have access to. Causes WordPress database error when visiting the Bulk title editor page in the admin. No other errors in the error log until visiting this page, then clears. Visit bulk title editor again, and creates issues. yoast wordpress seo bulk title editor qpractice com wordpress

jrfnl commented 10 years ago

Pinging @Faison - fancy having a look at this ? I remember seeing another thread about this, can't find it atm, but probably at the WP forums. It mentioned the bulk editor pages going hay-wire on a site with over 10.000 posts.

Some questions which come to mind for me:

Faison commented 10 years ago

Hi @lisaleague and @jrfnl,

Woah, that's nasty looking! I'm sorry you've had to deal with that :( Anyways, you can disable the Bulk Editors by removing the user capability wpseo_bulk_edit for now. There is a user capability plugin, called Members, which allows you to enable and disable capabilities for all user roles, so that might be a temporary option for you, @lisaleague.

To answer your questions, @jrfnl, the results are paged. The problem actually lies in how the database query is built to handle Post Types that the user has full and limited capabilities to. If you look at the DB subquery here, It runs a SELECT query on the posts table twice, once for post types that you have full capabilities for, and once for post types that you have limited capabilities for.

Looking at the screenshot provided by @lisaleague, you can see that the second half of the subquery is useless for her current user: ...WHERE post_type IN ('').... So I can add a conditional in there that removes one half of the subquery if there are no post types for the current user. And while this will make the query more efficient sometimes, I'm not sure it will fix the problem completely.

I believe the best I can say right now is that I can work at continuing to optimize the database queries to help relieve this issue for websites with huge amounts of posts. However, If this issue can't be fixed with query optimization, then perhaps we can look into deviating from the standard WordPress workflow (In other words, looking into using ajax and search forms).

Thanks, Faison

offordscott commented 10 years ago

I know the devs are usually busy and work for free. I look forward seeing if anyone can take a stab at contributing code to address this interesting issue.

lisaleague commented 10 years ago

Hi @Faison,

Not sure that I'm fully understanding the Members suggestion. Really no one has access to this but me and my assistant if that's what you mean. I have a membership site and several hundred active are all subscriber level so not encountering this - but I don't want my DB affected by this in anyway.

Here's a typical query that failed, but I'm hoping my developer can comment in on this later as he can provide a bit more insight. I think you'll start seeing more of these.

SELECT ID, post_title, post_type, meta_value AS seo_title, post_status, post_modified
FROM (
    SELECT *
    FROM wp_posts
    WHERE post_type IN ('post', 'page', 'attachment', 'qa_faqs', 'course_unit')
    UNION ALL
    SELECT *
    FROM wp_posts
    WHERE post_type IN ('') AND post_author = 169
)sub_base
LEFT JOIN (
    SELECT *
    FROM wp_postmeta
    WHERE meta_key = '_yoast_wpseo_title'
)a
    ON a.post_id = ID
    WHERE post_status IN ('publish', 'future', 'draft', 'pending', 'private', 'trash')
ORDER BY post_title ASC

[Redacted by @jrfnl: I've taken the liberty of making the SQL code readable]

jdevalk commented 10 years ago

Ouch that's a rather hefty query. Might actually be a lot more efficient to split it into several smaller ones, MySQL would need to keep everything in memory to be able to do this. Since there's no LIMIT in the query, on a large DB this would go nuts... In fact, limiting properly, just getting ALL posts by the post_author and then doing the rest of the looping in PHP might actually be a lot more efficient.

Faison commented 10 years ago

Thanks for the suggestion @jdevalk, I'll keep that in mind when I spend some time optimizing tonight :)

jrfnl commented 10 years ago

@Faison I presume there is no need to mention the use of EXPLAIN ? ;-)

lisaleague commented 10 years ago

Thanks @jrfnl Juliette for starting the ball rolling on this, hopefully there will be a fix:)

jdevalk commented 10 years ago

These queries have been massively improved by @andizer. @lisaleague would you fancy testing the current GitHub master to see if that improves things for you?

jdevalk commented 10 years ago

(BTW I did remove the bulk editor for contributors, only editors and up now have the feature)

lisaleague commented 10 years ago

Hey @jdevalk - Issue actually turned out to be a conflict with WPE mu plugin somehow causing duplication of entries of each post. But will be glad to test next update:)

lisaleague commented 10 years ago

@jdevalk I should add I'd be happy to put this on my staging and test, but It's too critical now for me to implement with active members during season on the live site until after end of October.

jdevalk commented 10 years ago

Well even a test on staging would be appreciated :)

lisaleague commented 10 years ago

Will do.

offordscott commented 9 years ago

What are you looking to test exactly? I'm sure I can help.

tacoverdo commented 9 years ago

No more reports received for this problem. It's safe to say the optimizations by @andizer fixed the issues.