wp-e-commerce / WP-e-Commerce

WP eCommerce - The most popular independent eCommerce platform for WordPress
https://wpecommerce.org
GNU General Public License v2.0
216 stars 216 forks source link

Large Number of Products Crashes Wordpress 3.5 Admin and possible other places in WP 3.5 #136

Open JeffPyeBrook opened 11 years ago

JeffPyeBrook commented 11 years ago

Wordpress 3.5 by tries to sort hierarchical post types by menu_order and then by title. When trying to edit posts in the admin area, as well as in other places in WP) and your WPEC has a large number of wpsc-product posts there will be a crash.

The attempt to retrieve the database records and sort them by menu order takes more than a reasonable amount of database and/or php memory, and more than a reasonable amount of processing time. Crash can be caused with php memory at 384 and 1000 products, and 300 second timeouts.

A short term fix is the add the filter below to either WPEC or to your functions.php.

function fix_too_many_wpsc_products_query_crash( $query ) {
    if (
        ( isset( $query->query_vars['post_type'] ) && ( $query->query_vars['post_type'] == 'wpsc-product' ) )
            && ( isset( $query->query_vars['posts_per_page'] ) && ( $query->query_vars['posts_per_page'] == -1 ) ) 
                && ( isset( $query->query_vars['orderby'] ) && ( $query->query_vars['orderby'] == 'menu_order title') )  
            )
    {
            $query->query_vars['orderby'] = trim( preg_replace( '!menu_order!', "", $query->query_vars['orderby'] ) );
            $per_page = 'edit_' .  $query->query_vars['post_type'] . '_per_page';
            $query->query_vars['posts_per_page'] = (int) get_user_option( $per_page );
            $query->query['orderby'] = $query->query_vars['orderby'];
            $query->query['posts_per_page'] = $query->query_vars['posts_per_page'];
            $query->query['posts_per_archive_page'] = $query->query_vars['posts_per_page'];     
    }   
}

add_action('pre_get_posts', 'fix_too_many_wpsc_products_query_crash', 1);

Over the longer term some consideration should be given as to whether products and thier variations are best represented as a hierarchical post type.

JustinSainton commented 11 years ago

I might be a bit confused by this - I get it crashing when sorting by menu_order/post_title - but are you telling me you'd have it display all 1,000+ posts without pagination? That's going to be incredibly slow regardless of sorting.

benhuson commented 11 years ago

I also get it crashing (certainly in previous versions of WPEC) on several sites I run when sorting by drag and drop in the admin. If I remember rightly The reason was when drag and drop was enabled the admin showed all products rather than paginating, presumably so you could drag and drop, and that was too memory intensive.

With my specific issue I hacked it to disable drag and drop in the admin and cobbled together a plugin similar to CMS Page Order (http://wordpress.org/extend/plugins/cms-page-order/) that allowed you to only sort products simply at a category level in a list form.

garyc40 commented 11 years ago

@JustinSainton

I might be a bit confused by this - I get it crashing when sorting by menu_order/post_title - but are you telling me you'd have it display all 1,000+ posts without pagination? That's going to be incredibly slow regardless of sorting.

Even with pagination enabled, the mysql server will still have to sort the whole table before it can return a subset of it. So with 1000+ posts, pagination enabled, this could still cause some performance issues.

If you look at the wp_posts table, the column menu_order is not even indexed. So sorting by that column is naturally slow. Even when sorting by drag and drop is disabled, WordPress will still default the sorting order of the admin page to menu_order, because wpsc-product is registered as a hierarchical post type.

We should disable that default menu_order sort when "drag and drop" is disabled, and default to sorting by ID or title instead (I'd say sorting by ID is more sane than sorting by title though).

@SparkleGear Thanks for the code snippet. If possible, please take our feedback into your consideration and reformat your snippet into a pull request. I'll be happy to review and commit it.

benhuson commented 11 years ago

Why is menu_order not indexed in WP? Surely the whole point of menu_order is that you'd want to order by it?

garyc40 commented 11 years ago

I have no idea why either and that question also has not been raised anywhere else as far as I know. @JustinSainton would you mind asking @nacin or @markjaquith about that on Twitter or IRC?

JustinSainton commented 11 years ago

Brought it up on IRC - @staylor answered with the following -

wonderboymusic : menu_order just recently got added as a query_var in 3.5 wonderboymusic : I add an index

I remember the trac ticket, I think it's this one - http://core.trac.wordpress.org/ticket/21618

I don't think that actually fixes our issue though.

JeffPyeBrook commented 11 years ago

@garyc40 commented

and default to sorting by ID or title instead (I'd say sorting by ID is more sane than sorting by title though).

Consider sorting by revision date in descending order. Many times from a workflow perspective the post you are most likely to want to change is the post that was most recently changed.

benhuson commented 11 years ago

I agree if Drag-n-Drop disabled then menu_order should not be used.

As @SparkleGear mentioned, sorting by most recent revision date would be most useful (probably) but if that introduces too many queries then title or publish date would be next preferable - ID is not useful at all.

JustinSainton commented 11 years ago

All this talk about menu_order makes me long for the ability to ditch menu_order altogether in favor of a better solution that allows you to order products one way in a specific category, and a different way in a different category, and have it behave exactly as you'd expect it to.

Le sigh.

benhuson commented 11 years ago

@JustinSainton I guess you're thinking of a post meta order approach or something so a product can have multiple order locations? I've always thought it a bit buggy that if a product exists in too categories you couldn't order them differently.

JustinSainton commented 11 years ago

@benhuson - Nope, not really. Using the term_order column in the term_relationships table

JustinSainton commented 11 years ago

Testing it out on a client site, going to see how it goes.

JustinSainton commented 11 years ago

As I'm digging through it - I'm not necessarily seeing the two sort types as mutually exclusive. There are naturally many times when product categories aren't used (e.g. there's only one or the sort has never been done or will never be done when filtered by category) or aren't displayed (any non-categorical listing).

JustinSainton commented 11 years ago

As an aside, there was a bit of talk, with very little core dev feedback, re: menu_order index

http://core.trac.wordpress.org/ticket/9864#comment:14

benhuson commented 11 years ago

Not how how much we use get_term_by but this should help reduce a few queries when implemented in WordPress core? http://core.trac.wordpress.org/ticket/21760

It certainly gets called when creating products links with Hierarchical URL but not sure how much it may help with the admin.

JustinSainton commented 11 years ago

We use get_term_by 18 times, primarily on the front-end. If #21760 makes it into core, it will definitely help. But until then (and even if by some miracle, it makes it into 3.6, that won't be our minimum required version until 3.8), it would be nice to have a helper function like this.

benhuson commented 11 years ago

Ah yes, I knew I had seen that discussion somewhere but couldn't find it - thought it was still on an open ticket.

benhuson commented 11 years ago

@JustinSainton Did you get anywhere with testing ordering using term_order column in the term_relationships table so that you could order differently in different categories.

This issue has been brought up again by a couple of clients and it would be good to know if your testing seemed to suggest that might be a good solution?

As you mentioned, there are sort cases where categories arne't used which need to be considered. Does this warrant it's own ticket?

JustinSainton commented 11 years ago

@benhuson - Yeah, it's definitely a separate issue from the original post here, I'd say it warrants its own ticket.

Worth noting a couple things -

1) WordPress currently has no exposed API for term_order, so we'd be doing direct queries, at least for the immediate future. Would need to do some careful caching around this.

2) Optimising for the lack of an index on term_order (same issue there as menu_order) can be done a few ways. I've done it via FORCE INDEX on the direct query, caching that query, then grabbing the IDs and sorting by ID - a bit hacky, but it worked really, really well. A few ways to skin this cat though.

