fix(dispatch): broker-create onder belasting laat geen wees achter (M41 T-1956) #97

Merged

janpeter merged 3 commits from fix/m41-broker-create-under-load into master 2026-10-02 21:09:30 +02:00

Conversation 5 Commits 3 Files changed 5 +132 -5

janpeter commented 2026-10-02 20:49:21 +02:00

Owner

Copy link

Waarom

In de M41-praktijkproef gemeten op scrum4me-server (verzoek 8d3d8df1-…, poging a288a4c8-…, 2026-10-02 19:29). Tijdens een gelijktijdige webbuild (load 6,7, swap) gebeurde dit:

• De broker deed er ongeveer 14 s over vóór docker create, en docker create zelf duurde ongeveer 75 s.
• dockerCommand doodde de CLI na 30 s, maar de daemon maakte de create gewoon af.
• discard verwijderde pogingsmap en journal, en deed rm -f op de naam voordat de container bestond.
• Gevolg: een container in status created die niets meer kende.
• De supervisor gaf na 45 s op (DISPATCH_RUNTIME_TRANSPORT_FAILED), en de poging werd UNCERTAIN met een bezette reservering. Herstel ging met de hand: container weg, operator_attested en recover close_failed.

Wat

• Timeouts, elke laag wacht langer dan de laag eronder:
  • docker create krijgt 180 s (DOCKER_CREATE_TIMEOUT_MS); andere commando's houden 30 s;
  • de supervisor wacht 240 s op een broker-create (BROKER_CREATE_REQUEST_TIMEOUT_MS), en dat valt binnen de bestaande prepare-grens van 300 s;
  • andere broker-methodes houden 45 s.
• Opruimen op label, met docker rm zonder -f, zodat een gestarte container nooit wordt verwijderd:
  • discard verwijdert ook de nooit gestarte containers van die poging (label=s4m.dispatch.slot, label=s4m.dispatch.attempt, status=created);
  • bij broker-start en bij elke create verwijdert de broker nooit gestarte containers van dit slot waarvan de poging geen journal heeft en ook niet net wordt aangemaakt, met een extra controle via inspect (labels, created, pid 0).
• Operator-doc bijgewerkt.

Bewijs

• Rood eerst: 4 nieuwe tests faalden.
  • broker-docker-timeouts.test.ts: de timeoutlagen;
  • dispatch-broker.test.ts: de wees na een mislukte create wordt opgeruimd; een te laat afgemaakte create wordt bij de volgende broker-start opgeruimd; gestarte, gejournalde en vreemde containers worden nooit verwijderd.
  • Daarna groen: 42/42 in de broker-bestanden.
• Aangepaste bestaande test: een geweigerde release mocht helemaal geen ps doen. Nu geldt: geen rm en geen ps op het exacte id. Het opruim-ps bij create is nieuw en bewust.
• Volledige suite: npm test geeft 1040 groen, met de 2 bekende macOS-failures in transcript-retention. tsc -p tsconfig.dispatch.json is groen.
• Linux, scrum4me-server (tijdelijke worktree op 6968c18, daarna verwijderd):
  • npm run test:dispatch-runtime en npm run test:dispatch-model-image slagen allebei;
  • tegen echte Docker vindt de labelfilter de nooit gestarte probecontainer, niet die van een andere poging, en niet meer zodra hij gestart is. De probe is daarna verwijderd.

Niet bewezen: een create van meer dan 30 s onder echte belasting. Die heb ik niet bewust opgewekt.

Story ST-1617, taak T-1956 (Scrum4Me).

🤖 Generated with Claude Code

## Waarom In de M41-praktijkproef gemeten op scrum4me-server (verzoek `8d3d8df1-…`, poging `a288a4c8-…`, 2026-10-02 19:29). Tijdens een gelijktijdige webbuild (load 6,7, swap) gebeurde dit: - De broker deed er ongeveer 14 s over vóór `docker create`, en `docker create` zelf duurde ongeveer 75 s. - `dockerCommand` doodde de CLI na 30 s, maar de daemon maakte de create gewoon af. - `discard` verwijderde pogingsmap en journal, en deed `rm -f` op de naam voordat de container bestond. - Gevolg: een container in status `created` die niets meer kende. - De supervisor gaf na 45 s op (`DISPATCH_RUNTIME_TRANSPORT_FAILED`), en de poging werd `UNCERTAIN` met een bezette reservering. Herstel ging met de hand: container weg, `operator_attested` en `recover close_failed`. ## Wat - **Timeouts, elke laag wacht langer dan de laag eronder:** - `docker create` krijgt 180 s (`DOCKER_CREATE_TIMEOUT_MS`); andere commando's houden 30 s; - de supervisor wacht 240 s op een broker-`create` (`BROKER_CREATE_REQUEST_TIMEOUT_MS`), en dat valt binnen de bestaande prepare-grens van 300 s; - andere broker-methodes houden 45 s. - **Opruimen op label, met `docker rm` zonder `-f`, zodat een gestarte container nooit wordt verwijderd:** - `discard` verwijdert ook de nooit gestarte containers van die poging (`label=s4m.dispatch.slot`, `label=s4m.dispatch.attempt`, `status=created`); - bij broker-start en bij elke `create` verwijdert de broker nooit gestarte containers van dit slot waarvan de poging geen journal heeft en ook niet net wordt aangemaakt, met een extra controle via `inspect` (labels, `created`, pid 0). - Operator-doc bijgewerkt. ## Bewijs - **Rood eerst:** 4 nieuwe tests faalden. - `broker-docker-timeouts.test.ts`: de timeoutlagen; - `dispatch-broker.test.ts`: de wees na een mislukte create wordt opgeruimd; een te laat afgemaakte create wordt bij de volgende broker-start opgeruimd; gestarte, gejournalde en vreemde containers worden nooit verwijderd. - Daarna groen: 42/42 in de broker-bestanden. - **Aangepaste bestaande test:** een geweigerde `release` mocht helemaal geen `ps` doen. Nu geldt: geen `rm` en geen `ps` op het exacte id. Het opruim-`ps` bij `create` is nieuw en bewust. - **Volledige suite:** `npm test` geeft 1040 groen, met de 2 bekende macOS-failures in `transcript-retention`. `tsc -p tsconfig.dispatch.json` is groen. - **Linux, scrum4me-server** (tijdelijke worktree op `6968c18`, daarna verwijderd): - `npm run test:dispatch-runtime` en `npm run test:dispatch-model-image` slagen allebei; - tegen echte Docker vindt de labelfilter de nooit gestarte probecontainer, niet die van een andere poging, en niet meer zodra hij gestart is. De probe is daarna verwijderd. Niet bewezen: een create van meer dan 30 s onder echte belasting. Die heb ik niet bewust opgewekt. Story ST-1617, taak T-1956 (Scrum4Me). 🤖 Generated with [Claude Code](https://claude.com/claude-code)

janpeter added 1 commit 2026-10-02 20:49:21 +02:00

fix(dispatch): broker-create onder belasting laat geen wees achter (M41 T-1956) ... 6968c1865c

Op scrum4me-server gemeten tijdens een webbuild: docker create duurde
~75 s, de CLI werd na 30 s gedood terwijl de daemon de create afmaakte,
de discard zocht de container op naam voordat hij bestond, en er bleef
een nooit gestarte container achter; de supervisor gaf na 45 s op en de
poging werd onzeker. Nu: create krijgt 180 s, de supervisor wacht 240 s
op een broker-create (binnen de prepare-grens van 300 s), discard ruimt
nooit gestarte containers van die poging op label op, en broker-start en
elke create ruimen nooit gestarte containers van dit slot zonder journal
op (docker rm zonder -f).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

s4m-codex-reviewer requested changes 2026-10-02 20:54:49 +02:00

Dismissed

s4m-codex-reviewer left a comment

Copy link

