Three code paths decremented the same stock column, and only one of them took a lock

Taking stock out of a warehouse was implemented three times in one backend. Not three times for three different things — three times for the same thing, by three people at three different points in the project, and the three implementations do not agree with each other about what safety means.

The first one is a shared service. It finds the inventory row, takes FOR UPDATE on it, compares the current stock against the requested quantity, throws a conflict if there is not enough, and writes the new figure. That is correct. A second transaction attempting the same decrement blocks on the lock until the first one commits, then re-reads and sees the reduced figure.

The second one is a production service. Same shape — read the available quantity, refuse if short, apply the change — except the read that decides the outcome takes no lock. It does write a row into the movement table first, which the shared service does not, so it has a better audit trail and a worse guarantee.

The third one is not a service at all. It is written inline inside the sale-creation handler: find the inventory row, compare, subtract, save. No lock, no movement row. It runs inside a transaction, which is the thing that makes it look safe and is exactly why nobody questioned it.

The wrong assumption

I assumed that because a stock service existed, the stock service was being used. That is the assumption you make when you see a well-written service file with typed parameter interfaces and a transaction argument, and it is a bad assumption in any codebase older than about six months.

A search for callers settled it. The movement table — the one designed to be the audit trail for every inventory change, with an item type, a direction, a reason and a reference back to the source transaction — is written from exactly one file. Everything else that changes stock changes it by updating the balance column directly.

So the schema has a movement ledger, and the ledger is missing every sale, every purchase receipt and every manual correction. The table is real, the data in it is real, and the answer it gives to “where did this stock go” is wrong for four of the five ways stock moves.

What the missing lock actually costs

The mechanism is the ordinary lost update, and it is worth spelling out because the transaction makes people think it cannot happen.

Two till operators sell the last case of the same product at the same moment. Both transactions read the inventory row and both see the same figure. Both compare it against a quantity of one and both pass. Both compute the new figure by subtracting from the value they read, and both write it. The second write overwrites the first. Stock is decremented once. Two cases left the building.

The transaction does not prevent this. Under read committed — the default in most deployments — two transactions may read the same row concurrently and both proceed. A transaction gives you atomicity and a consistent snapshot; it does not give you serialisation unless you either ask for the lock or ask for the isolation level, and the code asked for neither.

The correct read is SELECT ... FOR UPDATE, which is what the shared service does and what the other two do not. The alternative — writing SET current_stock = current_stock - :qty and letting the database do the arithmetic — fixes the lost update but not the oversell, because you still have to check the result and the check is a second statement. If you need “do not go below zero” you either take the lock or add a check constraint and treat the violation as the refusal.

Why the duplicates existed

Nobody duplicated this deliberately. The sale handler was written first, when the system had one warehouse and one till and the question had not come up. The production service was written next, by someone who cared about the audit trail and added the movement row. The shared service was written last, by someone who cared about concurrency and added the lock. Each author solved the problem they had arrived to solve, and none of them went back and changed the other two, because changing a working sale path to route through a new service is a risky change with no visible benefit on the day you make it.

That is the ordinary way this happens, and the reason it is worth naming is that the fix is organisational rather than clever. There is nothing hard about the correct implementation. The hard part is having exactly one of it.

The rule that would have prevented it

A quantity column that more than one module can write needs a single writer, and the writer has to be the only thing that knows how. Not a convention, not a documented pattern — one function, and the underlying tables treated as private to it.

That is enforceable in a way conventions are not. Put the stock tables behind a service and grant the application role update rights only through it, or if that is too heavy, make the movement row mandatory: a trigger that recomputes the balance from the movements, so a handler that skips the ledger simply does not move the stock. Either way, the correctness stops depending on whether the next developer knows the service exists.

The version of this that people usually reach for — a code review rule, a comment on the table, a line in the contributing guide — does not survive a deadline. The inline sale handler was not written by someone who disagreed with the service. It was written before the service, and then it was never revisited, because it worked.

The second rule, about audit tables

An audit table that some writers populate is worse than no audit table, because people query it and believe the answer. A ledger with production movements and no sales movements will happily report that a product’s stock has only ever gone up. Nobody reading that report will think to ask which code paths were instrumented.

If you add a movement table, the acceptance criterion is not “the movement table is written when stock changes in the module I am building”. It is “every path that changes stock writes a movement, and I have listed the paths.” Listing them takes an afternoon. On this system the list was five and the count instrumented was one, and that ratio was invisible until somebody went looking.

Where it still is not solved

The three implementations were not merged in one pass, because the sale path also computes totals, tax, delivery state and payment state in the same handler, and pulling the inventory part out of it is a change to the most sensitive route in the product. The order that was chosen was: instrument first, consolidate second. Every path writes a movement row before any path is rewritten to use the shared service, so that the ledger becomes true before the code becomes tidy.

That is the honest state of it. The ledger being right matters more than the code being right, because a wrong number in an audit trail is a lie that outlives the refactor.