Problem/Motivation
Holding a party of N units on one session issues about 8N + 6 queries, and roughly half of that is the same question asked again. Measured with Database::startLog() around one holdMany(), SQLite kernel, a 24-place single-row hall, one prior booking so every cache is warm:
- party of 1: 14 queries, of which the slot-wide consumption sum is 2 and the per-place count is 2
- party of 2: 22 queries, 4 and 4
- party of 6: 54 queries, 12 and 12
- party of 12: 102 queries, 24 and 24
Two reads account for the growth, and they are not equally wasteful.
assertFits()'s SELECT b.tariff, b.allotment, SUM(b.quantity) FROM yoyaku_booking b WHERE b.slot = ? AND b.state IN (...) GROUP BY b.tariff, b.allotment is identical in statement and in parameters for every line of the group: nothing in it is per line. Above the locks, where nothing can change, all N of them are one question asked N times. Under the locks it must genuinely re-read, because each line saved changes the answer.
PlaceBookable's SELECT COUNT(*) FROM yoyaku_booking WHERE slot = ? AND place = ? is the same statement with a different place each time, so it is not redundant as it stands. Its N calls could be one read naming all N places.
Why no performance test caught this
They measured the wrong axis. Both the alpha3 audit and the Together audit asked whether anything scales worse than linearly in hall size, and nothing does. This scales with the number of lines in one booking, which nothing asked about.
Worse, the cost test encodes the waste as the budget. HoldCostTest asserts ONE_LINE_CEILING = 10 and PER_EXTRA_LINE_CEILING = 5, both written from what the code already did, so a design that re-asks a slot-wide question once per line passes its own ceiling by construction and always will. A per-line ceiling derived from observed behavior is a description of the code, not a test of it.
The rule this suggests: a booking performance test must compare a party of one against a party of several and assert the slope, not a ceiling per line. A test measuring one seat and six and asserting that the slot-wide read count does not grow with the party would have failed the day this appeared.
Proposed resolution
A request cache with the slot locks as its boundary, and deliberately two of them: one for everything read before the locks are taken, flushed when they are, and one for the locked section, flushed when it ends. A read taken before the lock must never answer a question asked inside it, which is the race the second evaluation pass exists to close: two requests each reading room for the last unit would both commit. Two docblocks already say so, one on ConstraintPolicyManager::$outcomes and one on PolicyContext's fact memo.
That removes the pre-lock repetitions. The locked ones need two different fixes: batch PlaceBookable's per-place counts into one read of the places the group names, and make assertFits() add up what the group has already taken in memory rather than re-reading after each insert, which is a refactor rather than a cache.
Design goal for the locked section: the read there should be the session's place availability, not these places' counts. Both shapes remove the per-line repetition, but only the first is reusable: PlaceBookable, SectionPoolBookable and the orphaned_places policy from #3616316: Refuse a booking that orphans a place, alone between bookings or at the end of a row all answer from one availability read, so that policy's remaining extra query becomes zero instead of surviving beside a batched count. Shaped as a batch of counts, each of them keeps a read of its own.
Remaining tasks
- The two request caches and the flush at the lock boundary.
- Batch the per-place availability check.
- Shape the locked-section read as the session's availability, so the constraints and the orphan policy all answer from one read.
assertFits()accumulating the group's own consumption instead of re-reading it.- Replace
HoldCostTest's per-line ceilings with a comparison of one seat against several, asserting that the slot-wide read count does not grow with the party, so no budget can silently absorb this again. - Before and after query counts for parties of 1, 2, 6 and 12 in the issue.
User interface changes
None.
API changes
PlaceAvailability and the hold path gain a request-scoped memo; assertFits() changes shape. Nothing released depends on either.
Data model changes
None.
AI-Generated: Yes (Claude Code found this while measuring another change, produced the query counts above with a throwaway probe, and drafted this summary. No fix is written yet.)
Issue fork yoyaku-3616341
Show commands
Start 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:
Comments
Comment #2
mably commentedComment #4
mably commentedComment #6
mably commented