REQUEST_CHANGES

• error (blokkerend) — lib/dispatch-runtime-broker.ts:135: Een late weescontainer wordt niet opgeruimd bij een succesvolle retry met hetzelfde attempt-ID. create() registreert de retry al in creating vóór de reap; daardoor wordt de oude container overgeslagen. Na de succesvolle retry zorgt attempts.has(attemptId) ervoor dat ook toekomstige reaps de oude container behouden, terwijl het journal alleen de nieuwe container vastlegt. Maak het onderscheid per container/scope en verwijder eerdere residue vóór de nieuwe create. Voeg een regressietest toe: create faalt, daemon voltooit de oude container na discard, dezelfde attempt wordt succesvol opnieuw gemaakt zonder brokerrestart.

geen gekoppeld plan gevonden — beoordeeld op codekwaliteit + product-standaarden.

Gecontroleerd op commit 6968c1865c247dabb20053c133ed6ade50b82fab, met de gepinde shared-submodule en relevante productdocumentatie. De wijziging volgt verder de bestaande broker- en journalpatronen en documenteert de timeoutaanpassing. Typecontrole (tsc -p tsconfig.dispatch.json) slaagt; diff-whitespacecontrole slaagt. Het beschreven retrypad is met de bestaande fixture en een tijdelijke TypeScript-loader gereproduceerd: de retry slaagt en voert geen verwijdering van de oude container uit. De volledige Vitest-suite kon niet starten doordat deze omgeving uitvoering/native-library-mapping vanuit /tmp blokkeert; dus geen claim dat de suite groen is.

# REQUEST_CHANGES - **error (blokkerend)** — `lib/dispatch-runtime-broker.ts:135`: Een late weescontainer wordt niet opgeruimd bij een succesvolle retry met hetzelfde attempt-ID. `create()` registreert de retry al in `creating` vóór de reap; daardoor wordt de oude container overgeslagen. Na de succesvolle retry zorgt `attempts.has(attemptId)` ervoor dat ook toekomstige reaps de oude container behouden, terwijl het journal alleen de nieuwe container vastlegt. Maak het onderscheid per container/scope en verwijder eerdere residue vóór de nieuwe create. Voeg een regressietest toe: create faalt, daemon voltooit de oude container na discard, dezelfde attempt wordt succesvol opnieuw gemaakt zonder brokerrestart. geen gekoppeld plan gevonden — beoordeeld op codekwaliteit + product-standaarden. Gecontroleerd op commit `6968c1865c247dabb20053c133ed6ade50b82fab`, met de gepinde shared-submodule en relevante productdocumentatie. De wijziging volgt verder de bestaande broker- en journalpatronen en documenteert de timeoutaanpassing. Typecontrole (`tsc -p tsconfig.dispatch.json`) slaagt; diff-whitespacecontrole slaagt. Het beschreven retrypad is met de bestaande fixture en een tijdelijke TypeScript-loader gereproduceerd: de retry slaagt en voert geen verwijdering van de oude container uit. De volledige Vitest-suite kon niet starten doordat deze omgeving uitvoering/native-library-mapping vanuit `/tmp` blokkeert; dus geen claim dat de suite groen is.

janpeter referenced this pull request from a commit 2026-10-02 20:56:07 +02:00

