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

Improve performance rendering the Last Order Date column value on the WooCommerce > Subscriptions list table #782

Open
mattallan opened this issue Feb 13, 2025 · 2 comments · May be fixed by #819
Assignees
Labels
category: performance The issue/PR is related to performance. needs prioritisation Issue needs a priority label set (or existing priority label reviewed). woocommerce-subscriptions-core

Comments

@mattallan
Copy link
Contributor

Description

On the WooCommerce > Subscriptions list table, we display a column for each Subscription's last order date:

Image

Each date value displayed in the Last Order Date column is calculated by:

  1. Getting the last order linked to the subscription
  2. To fetch this, we need to first fetch the list of parent, renewal & switch order IDs linked to the subscription. We store this list of related order IDs on the subscription in a cache, but if that cache doesn't exist, we run separate order meta queries to fetch all orders that are linked to this subscription.
  3. With the list of order IDs, we then take the latest order ID, and read it into memory
  4. Get this order's date_created value

Needing to calculate this value for every subscription rendered on the WooCommerce > Subscriptions page adds unnecessary performance burden, not to mention the queries required to sort by this column as well.

Possible Solutions

  1. On large sites, don't populate the Last Order Date column

This is not an ideal solution, but more of a temporary short-term fix.
In Subscriptions we have a helper function (see wcs_is_large_site()) that categorises large sites as those with 3,000+ subscriptions. To increase the performance of this page, we could prevent this column from being loaded/calculated on large subscription stores. If stores want it back, we could add a filter to include it.

Important

Sorting by this column allows merchants to find the most recently renewed subscriptions, so it might introduce a bad experience to merchants if we just remove this column entirely.

  1. Store the last_order_date as metadata on the subscription.

For our other date fields (i.e. next payment, trial end, end date etc), even though these dates are already calculated and stored in Action Scheduler as pending scheduled actions, we also store these values as metadata on the subscription and keep these values in sync.

Image

The idea of this solution would be to calculate and store this value in metadata and then as part of our related orders handling, we could keep this value in sync.

Storing this value as metadata would reduce the need to fetch and instantiate the last order, but also improve our queries that sort the table by last order date.

@mattallan mattallan added category: performance The issue/PR is related to performance. needs prioritisation Issue needs a priority label set (or existing priority label reviewed). labels Feb 13, 2025
@james-allan
Copy link
Contributor

@mattallan am I correct in that this issue is related to the discussion we had around fetching a subscription's latest order.

ie There are multiple get_last_order() calls on the list table and each one of those is leading to a direct read of the subscriptions metadata from the DB. For example you shared this screenshot:

Image

Duplicate meta read queries from Query Monitor all for the same subscription ID

I wonder if we should take a serious look at our approach of reading and saving related order caches directly into the database.

I understand why we took that approach at the time, however, fetching a subscription's related orders without hitting the database each time seems like the best overall approach. It would provide broader performance benefits.

IIRC, one of the main reasons we chose to save and read related order caches directly from the database was to maintain a single source of truth when multiple instances of a subscription exist in memory. Relying on a last order date stored on the object essentially bypasses that, which could lead to consistency issues. eg a related orders list out of sync with the last order date because one's reading from the database and the other is from memory.

If we're going to store the last order date and read it on meta reads, why not do the same for the related orders. The only reason I can imagine is we plan on keeping the raw related order caches as direct reads/writes, but for less critical purposes (displaying a date in this screen) we can use a cached value stored on the object.

@james-allan
Copy link
Contributor

IIRC, one of the main reasons we chose to save and read related order caches directly from the database was to maintain a single source of truth when multiple instances of a subscription exist in memory.

To expand on this, it's important to note that these could be multiple instances of the same subscription in memory—either within a single request or across multiple requests, including async ones.

For example, if a renewal is processing in an async request while another request is modifying the same subscription, a related order list that was read into memory once could lead to a race condition. One instance may end up working with an outdated related orders cache.

Reading and writing directly to the database prevents this issue by ensuring each request operates with the most up-to-date data.

With that in mind, I'm starting to come around to the idea of adding a last_order_date meta row for use cases like displaying the date or for reporting, where querying the database in real time isn’t strictly necessary.

@alefesouza alefesouza self-assigned this Mar 13, 2025
@alefesouza alefesouza linked a pull request Mar 21, 2025 that will close this issue
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: performance The issue/PR is related to performance. needs prioritisation Issue needs a priority label set (or existing priority label reviewed). woocommerce-subscriptions-core
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants