d48feb30d7
PR#65 set the draining flag after releasing the concurrency slot, so a build waiting on that semaphore could be woken by the release and start before it could read the flag — under the version on its way out, to be cut short by the restart and re-run by the recovery. That is the window the drain exists to close, one layer further in than the one PR#65 closed. The flag is now set before the slot is released; the wait for the executing builds stays after it. Found by Scenario#65.05 on the production host, which is slower and busier than the machine the change was written on, where it had passed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: mhoennig <michael@hoennig.de> Reviewed-on: #66
65 lines
3.2 KiB
Markdown
65 lines
3.2 KiB
Markdown
> **WARNING:** This document describes only the change applied in this PR.
|
|
> It may already be outdated once the next PR is merged.
|
|
> Historic PR-documentation is not maintained along with new PRs — treat it as a snapshot, not as current documentation.
|
|
|
|
## Related Links
|
|
|
|
- [PR#65](2026-09-15-PR%2365-the-instance-ends-itself.md) — the draining state this fixes
|
|
- [RFC 0005](../rfcs/0005-werkator-deploys-itself.md), step 2
|
|
|
|
## The Problem
|
|
|
|
[PR#65](2026-09-15-PR%2365-the-instance-ends-itself.md) exists to close a window: a build that starts in the moment between "is the instance idle?" and the stop is cut short and re-run by the recovery.
|
|
It left a smaller one of the same kind, one layer further in.
|
|
|
|
The executor set its draining flag *after* releasing the concurrency slot.
|
|
A build waiting on that semaphore is woken by the release, and it reads the flag on the other side of the wake-up.
|
|
Between the two lies a handful of instructions, and a worker that gets there first starts a build under the version that is on its way out — the outcome the drain exists to prevent.
|
|
|
|
The test for the queued build (Scenario#65.05) caught it on the first run on the production host, where the machine is slower and busier than the laptop the change was written on.
|
|
It had passed locally.
|
|
|
|
## Non-Goals
|
|
|
|
- No new behaviour: the drain, the pinned setting and the deployment script are unchanged.
|
|
|
|
## The Scenarios
|
|
|
|
### Feature: nothing starts under the outgoing version
|
|
|
|
#### Scenario#66.01: The flag is set before the slot is free
|
|
|
|
So that a build waiting for a slot can never observe it free while the drain is not yet declared.
|
|
|
|
- **Given** a deployment holds the only concurrency slot and another build is waiting for it
|
|
- **When** the deployment ends `SUCCESS`
|
|
- **Then** the waiting build sees the instance draining as soon as it acquires the slot
|
|
- **and** it stays `PENDING` and prepares no workspace
|
|
|
|
##### Verified by
|
|
|
|
- [BuildExecutorTest — a build queued when the deployment ends stays pending and starts nothing](../../src/test/kotlin/de/hoennig/werkator/build/BuildExecutorTest.kt)
|
|
|
|
## The Solution
|
|
|
|
The flag is now set in the same `finally`, before `builds.remove` and before `slot.release()`, and the waiting for the executing builds stays where it was — after both.
|
|
`beginDrain` therefore splits along the line the ordering demands: the declaration is a `compareAndSet` in the release path, the wait is `awaitDrained`.
|
|
|
|
The order of the three statements is now load-bearing in both directions, which is worth saying out loud: the flag must come *before* the release, and the removal from the current builds must stay *after* the terminal status and the artifacts, for the reason it always did.
|
|
|
|
The scenario's test now makes the deployment hold the slot long enough for the second build to be genuinely waiting when it ends.
|
|
That makes the situation real on every run; it does not make the old ordering fail deterministically, because the window is a few instructions wide.
|
|
The fix is correct by construction, and the test is the scenario's check rather than a proof of the race.
|
|
|
|
## Additional Changes
|
|
|
|
- None.
|
|
|
|
## Prerequisite PRs
|
|
|
|
- [PR#65](2026-09-15-PR%2365-the-instance-ends-itself.md)
|
|
|
|
## Follow-up PRs
|
|
|
|
- The two measurements RFC 0005 left open: CPU time per build, and `/proc/pressure/io`.
|