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

Concurrent checkouts can cause negative inventory for items with backorder disabled #12467

Closed
TWarszawski opened this Issue Nov 25, 2016 · 27 comments

Comments

Projects
None yet
@TWarszawski
Copy link

TWarszawski commented Nov 25, 2016

EXPLANATION OF THE ISSUE

When two customers check out concurrently for the same product (with backorder disabled), both checkouts may complete successfully when this would cause the inventory to become negative. This negative inventory is reflected in the admin console.

It seems likely that a solution to #5966 would resolve this issue, but it also seems that this could be fixed without implementing the functionality proposed in #5966.

STEPS TO REPRODUCE THE ISSUE

  1. Start site, create two customers, create/pick test product and allocate some stock.
  2. Both customers add the product to their carts such that each cart individually is under the available stock, but combined they exceed the available stock.
  3. Perform a checkout concurrently, making sure both customers finish checkout (click the 'Place Order' button) as close to the same time as possible.

We have reproduced this behavior on a single machine, by performing the above steps by simulating one customer in one browser window and another customer in a second browser window.

Expected Result:
One of the two checkouts fails to complete.

Actual Result:
Both checkouts succeed and the final inventory is negative.

SYSTEM STATUS REPORT

WordPress Environment

Home URL:
Site URL:
WC Version: 2.6.8
Log Directory Writable: ✔
WP Version: 4.6.1
WP Multisite: –
WP Memory Limit: 256 MB
WP Debug Mode: –
WP Cron: ✔
Language: en_US

Server Environment

Server Info: Apache/2.4.18 (Unix) OpenSSL/1.0.2g PHP/5.6.19 mod_perl/2.0.8-dev Perl/v5.16.3
PHP Version: 5.6.19
PHP Post Max Size: 128 MB
PHP Time Limit: 30
PHP Max Input Vars: 1000
cURL Version: 7.45.0
OpenSSL/1.0.2g

SUHOSIN Installed: –
Max Upload Size: 128 MB
Default Timezone is UTC: ✔
fsockopen/cURL: ✔
SoapClient: ✔
DOMDocument: ✔
GZip: ✔
Multibyte String: ✔
Remote Post: ✔
Remote Get: ✔

Database

WC Database Version: 2.6.8
:
woocommerce_sessions: ✔
woocommerce_api_keys: ✔
woocommerce_attribute_taxonomies: ✔
woocommerce_downloadable_product_permissions: ✔
woocommerce_order_items: ✔
woocommerce_order_itemmeta: ✔
woocommerce_tax_rates: ✔
woocommerce_tax_rate_locations: ✔
woocommerce_shipping_zones: ✔
woocommerce_shipping_zone_locations: ✔
woocommerce_shipping_zone_methods: ✔
woocommerce_payment_tokens: ✔
woocommerce_payment_tokenmeta: ✔
MaxMind GeoIP Database: ✔

Active Plugins (1)

WooCommerce: by WooThemes – 2.6.8

Settings

Force SSL: –
Currency: USD ($)
Currency Position: left
Thousand Separator: ,
Decimal Separator: .
Number of Decimals: 2

API

API Enabled: ✔

WC Pages

Shop Base: #4 - /shop/
Cart: #5 - /cart/
Checkout: #6 - /checkout/
My Account: #7 - /my-account/

Taxonomies

Product Types: external (external)
grouped (grouped)
simple (simple)
variable (variable)

Theme

Name: Twenty Sixteen
Version: 1.3
Author URL: https://wordpress.org/
Child Theme: – If you're modifying WooCommerce on a parent theme you didn't build personally
then we recommend using a child theme. See: How to create a child theme

WooCommerce Support: ✔

Templates

Overrides: –

@mikejolley

This comment has been minimized.

Copy link
Member

mikejolley commented Feb 6, 2017

@coderkevin suggested using a queue. How do you see that working Kevin?

@coderkevin

This comment has been minimized.

Copy link
Member

coderkevin commented Feb 6, 2017

The queue could be implemented a few ways, maybe even in a custom orders table by adding a new order status to indicate the order quantities were successfully secured for the order.

In any case, there would also need to be a new UI flow to show the user upon checkout if the quantities couldn't be secured for their order.

@mikejolley

This comment has been minimized.

Copy link
Member

mikejolley commented Feb 13, 2017

The queue could be implemented a few ways, maybe even in a custom orders table by adding a new order status to indicate the order quantities were successfully secured for the order.