benhuson commented 11 years ago

FYI I asked the question about menu_order indexing on the trac ticket. I'm no database expert and don;t understand fully how indexes work but Denis-de-Bernardy gave a fairly comprehensive reply: http://core.trac.wordpress.org/ticket/9864#comment:25

JustinSainton commented 11 years ago

Punting for 3.9. Lots of work to be done here and it likely won't occur in a minor release.

instinct commented 11 years ago

Do we know what's causing this?

Sent from my iPhone

On 30/04/2013, at 7:02 AM, JustinSainton notifications@github.com wrote:

Punting for 3.9. Lots of work to be done here and it likely won't occur in a minor release.

— Reply to this email directly or view it on GitHub.

JustinSainton commented 11 years ago

Yep. Lots of variations and variation sets end up creating thousands of products, causing the database to choke.

Thanks,

M. Justin Sainton Zao Web Design, LLC Principal / Project Manager http://www.zaowebdesign.com (c) 971.222.6330 (f) 1.413.723.0396

On Mon, Apr 29, 2013 at 1:38 PM, Instinct Entertainment < notifications@github.com> wrote:

Do we know what's causing this?

Sent from my iPhone

On 30/04/2013, at 7:02 AM, JustinSainton notifications@github.com wrote:

Punting for 3.9. Lots of work to be done here and it likely won't occur in a minor release.

— Reply to this email directly or view it on GitHub.

— Reply to this email directly or view it on GitHubhttps://github.com/wp-e-commerce/WP-e-Commerce/issues/136#issuecomment-17192649 .

JeffPyeBrook commented 11 years ago

Justin is of course correct in that the large number of variation sets creating the large number of products causing issues with the database.

I have found that there are two big problem areas. First when the variations are created it is a very database intensive task, and causes key tables to lock for a measurable period of time. If a web site is even a little busy while the locks are in place queries quickly queue up and a database might have trouble recovering gracefully.

The second, and more frequent, issue with the large number of variations is a problem when our WPEC themes access product sets in a way that emulates the WordPress loop. The queries that WPEC sends to the database are usually a little more complicated than the typical WordPress loop queries because they almost always have a category and frequently include taxonomy terms. The queries require a database to positionally access the result set, which is expensive, and may not be entirely cached. The positional accesses also seem to require table locks, so queries start to queue up, and even a slight busy site could slow down to an unusable state or even deadlock.

However, it is possible to do some work in the WPEC theme get the performance you need. We are at over 10K products and 90K variations, and our on server page generation times are typically around 800ms. That's still slow, but it's getting better as we spend more time working out some of the tuning issues.

benhuson commented 11 years ago

Also, one particularly bad area of the admin is when you have categories with many products in and set the product order to drag and drop. This alters the products list view in the admin to display all posts (when filtering by category) rather than page which then creates the issues mentioedn by @JustinSainton and @SparkleGear above.

JustinSainton commented 11 years ago

Interesting infinite scroll approach to the List_Table class in admin

https://gist.github.com/bradyvercher/6102036

Related: #238