Giving seats back asks the order what it holds once per place, where taking them asks once for the run. A booker clearing six seats sends one run of clicks naming six places, exactly as taking them sent one naming six, and the two cost very different things.
Measured with a kernel cost test against a 24-place single-row hall, driving the picker as the map drives it:
One read of the whole order per place, and each of them inside a transaction that locks the order's row, so a basket of six takes six locks and six full reads of itself where one of each would do. It is quadratic in the lines of a basket rather than linear.
Not from the picker. OrderLifecycleSubscriber::onChange recomputes the order on every released line, and it already knows this is a hazard: it steps aside when TransactionManagerInterface::isTransitioning() says a whole-transaction transition is under way, with the comment that this avoids an O(n^2) storm and intermediate inconsistent states. A run of clicks is not one of those transitions, so nothing steps aside.
The clear op is the shape to copy. It releases every line the session holds and recomputes once at the end, because what the order holds is one question however many places the run names, and nothing between one place and the next reads the answer.
The mechanism exists and only lacks a way in. The transaction manager knows how to say "I am driving a batch on this order, do not recompute per line, I will recompute at the end"; it just says it to itself, privately, inside its own whole-transaction transitions. Making that a public bracket a caller can open, and having the picker's run of clicks open it, would leave the subscriber untouched and the guarantee unchanged.
Two things to settle. The bracket has to close on failure as well as success, or an order left marked as transitioning would stop recomputing altogether, which is worse than recomputing too often. And releasing can empty a basket, so the order has to be read before the releases rather than after, as the clear op already does.
PlaceReleaseCostTest is written and lands with the fix. It compares one place against six, in the manner the register asks of every booking operation, and asserts that the count of reads is the same for both rather than asserting a ceiling: a number written from what the code does today would be passed by construction by the very shape this is about. It counts the read of the order's lines rather than the row lock taken around it, because a locking read is written FOR UPDATE on MySQL and nothing at all on SQLite, so counting the lock would measure the driver instead of the code. Seen to fail against the current path, at 7 against 2.
Found while measuring the release path for the load harness in #3615593: Load test the booking path: what it bears, and what gives way first, which is also where the figures come from.
AI-Generated: Yes (Claude Code was used to draft this issue summary, to write the cost test and to run it. The measurements above came from that test rather than from reading the code, and the test was seen to fail against the current path; no fix is written yet.)
Show commandsStart within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes
Comments
Comment #3
mably commentedComment #4
mably commentedMR !242, every CI job green on the first pass.
A correction to the summary above, which overstated this. I wrote that the cost was quadratic in the lines of a basket. Measured with the fix off and on in one environment, giving back six places in one run of clicks goes from 58 queries and 6 recomputes to 53 and 1. The recompute count was linear in the places, not quadratic, and the reads of the order's lines it would have multiplied were largely served from the per-request cache, so the total saving is five queries rather than the blow-up the word suggests.
What genuinely improves is five fewer locks on the order's row per release, and a shape that no longer grows with the size of the basket. That is worth having on a gesture a booker repeats, but it is not the emergency the summary implied, and Major is probably generous.
Two things the work turned up that the summary did not know. Remove all had the same defect: it recomputed once at the end but released every line one at a time, so the per-line listener answered each one; it opens the same bracket now. And my first attempt at a fix, removing the picker's own resync, changed nothing measurable and was reverted, which is how I found that the listener rather than the controller was doing the work.
The test counts the read of the order's row rather than the read of its lines. The line read is refilled by the cache invalidation after every write whatever the recompute does, so counting it would have passed a fix that did nothing, which is exactly what my first metric did.
AI-Generated: Yes (Claude Code wrote the fix, the test and this comment. The figures come from running that test with the change off and on in one environment; the test was seen to fail against the unfixed path.)
Comment #5
mably commented