I wonder if the sessions table would suffice? But still, how can we ensure 2 orders coming in at the same time are not checking and updating this table at the same time? Locks?

@coderkevin

This comment has been minimized.

Copy link
Member

coderkevin commented Feb 14, 2017

The best way to do it is to have a single background job do the fulfillment (e.g. inventory decrementing) of the orders. Then it's a serial operation. Just because an order is placed doesn't mean it will be fulfilled, so we'll need a status to show that, too.

@coderkevin

This comment has been minimized.

Copy link
Member

coderkevin commented Feb 14, 2017

By the way, this is where back-orders should be handled as well.

@mikejolley

This comment has been minimized.

Copy link
Member

mikejolley commented Feb 15, 2017

@coderkevin Back-order stock works in the same way as regular stock so that shouldn't be an issue here.

I guess I'm just unsure how this 'job' runs, technically. The only way I can think of to only do this one-at-a-time would be table locks on the database https://dev.mysql.com/doc/refman/5.7/en/lock-tables.html

@franticpsyx

This comment has been minimized.

Copy link
Member

franticpsyx commented Feb 15, 2017

Not sure if you've been able to replicate but I can confirm this is an issue I have seen in the past.

It mostly affects high-traffic stores and stores which sell highly-demanded items which run out of stock quickly. Used to run a store where stock replenishment was announced in advance and done at a predetermined time -- customers would wait for that time and then pull the trigger and we'd watch stock disappear in minutes. Stock would go down to -2 or -3 sometimes.

I thought it happened due to the delay involved in processing a payment off-site. By the time payment for the last available item had been processed (and order stock was permanently reduced to 0), another customer (or more) could be checking out.

One solution would be to keep track of orders/SKUs that are currently "being paid for" and deduct them from the available stock when another customer attempts to check out. The message displayed to the customer should not be a "product X is out of stock" message, but a message that encourages them to retry in a few seconds/minutes.

Note that this is not the same as the "Hold Stock" feature, which operates already at cart-level and auto-cancels orders.

What's needed is a light version of "Hold Stock", which operates at checkout level (so customers can still add products to their carts) and works based on a hard-coded (perhaps filterable) duration to prevent other customers from checking out successfully.

@mikejolley

This comment has been minimized.

Copy link
Member

mikejolley commented Feb 22, 2017

We determined that even with table locking, this data would need to be in a separate table to avoid locking up the entire site. Therefore, 3.0 seems to be the right milestone.

@mikejolley mikejolley added this to the 3.0 milestone Feb 22, 2017

@franticpsyx

This comment has been minimized.

Copy link
Member

franticpsyx commented Feb 22, 2017

@mikejolley How would table locking be helpful? Customers may spend a long time paying for a pending order off-site. In older WC versions, it was even possible to pay for a pending order without WC double-checking line item availability.

"Hold stock" would resolve the problem here, but many shop owners choose not to enable it knowing that some customers may wish to pay for their "pending" orders with a delay (sometimes hours/days).

In UX terms, a "softer", always-on alternative would be to:

  1. Deduct stock in "pending" orders that go back as far as X minutes when checking availability at checkout -- and show a stock-related message that encourages customers to "try again in a few minutes" if the "stock on hold"
  2. Always check availability when attempting to pay for a pending order (might already be fixed in core).
@mikejolley

This comment has been minimized.

Copy link
Member

mikejolley commented Feb 22, 2017

@franticpsyx we're talking about 2 requests at the same time.

"Hold stock" would resolve the problem here

This won't fix anything because there is still a risk of 2 concurrent checkouts, both seeing something in stock, and both creating the order/reducing stock at the same time. Thats why you need table locking or a queue.

@franticpsyx

This comment has been minimized.

Copy link
Member

franticpsyx commented Feb 23, 2017

Thanks for the explanation @mikejolley -- looks like I misunderstood the original issue.

@SukcesStrony

This comment has been minimized.

Copy link

SukcesStrony commented May 4, 2017

Same here - negative stock amounts with backorders turned off. More people are experiencing this: https://wordpress.org/support/topic/woo-commerce-inventory-issue-negative-stock/
The WooCommerce version 4.0 milestone looks like too long of a wait for such a critical bug imho. It would be great if this can be pushed forward earlier.

@claudiosanches

This comment has been minimized.

Copy link
Member

claudiosanches commented May 4, 2017

