Problem/Motivation
One press of a stepper takes the slot locks twice. The hold places the seats, commits, and releases its locks. A consolidation is then asked for as a separate call, and that call reaches the exchange through hold(), so it locks and commits a second time.
Nothing is left incorrect by this. The exchange is atomic, and a proposal that went stale between the two is refused under the second set of locks, leaving the booker holding exactly what they held. What it costs is everything short of correctness.
The consolidation is the first thing lost under contention. It is decided against the world as the first transaction left it, and by the time the second lock is in hand another booker may hold the places it wanted. That refusal is silent, so a house that fills quickly loses the consolidation it was promised and nothing says so. On most claims there is nothing to consolidate and no second lock is taken, so the cost falls on the claims that do. Between the two commits, the split arrangement is also what every other request sees.
Proposed resolution
A hold now consolidates before it releases its locks, so the placement and the consolidation land in one transaction under one set of locks.
hold() carries a $consolidate parameter, on by default. The recursion needs saying out loud: an exchange reaches the engine through hold(), so a hold that offered a consolidation would have that one offer another. consolidate() passes the parameter off, which puts the guard at the call rather than in state nobody can see.
A hold consolidates only what its own locks already cover. It has already read the candidates, the judged rows, and the party it placed around, so the consolidation asks almost nothing of its own: one statement on a hold that has a party on the slot, and none on a hold that does not, however large the party. It never takes a second lock. An exchange reaching a part of the resource the hold has not locked is not made, and the result says so, so the caller knows to ask the wider call. Taking a second lock while holding the first is the deadlock #3616975: Put two bookers in flight at once, and guard the sort that stops them deadlocking was about.
An exchange is whole or nothing. It has to write before it can be checked, since the places it takes are mostly the ones it is handing back. So it runs inside a savepoint and marks the deferred batch, and a refusal puts back both. Either alone leaves a wreck: a savepoint returns the rows but cannot reach the batch, whose seats would stay marked gone at the flush with no booking holding them. Where the batch holds work no registrant can hand back, the exchange is not attempted at all.
Four faults older than this branch were fixed alongside it, because the change could not be made correct without the first two:
- The lock scopes covered only the offers a hold takes, not the ones it gives up, so a hold could rewrite rows it had not locked.
- A slot's spent units were read once and kept for the whole hold, so an exchange was refused for want of room it had itself just handed back. On a quota whose last unit the party holds, that was every exchange.
- A party holding one place the booker chose could not be improved at all. The veto was put to the whole party rather than to the place being considered, so one chosen seat in the balcony froze four places in the stalls.
- A party carried into another part of the venue was told it had moved to another row, and went looking down the part it had left.
Remaining tasks
None. The lock traffic was measured on the load rig. A claim that consolidates saves one transaction and one acquisition, and on a hall where the party is longer than the longest row about one claim in thirty consolidates, so the saving is near 0.03 of a transaction per claim. Throughput is unchanged within the rig's noise. The figure to quote is the transaction a consolidating claim no longer needs, not a share of the hot path.
User interface changes
A party carried into another part of the venue is now told that, rather than that it changed row, in English and in French. Nothing else changes for a booker.
API changes
- hold() gains
$consolidate, defaulting to the behavior every caller has today. - BookingResult gains
needsConsolidation, TRUE unless a hold both looked at how the party sits and finished with it, so an operation with nothing to say about it leaves its caller doing what it did before. - SelectionBooker::placeHolds() carries it out as
needs_consolidation. - DeferredWrites gains registerGatheredState(), mark() and rollBackTo(). mark() answers NULL where a registrant cannot hand its work back, since undoing half a batch is worse than undoing none.
- BookingManager::consolidate() is unchanged, and stays the entry point that reaches wider than a hold can, having taken the locks it needs itself.
Data model changes
None.
AI-Generated: Yes (Claude Code found this while fixing a neighboring bug and drafted this summary. The design is mine; I reviewed the summary before posting.)
Issue fork yoyaku-3620039
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 #5
mably commentedComment #6
mably commentedComment #7
mably commentedComment #9
mably commented