fix(dispatch): retry van dezelfde poging ruimt eerdere wees eerst op (review #97)

janpeter added 1 commit 2026-10-02 20:56:07 +02:00

fix(dispatch): retry van dezelfde poging ruimt eerdere wees eerst op (review #97) ... 40f36b8b14

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

janpeter commented 2026-10-02 20:56:14 +02:00

Author

Owner

Copy link

BLOCKER verwerkt in 40f36b8. performCreate verwijdert nu vóór het maken van een nieuwe container eerst de nooit gestarte containers van precies deze poging (removeUnstarted({attemptId}), op label, docker rm zonder -f). Dat gebeurt ook wanneer er geen oud record meer is. Zo ruimt een retry van dezelfde poging de wees op die de daemon na de discard nog heeft afgemaakt, zonder brokerrestart. Er is een regressietest bij gekomen voor precies jouw scenario (create faalt, de daemon maakt hem later af, dezelfde poging wordt opnieuw gemaakt en het rm van de wees gebeurt vóór de nieuwe create). Die test faalde eerst (0 verwijderingen) en slaagt nu. Broker-tests 39/39, npm test 1041 groen (plus de 2 bekende macOS-failures in transcript-retention), tsc groen.

BLOCKER verwerkt in `40f36b8`. `performCreate` verwijdert nu vóór het maken van een nieuwe container eerst de nooit gestarte containers van precies deze poging (`removeUnstarted({attemptId})`, op label, `docker rm` zonder `-f`). Dat gebeurt ook wanneer er geen oud record meer is. Zo ruimt een retry van dezelfde poging de wees op die de daemon na de discard nog heeft afgemaakt, zonder brokerrestart. Er is een regressietest bij gekomen voor precies jouw scenario (create faalt, de daemon maakt hem later af, dezelfde poging wordt opnieuw gemaakt en het `rm` van de wees gebeurt vóór de nieuwe `create`). Die test faalde eerst (0 verwijderingen) en slaagt nu. Broker-tests 39/39, `npm test` 1041 groen (plus de 2 bekende macOS-failures in transcript-retention), tsc groen.

s4m-codex-reviewer requested changes 2026-10-02 21:02:21 +02:00

s4m-codex-reviewer left a comment

Copy link

REQUEST_CHANGES

• error (blokkerend) — lib/dispatch-runtime-broker.ts:135: een oude daemon-create die pas ná de attempt-specifieke sweep (:183), tijdens de retry, voltooit, blijft permanent als wees bestaan. Het nieuwe journal wijst naar de retry-container, maar attempts.has(attemptId) beschermt alle containers met hetzelfde attempt-label. Ook herstart en release lossen dit niet op: het journal blijft behouden en release verwijdert alleen de actuele scope. Maak de bescherming afhankelijk van de actuele containeridentiteit en zorg voor veilige opruiming van de oudere, nooit gestarte container. Voeg een regressietest met twee container-ID's toe waarin de oude create tijdens de retry voltooit; __tests__/dispatch-broker.test.ts:127 simuleert alleen voltooiing vóór de retry.

geen gekoppeld plan gevonden — beoordeeld op codekwaliteit + product-standaarden.

De timeoutwijzigingen en operator-documentatie sluiten aan bij de bestaande broker/runtime-opzet. Typecheck (node node_modules/typescript/bin/tsc -p tsconfig.dispatch.json) geslaagd na initialisatie van de gepinde shared-submodule. Tests konden hier niet starten: /tmp is noexec, waardoor de native Rolldown-binding niet geladen kan worden. Dit is een verificatiebeperking van de reviewomgeving, geen aangetoonde PR-regressie.

# REQUEST_CHANGES - **error (blokkerend)** — `lib/dispatch-runtime-broker.ts:135`: een oude daemon-create die pas ná de attempt-specifieke sweep (`:183`), tijdens de retry, voltooit, blijft permanent als wees bestaan. Het nieuwe journal wijst naar de retry-container, maar `attempts.has(attemptId)` beschermt alle containers met hetzelfde attempt-label. Ook herstart en release lossen dit niet op: het journal blijft behouden en release verwijdert alleen de actuele scope. Maak de bescherming afhankelijk van de actuele containeridentiteit en zorg voor veilige opruiming van de oudere, nooit gestarte container. Voeg een regressietest met twee container-ID's toe waarin de oude create tijdens de retry voltooit; `__tests__/dispatch-broker.test.ts:127` simuleert alleen voltooiing vóór de retry. geen gekoppeld plan gevonden — beoordeeld op codekwaliteit + product-standaarden. De timeoutwijzigingen en operator-documentatie sluiten aan bij de bestaande broker/runtime-opzet. Typecheck (`node node_modules/typescript/bin/tsc -p tsconfig.dispatch.json`) geslaagd na initialisatie van de gepinde shared-submodule. Tests konden hier niet starten: `/tmp` is `noexec`, waardoor de native Rolldown-binding niet geladen kan worden. Dit is een verificatiebeperking van de reviewomgeving, geen aangetoonde PR-regressie.

janpeter commented 2026-10-02 21:03:56 +02:00

Author

Owner

Copy link

BLOCKER verwerkt in b872855. De bescherming volgt nu het gejournalde container-id in plaats van het attempt-label: removeUnstarted slaat alleen attempts.get(attemptId).binding.scope.scopeId === id en pogingen die nog worden aangemaakt over, zowel in de label-modus als in de modus per poging. Een oude, nooit gestarte container van dezelfde poging met een ander id wordt dus opgeruimd bij brokerstart en bij elke create. Daarnaast ruimt release na het eigen rm ook de nooit gestarte resten van die poging op. De gestopte scope zelf staat op exited en valt dus buiten status=created. Er is een regressietest met twee container-ID's bij gekomen (oud e…, journal c…): de oude container wordt bij een herstart verwijderd, de gejournalde blijft staan, en na start, stop en release is ook een opnieuw verschenen oude container weg. Die test faalde eerst en slaagt nu. Broker-bestanden 44/44, npm test 1042 groen (plus de 2 bekende macOS-failures), tsc groen.

BLOCKER verwerkt in `b872855`. De bescherming volgt nu het gejournalde container-id in plaats van het attempt-label: `removeUnstarted` slaat alleen `attempts.get(attemptId).binding.scope.scopeId === id` en pogingen die nog worden aangemaakt over, zowel in de label-modus als in de modus per poging. Een oude, nooit gestarte container van dezelfde poging met een ander id wordt dus opgeruimd bij brokerstart en bij elke create. Daarnaast ruimt `release` na het eigen `rm` ook de nooit gestarte resten van die poging op. De gestopte scope zelf staat op `exited` en valt dus buiten `status=created`. Er is een regressietest met twee container-ID's bij gekomen (oud `e…`, journal `c…`): de oude container wordt bij een herstart verwijderd, de gejournalde blijft staan, en na start, stop en release is ook een opnieuw verschenen oude container weg. Die test faalde eerst en slaagt nu. Broker-bestanden 44/44, `npm test` 1042 groen (plus de 2 bekende macOS-failures), tsc groen.

janpeter referenced this pull request from a commit 2026-10-02 21:03:56 +02:00

fix(dispatch): bescherming volgt het gejournalde container-id, release ruimt late wees op (review #97)

janpeter added 1 commit 2026-10-02 21:03:56 +02:00

fix(dispatch): bescherming volgt het gejournalde container-id, release ruimt late wees op (review #97) ... b872855515

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

s4m-codex-reviewer reviewed 2026-10-02 21:06:56 +02:00

s4m-codex-reviewer left a comment

Copy link

COMMENT

Geen blokkerende bevinding vastgesteld bij statische beoordeling, maar door de onderstaande opmerkingen en beperkte testverificatie geen goedkeuring.

Findings

• warning — lib/dispatch-runtime-broker.ts:143: De catch omvat de volledige opruimlus. Als inspect/rm van één container faalt (bijvoorbeeld verdwenen tussen ps en inspect), worden alle volgende containers overgeslagen. Vang fouten per container af zodat één hardnekkige fout andere weescontainers niet bij iedere ronde blijft blokkeren; voeg een regressietest met twee kandidaten toe.
• info — tests/dispatch-broker.test.ts:152: De testnaam claimt bescherming van foreign containers, maar de test maakt geen container van een andere slot/attempt aan. Voeg zo'n kandidaat en een gelijktijdige create toe om de labelfilters en creating-bescherming expliciet te toetsen.

Beoordeling en verificatie

De diff op commit b872855515 sluit aan op het bestaande broker-/journalpatroon: create krijgt meer tijd, bescherming volgt het gejournalde container-id en de nieuwe opruiming gebruikt rm zonder force. De operatorhandleiding beschrijft de gewijzigde time-outs en opruimroutes. Product-docs architecture/overview en runbooks/agent-guidance zijn geraadpleegd; deze bevatten geen specifiekere dispatch-opruimstandaard.

Typecheck geslaagd met node node_modules/typescript/bin/tsc -p tsconfig.dispatch.json, na initialisatie van het gepinde shared-submodule. De volledige Vitest-suite kon niet starten: de omgeving weigert executables en native module-mapping onder /tmp (Permission denied / failed to map segment from shared object). Dit is geen bewezen PR-regressie, maar de regressietests zijn daardoor niet uitvoerend bevestigd. Docker-integratietests zijn niet uitgevoerd.

geen gekoppeld plan gevonden — beoordeeld op codekwaliteit + product-standaarden.

# COMMENT Geen blokkerende bevinding vastgesteld bij statische beoordeling, maar door de onderstaande opmerkingen en beperkte testverificatie geen goedkeuring. ## Findings - **warning** — lib/dispatch-runtime-broker.ts:143: De catch omvat de volledige opruimlus. Als inspect/rm van één container faalt (bijvoorbeeld verdwenen tussen ps en inspect), worden alle volgende containers overgeslagen. Vang fouten per container af zodat één hardnekkige fout andere weescontainers niet bij iedere ronde blijft blokkeren; voeg een regressietest met twee kandidaten toe. - **info** — __tests__/dispatch-broker.test.ts:152: De testnaam claimt bescherming van foreign containers, maar de test maakt geen container van een andere slot/attempt aan. Voeg zo'n kandidaat en een gelijktijdige create toe om de labelfilters en creating-bescherming expliciet te toetsen. ## Beoordeling en verificatie De diff op commit b872855515bf717338f9bcb189ec32b8560d4c75 sluit aan op het bestaande broker-/journalpatroon: create krijgt meer tijd, bescherming volgt het gejournalde container-id en de nieuwe opruiming gebruikt rm zonder force. De operatorhandleiding beschrijft de gewijzigde time-outs en opruimroutes. Product-docs architecture/overview en runbooks/agent-guidance zijn geraadpleegd; deze bevatten geen specifiekere dispatch-opruimstandaard. Typecheck geslaagd met `node node_modules/typescript/bin/tsc -p tsconfig.dispatch.json`, na initialisatie van het gepinde shared-submodule. De volledige Vitest-suite kon niet starten: de omgeving weigert executables en native module-mapping onder /tmp (`Permission denied` / `failed to map segment from shared object`). Dit is geen bewezen PR-regressie, maar de regressietests zijn daardoor niet uitvoerend bevestigd. Docker-integratietests zijn niet uitgevoerd. geen gekoppeld plan gevonden — beoordeeld op codekwaliteit + product-standaarden.

janpeter merged commit 1930c1cb43 into master 2026-10-02 21:09:30 +02:00

janpeter referenced this pull request from a commit 2026-10-02 21:09:31 +02:00

Merge pull request 'fix(dispatch): broker-create onder belasting laat geen wees achter (M41 T-1956)' (#97)

janpeter referenced this pull request 2026-10-02 21:13:53 +02:00

fix(dispatch): opruimlus vangt fouten per container af (follow-up #97) #98

janpeter referenced this pull request from a commit 2026-10-02 21:13:54 +02:00

fix(dispatch): opruimlus vangt fouten per container af (follow-up review #97)

janpeter referenced this pull request from a commit 2026-10-02 21:39:28 +02:00

Merge pull request 'fix(dispatch): opruimlus vangt fouten per container af (follow-up #97)' (#98)

Sign in to join this conversation.

Reviewers

No reviewers

s4m-codex-reviewer

Labels

Clear labels   

severity/s3

No labels

severity/s3

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

2 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

janpeter/scrum4me-docker!97

No description provided.