Setup
The ticket: “Stop overselling. Reserve stock when the customer checks out, then charge their card.” An AI assistant wrote most of this, and the unit tests pass.
Context you need:
- The shop runs 8 web workers behind a load balancer. Flash sales put 200+ checkouts per second on the same few SKUs.
payments.chargecalls an external payment provider. It raisesPaymentTimeoutif the provider doesn’t answer in 10 seconds. A timeout doesn’t mean the charge failed; the provider may still complete it.- The provider supports an
idempotency_keyargument.
--- a/shop/checkout.py
+++ b/shop/checkout.py
@@ -1,4 +1,33 @@
from shop.models import Inventory, Order
from shop import payments
+from shop.errors import OutOfStock, PaymentTimeout
+
+
+def reserve_stock(sku: str, qty: int) -> bool:
+ item = Inventory.objects.get(sku=sku)
+ if item.available < qty:
+ return False
+ item.available -= qty
+ item.save()
+ return True
+
+
+def checkout(cart, payment_method):
+ for line in cart.lines:
+ if not reserve_stock(line.sku, line.qty):
+ raise OutOfStock(line.sku)
+
+ for attempt in range(5):
+ try:
+ charge = payments.charge(payment_method, cart.total)
+ break
+ except PaymentTimeout:
+ continue
+
+ return Order.objects.create(cart=cart, charge=charge)Outcome
A version of this shipped before a flash sale. Two problems showed up within the hour:
- Oversold stock. Inventory went negative on the three most popular SKUs, and about 140 orders had to be cancelled.
- Double charges. When the payment provider slowed down, some customers were charged two or three times for one order, because each timeout triggered a fresh charge.
Support also found carts that failed with “out of stock” on their second item while their first item stayed reserved for good, so the store showed less stock than it really had.
Senior commentary
“Two words in the context jump out: ‘eight workers’ and ‘timeout’. Eight workers means reserve_stock runs concurrently, and it’s a classic check-then-act. Two requests read available = 1, both pass the check, both save 0. The database never sees a conflict, because each one just writes its own number. The fix is to make the database do the check: UPDATE ... SET available = available - qty WHERE sku = ? AND available >= qty, then look at the row count.
“‘Timeout’ plus ‘retry’ means I immediately ask whether the operation is idempotent. Charging a card isn’t, unless you send the same idempotency key on every attempt. A timeout means ‘I don’t know’, not ‘it failed’, and retrying a ‘don’t know’ on a payment is how you charge people twice.
“Then I trace the unhappy paths: what if the second line is out of stock, and what if all five attempts time out? Neither path releases the stock reserved before it. And if every attempt times out, charge is never assigned, so the code crashes with an UnboundLocalError instead of a clean error.”
Discussion questions
- Why don’t the unit tests catch the race condition? What kind of test would?
- Where should the transaction boundary be: around reservation only, around reservation plus order creation, or somewhere else? What can’t be inside it?
- How should the code handle a timeout that might have charged the card?
- Rank the flaws by severity. Did you rank any of them differently from the key, and why?
Answer key
| # | Flaw | Severity |
|---|---|---|
| 1 | Race condition in reserve_stock. Check-then-act across a read and a save. Concurrent checkouts oversell. Use an atomic conditional update or a row lock in a transaction. | High |
| 2 | Retrying a non-idempotent charge. A PaymentTimeout may still succeed at the provider; each retry can charge again. Pass the same idempotency_key on every attempt. | High |
| 3 | Reservations never released on failure. If a later line is out of stock, or payment fails, stock reserved earlier stays reserved forever. Release it (compensate), or do reserve and order creation in one transaction. | High |
| 4 | charge unbound after 5 timeouts. Falls through to Order.objects.create and raises UnboundLocalError instead of a clear payment error. | Medium |
| 5 | No transaction around reserve and create. A crash between the steps leaves stock reserved with no order to account for it. | Medium |
| 6 | Retries without backoff. Five immediate retries against a slow provider add load right when it’s struggling. | Low |