@SukcesStrony not sure what "WooCommerce version 4.0 milestone looks like too long" means, 4.0 could come still this year, maybe next week, like after 3.1.0 could come 4.0.
Still the changes required here are for a Major release, please check it: https://woocommerce.wordpress.com/2017/03/13/important-update-regarding-the-upcoming-woocommerce-release-2-7-will-be-3-0-0/ and please stop making assumptions with version number and time, because they are not related. Thank you.

@SukcesStrony

This comment has been minimized.

Copy link

SukcesStrony commented May 4, 2017

@claudiosanches My assumption is based on the previous major version's date, which was released 10 months before this major release. Don't assume I'm assuming based on version numbering, thanks :)
As this is an important bug, it would be great if the release cycle could work faster for it, no matter the version numbering.

@claudiosanches

This comment has been minimized.

Copy link
Member

claudiosanches commented May 4, 2017

@SukcesStrony "no matter the version numbering." !== "The WooCommerce version 4.0 milestone looks like too long of a wait for".
You are contradicting yourself. But ok, as I said, this is something that we are working on and will require a big change and then need to go into a Major version.

@SWS-Denzle

This comment has been minimized.

Copy link

SWS-Denzle commented Jun 8, 2017

Hi all,

Do you have any update on this please?

I have a meeting with my client this afternoon and he's going to ask me if I've made any progress, I would appreciate it if I had something to say :)

Cheers.

@mikejolley

This comment has been minimized.

Copy link
Member

mikejolley commented Jun 8, 2017

@SWS-Denzle No. This issue will not be tackled until the next major release. You're free to contribute if you want it sooner though :)

@SWS-Denzle

This comment has been minimized.

Copy link

SWS-Denzle commented Jun 8, 2017

@mikejolley okay, as least that is something I can still pass on to my client. I would love to help but I think its beyond the scope of my skills :)

Cheers

@mikejolley

This comment has been minimized.

@Exellent1988

This comment has been minimized.

Copy link

Exellent1988 commented Jul 21, 2017

The trello link istn't working. IS there now a fix or eaven temporary workaround availible?

@mikejolley

This comment has been minimized.

Copy link
Member

mikejolley commented Jul 21, 2017

Until you see a link to a commit or pull request here, no.

@mikejolley mikejolley modified the milestone: 3.3.0 Sep 6, 2017

@lab21gr

This comment has been minimized.

Copy link

lab21gr commented Sep 28, 2017

Hello, i am having the same problem with stock turning to -1.
This happened for 2 orders placed at the same time, but also in 1 more case:

I was testing the site as my client told me of the issue with stock being -1 for 2 products.
So at the checkout the page kinda "froze" so i did the "stupid user" action: refreshed the page and clicked to submit the order again. The stock went to -1, but i have 2 same orders in the backend so i guess in these cases i can just cancel the second order.

I am running latest Woo (3.1.2)

not sure if this info helps out in solving this issue.

thanks

@mikejolley

This comment has been minimized.

Copy link
Member

mikejolley commented Sep 28, 2017

Right; locking wasn't really working well for this. We've logged an issue for the queue idea here to be done (hopefully) this year.

#16971

@mikejolley mikejolley closed this Sep 28, 2017

@salar90

This comment has been minimized.

Copy link

salar90 commented Jan 23, 2018

there would be a temporary solution:
add a hook to woocommerce_order_status_completed that checks all pending orders. cancel any of the orders which have items that are now out of stock.

update: use woocommerce_reduce_order_stock hook.

@SukcesStrony

This comment has been minimized.

Copy link

SukcesStrony commented Jan 23, 2018

@salar90 that would be an awful solution. Canceling orders that were already made and/or paid and emailed to the customer? Come on, man :)

@salar90

This comment has been minimized.

Copy link

salar90 commented Jan 24, 2018

@SukcesStrony as far as I know this issue would only happen when a "pending order" is paid after another pending order is paid which have items in common. this is very very rare and I think it usually happens when customer leave the payment step to another time. happens maybe once in 3 month with a 7k customer shop. and as I said it's a temporary solution and you are cancelling the pending order. not the paid one. good luck :)

@camilosw

This comment has been minimized.

Copy link

camilosw commented Nov 1, 2018

Why was this issue closed? I tested with 3.5.1 and the problem persist. The referenced issue #16971 was also closed but the merged pull referenced on that issue didn't solve this problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment