Skip to content

Commit cde62f6

Browse files
pt9912claude
andcommitted
Replace lock-eviction list-of-pflichten with formal state machine
Three review findings addressed: - HIGH: The previous DoD had three semantically inconsistent claims (acquire-path rejects tombstone, aborted-remove implies acquire bumps lease on tombstoned entry, re-add says caller should wait not GetOrAdd). Replaced with an explicit three-state machine (Live/Tombstoning/Removed) with a transition table plus formal acquire and sweep algorithms. Acquire treats Tombstoning as "no lease, retry"; sweep treats leaseCount > 0 as "CAS back to Live". Four mandatory unit tests now consolidated and named in one place (previously the aufwand line counted "zwei" while the text introduced more). - Cluster-smoke stage-1 failure handling now explicit: GitHub Actions UI + manual triage, no PR block. Automated issue creation is explicitly out of scope (separate follow-up slice if wanted) so the cluster-smoke slice does not implicitly inherit issue-automation work. - LOW: Kandidat A is no longer described as "klein". New wording: "concurrency-kritisch, scope-mässig komplexitätsarm" with an explanatory sentence that the 4-5 PT estimate reflects test and contract surface, not broad feature scope. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 6324c61 commit cde62f6

1 file changed

Lines changed: 145 additions & 120 deletions

File tree

docs/plan/planning/next/note-v1.1.0-scope.md

Lines changed: 145 additions & 120 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,14 @@ Diese Notiz fixiert pro Kandidat:
4444

4545
### Kandidat A: RM-M3-FUP-03 — Optimization-Lock-Eviction (OP-OPEN-06)
4646

47-
**Pflicht-Kandidat, klein, trigger-frei rechtfertigbar.**
47+
**Pflicht-Kandidat, concurrency-kritisch, scope-mässig
48+
komplexitätsarm, trigger-frei rechtfertigbar.** Der Slice ist
49+
**inhaltlich klein** (gemeinsamer Lease-Helper für zwei Use Cases,
50+
neuer Observability-Port, präventive Hardening-Maßnahme), aber
51+
**concurrency-technisch nicht trivial**: er führt eine Lease-/
52+
Tombstone-State-Machine ein, die unter Eviction-Race-Bedingungen
53+
korrekt sein muss. Aufwand 4-5 PT spiegelt den Test- und
54+
Vertrags-Anteil, nicht eine breite Feature-Fläche.
4855

4956
- **Quelle:** [`note-RM-M3-followups.md` Item 7](../open/note-RM-M3-followups.md).
5057
- **Stand heute:** **Zwei** Use Cases halten unbounded
@@ -109,123 +116,135 @@ Diese Notiz fixiert pro Kandidat:
109116
Hardening-Maßnahme; das Risiko-Profil verschlechtert sich mit
110117
jeder produktiven Stunde stillschweigend (Memory-Leak-Klasse),
111118
während der Fix klein und reversibel ist.
112-
- **Vertrags-Erhalt (Pflicht-DoD):** Die Serialisierungs-Garantie
113-
(„zwei parallele Aufrufe für denselben Key dürfen nicht denselben
114-
Base-Stand lesen und gegenseitig überschreiben") darf durch
115-
Eviction **nicht gebrochen werden**. `SemaphoreSlim` exponiert
116-
keinen öffentlichen Waiter-Count, und das naive
117-
Dictionary-Lookup-dann-Increment-Pattern hat eine Race: Eviction
118-
kann zwischen Lookup und Lease-Increment dieselbe Instanz
119-
entfernen. Daher konkrete Pflichten:
120-
- **Lease-Reservierung als atomar-verifizierte Operation
121-
(`TryAcquireLease`):** Pro Key wird ein Eintrag mit
122-
`(SemaphoreSlim semaphore, int leaseCount, int generation,
123-
bool tombstoned)` geführt. Acquire läuft als Schleife:
124-
(1) Eintrag lookup, (2) `tombstoned`-Flag prüfen,
125-
(3) `leaseCount` via `Interlocked.Increment` reservieren,
126-
(4) **nach** der Reservierung verifizieren, dass der
127-
Dictionary-Eintrag noch dieselbe aktive Generation hat
128-
(Instanz-Identität oder `generation`-Vergleich) — wenn nicht,
129-
Lease via `Interlocked.Decrement` zurücknehmen und mit
130-
`GetOrAdd` neu laden. Nur ein im selben Schritt verifizierter
131-
Lease darf auf `WaitAsync` gehen. Release dekrementiert
132-
`leaseCount` nach `semaphore.Release`.
133-
- **Cancellation-Pflicht (`WaitAsync` mit `CancellationToken`):**
134-
Wenn `WaitAsync(cancellationToken)` per
135-
`OperationCanceledException` abbricht, **wurde der Semaphore
136-
nicht gehalten**`Release()` darf nicht aufgerufen werden
137-
(würde `SemaphoreFullException` werfen), aber der bereits
138-
reservierte `leaseCount` muss via `Interlocked.Decrement`
139-
zurückgenommen werden. Sonst bleiben abgebrochene Aufrufer
140-
als künstlich aktive Leases hängen und Eviction wird dauerhaft
141-
blockiert. Standard-Muster: separater `try/catch` um
142-
`WaitAsync` für Lease-Rollback bei Cancellation, dann separater
143-
`try/finally` für `Release` + Lease-Decrement im Happy-Path.
144-
Pflicht-Unit-Test: Caller, dessen `CancellationToken` zwischen
145-
Lease-Reservierung und Semaphore-Akquise gefeuert wird, hinter­
146-
lässt `leaseCount == 0`.
147-
- **Idle-only mit Tombstone-Pattern:** Eviction-Kandidaten
148-
werden anhand „seit X Sekunden ohne Acquire/Release" gewählt.
149-
Eviction setzt zuerst `tombstoned = true` (per
150-
`Interlocked.CompareExchange` auf einen Status-Slot), prüft
151-
dann `leaseCount == 0`, und entfernt erst danach den Eintrag
152-
via **conditional remove mit Instanz-Identität** (semantisch:
153-
„entferne diesen Eintrag nur, wenn er noch genau diese
154-
Instanz ist"). Die exakte API-Form (Cast auf
155-
`ICollection<KeyValuePair<,>>` und `Remove(pair)`, eigene
156-
Tombstone-CAS, oder ein verfügbarer
157-
`TryRemove(KeyValuePair)`-Overload) ist Slice-Plan-
158-
Entscheidung — der Vertrag ist die Instanz-bedingte
159-
Entfernung, nicht eine spezifische BCL-Methode.
160-
- **Abgebrochener Remove (Tombstone bleibt, Eintrag bleibt
161-
aktiv):** Wenn der Final-Check `leaseCount == 0` scheitert,
162-
weil zwischen Tombstone-Setzen und Final-Check ein neuer
163-
Acquire das Lease hochgezogen hat, **darf** der tombstoned
164-
Eintrag nicht im Dictionary verharren. Sonst spinnt jeder
165-
neue Acquirer im Re-Load-Pfad (`GetOrAdd` ersetzt einen
166-
existierenden Eintrag nicht). Bei abgebrochenem Remove **muss**
167-
das `tombstoned`-Flag per CAS auf `false` zurückgesetzt
168-
werden (selbe Instanz wird wieder normal nutzbar — alte
169-
Acquirer-Referenzen und der gerade hochgezogene neue
170-
Acquirer benutzen denselben Semaphore und bleiben
171-
serialisiert). **Replacement durch eine frische Generation
172-
ist hier explizit verboten**, weil das die Per-Key-
173-
Serialisierung bricht: alte Caller auf der alten Semaphore
174-
und neue Caller auf der neuen Semaphore würden parallel in
175-
denselben Critical Section eintreten. Replacement passiert
176-
ausschließlich auf dem normalen Eviction-Pfad **nachdem**
177-
`leaseCount == 0` final bestätigt und der Eintrag entfernt
178-
wurde — der nächste `GetOrAdd` legt dann eine frische
179-
Generation an. Pflicht-Unit-Test: „Tombstone gesetzt während
180-
aktive Lease hält; neuer Acquire kommt **vor** finalem
181-
Remove" — beweist dass weder Spin noch Deadlock entsteht,
182-
der neue Acquire auf demselben Semaphore landet, und
183-
Serialisierung erhalten bleibt.
184-
- **Dispose der entfernten Semaphore:** Nach erfolgreicher
185-
Entfernung aus dem Dictionary **und** finaler Bestätigung
186-
`leaseCount == 0` muss die `SemaphoreSlim`-Instanz disposed
187-
werden (sie hält intern unmanaged-Ressourcen, insbesondere
188-
ein lazy `AvailableWaitHandle`). Ein vergessener Dispose
189-
reduziert den Dictionary-Leak, lässt aber Semaphore-Ressourcen
190-
unnötig liegen.
191-
- **Race-sicheres Re-Add:** Wenn `TryAcquireLease` einen
192-
Tombstone oder Generations-Mismatch entdeckt, läuft
193-
`GetOrAdd` **nicht** sofort — dieser würde den noch nicht
194-
entfernten tombstoned Eintrag zurückliefern und der Caller
195-
spinnt. Stattdessen retry-Schleife mit drei möglichen
196-
Auflösungen:
197-
(a) **Tombstone wurde zurückgesetzt** (CAS auf `false`,
198-
siehe Eviction-Abandoned-Pfad unten) — Caller liest den
199-
Eintrag erneut und reserviert auf derselben Instanz.
200-
(b) **Tombstone-Eintrag ist inzwischen entfernt** (Eviction
201-
hat den conditional remove erfolgreich abgeschlossen) —
202-
`GetOrAdd` legt eine frische Instanz an.
203-
(c) **Eviction-Sweep ist noch in der Tombstone→Remove-
204-
Phase** — Caller wartet kurz (bounded backoff, z. B. einige
205-
`SpinWait`/`Thread.Yield` und bei Erfolglosigkeit
206-
`await Task.Yield()`) und versucht erneut. Hartes Timeout
207-
nach N Iterationen (Slice-Plan: konkreter Wert) wirft, damit
208-
pathologische Sweep-Bugs nicht unbegrenzt blockieren statt
209-
sichtbar zu failen.
210-
- **Concurrency-Test "Acquire racing with Eviction-Remove":**
211-
Pflicht-Unit-Test, der genau die Race-Sequenz „Caller hat
212-
Instanz-Referenz, Eviction setzt Tombstone und entfernt,
213-
Caller ruft Lease-Reservierung auf" trifft und beweist, dass
214-
der Caller entweder (a) auf die neue Generation springt oder
215-
(b) seinen Lease beim Verify-Schritt zurücknimmt und
216-
sauber neu lädt.
217-
- **Concurrency-Test "Sweep während paralleler Calls":**
218-
Zweiter Pflicht-Test, der einen Eviction-Sweep parallel zu
219-
zwei aktiv haltenden Callern fährt und beweist, dass kein
220-
Eintrag mit `leaseCount > 0` entfernt wird.
221-
- **Aufwand:** ~4-5 PT (Slice + gemeinsame Eviction-Logik +
222-
Lease/Refcount-Wrapper + Metrik mit Label + zwei Concurrency-
223-
Tests + Aktualisierung der Ursprungs-Followup-Notiz +
224-
Dokumentation). **Doku-Ort:** wird im Slice-Plan festgelegt;
225-
`quality.md` §6 ist Native-/.NET-Parity (nicht passend),
226-
Persistenz-Doku ist ebenfalls fachfremd — vermutlich neuer
227-
Operations-/Metrik-Abschnitt in `quality.md` oder kurze
228-
Betriebsnotiz im Application-User-Doc.
119+
- **Vertrags-Erhalt (Pflicht-DoD) — State-Machine-formal:** Die
120+
Serialisierungs-Garantie („zwei parallele Aufrufe für denselben
121+
Key dürfen nicht denselben Base-Stand lesen und gegenseitig
122+
überschreiben") darf durch Eviction **nicht gebrochen werden**.
123+
`SemaphoreSlim` exponiert keinen öffentlichen Waiter-Count, und
124+
Lookup-then-Increment ist nicht atomar. Statt einer losen Liste
125+
von „Pflichten" wird **eine einzige State-Machine** pro
126+
Dictionary-Eintrag definiert — das schliesst die früheren
127+
Mehrdeutigkeiten zwischen Acquire-Pfad, Aborted-Remove und
128+
Re-Add aus.
129+
130+
**Pro Key gilt ein Eintrag `(SemaphoreSlim semaphore,
131+
int leaseCount, int state)` mit dem `state`-Feld in genau drei
132+
Werten:** `Live` (Default beim Anlegen), `Tombstoning` (Eviction
133+
hat den Eintrag zur Entfernung markiert), `Removed` (Eviction
134+
hat den Eintrag aus dem Dictionary entfernt — Endzustand für
135+
alte Referenzen). Alle Zustands-Transitions sind CAS.
136+
137+
**Erlaubte Transitions** (alles andere ist ein Bug):
138+
139+
| Von | Nach | Auslöser |
140+
| ------------ | ------------- | ---------------------------------------------------------------------------------------------------------------- |
141+
| Live | Tombstoning | Eviction-Sweep wählt Eintrag als idle-Kandidat (CAS muss erfolgreich sein, sonst Sweep überspringt). |
142+
| Tombstoning | Live | Sweep-Final-Check sieht `leaseCount > 0` (Race mit Acquire) und bricht Eviction ab. |
143+
| Tombstoning | Removed | Sweep-Final-Check sieht `leaseCount == 0`. Eintrag wird via conditional remove (Instanz-Identität) entfernt + Dispose. |
144+
145+
**Acquire-Algorithmus (`TryAcquireLease`):**
146+
1. Eintrag per `GetOrAdd` lookup (anlegen falls nicht vorhanden;
147+
neue Instanz startet in `Live`).
148+
2. CAS-Lesen des `state`-Feldes:
149+
- `Live`: weiter zu Schritt 3.
150+
- `Tombstoning`: **kein** `leaseCount`-Increment. Retry-
151+
Schleife mit bounded Backoff (`SpinWait` + bei
152+
Erfolglosigkeit `await Task.Yield()`), maximal N Iterationen.
153+
Beim Retry erneut Schritt 1 (Eintrag ist inzwischen `Live`
154+
(Sweep abgebrochen) oder durch eine frische Instanz ersetzt
155+
(Sweep erfolgreich + neuer GetOrAdd)). Hartes Timeout nach N
156+
Iterationen wirft, damit pathologische Sweep-Bugs sichtbar
157+
werden.
158+
- `Removed`: alte Referenz; `GetOrAdd` in Schritt 1 hat
159+
bereits die frische Instanz geliefert (Endzustand-Caller
160+
sind selten — entstehen nur, wenn ein Acquirer eine
161+
Referenz über die Removed-Transition hinweg festhält).
162+
Mit der frischen Instanz weitermachen.
163+
3. `leaseCount` per `Interlocked.Increment` reservieren.
164+
4. **Post-Reserve-Verify**: `state` erneut lesen. Wenn nicht mehr
165+
`Live` (Eviction war seit Schritt 2 schneller), Lease per
166+
`Interlocked.Decrement` zurücknehmen und zurück zu Schritt 1.
167+
5. Nur ein in Schritt 4 verifizierter Lease darf auf
168+
`semaphore.WaitAsync(cancellationToken)` gehen.
169+
170+
**Cancellation-Pflicht im Acquire (`WaitAsync`):** Wenn
171+
`WaitAsync(ct)` per `OperationCanceledException` abbricht,
172+
**wurde der Semaphore nicht gehalten**`semaphore.Release()`
173+
darf nicht aufgerufen werden (würde `SemaphoreFullException`
174+
werfen), aber der bereits reservierte `leaseCount` muss via
175+
`Interlocked.Decrement` zurückgenommen werden. Sonst bleiben
176+
abgebrochene Caller als künstlich aktive Leases hängen und
177+
Eviction wird dauerhaft blockiert. Standard-Muster: separater
178+
`try/catch (OperationCanceledException)` um `WaitAsync` für
179+
Lease-Rollback, dann separater `try/finally` für `Release` +
180+
Lease-Decrement im Happy-Path.
181+
182+
**Eviction-Sweep-Algorithmus:**
183+
1. Eintrag als idle-Kandidat identifiziert (TTL/LRU-Kriterium).
184+
2. CAS `state: Live → Tombstoning`. Wenn die CAS fehlschlägt
185+
(Eintrag ist nicht mehr `Live`), Sweep überspringt den
186+
Kandidaten.
187+
3. **Final-Check** `leaseCount == 0`:
188+
- **Ja**: CAS `state: Tombstoning → Removed`, conditional remove
189+
aus dem Dictionary (Instanz-Identität — exakte BCL-API ist
190+
Slice-Plan-Entscheidung: Cast auf
191+
`ICollection<KeyValuePair<,>>` mit `Remove(pair)`, oder
192+
verfügbarer `TryRemove(KeyValuePair)`-Overload), dann
193+
`semaphore.Dispose()`. (`SemaphoreSlim` hält intern
194+
unmanaged-Ressourcen, insbesondere ein lazy
195+
`AvailableWaitHandle` — Dispose ist Pflichtbestandteil
196+
dieser Transition.)
197+
- **Nein** (zwischen Schritt 2 und Final-Check hat ein Caller
198+
Schritt 3 des Acquire ausgeführt; sein Verify in Schritt 4
199+
wird zwar fehlschlagen und Lease zurücknehmen, aber im
200+
Zeitfenster vor dem Decrement ist `leaseCount > 0`
201+
sichtbar): CAS `state: Tombstoning → Live`. Eintrag bleibt
202+
aktiv. Beim nächsten Sweep neu evaluiert.
203+
204+
**Warum diese State-Machine die Per-Key-Serialisierung erhält:**
205+
Der Verify-Schritt-4 stellt sicher, dass kein Caller mit
206+
reserviertem Lease auf `WaitAsync` geht, während der Eintrag
207+
`Tombstoning` ist. Wenn Eviction abbricht (`Tombstoning → Live`),
208+
bleibt **derselbe Semaphore** im Dictionary — alle Caller (alte
209+
und neue) sehen dieselbe Instanz und bleiben serialisiert. Wenn
210+
Eviction erfolgreich entfernt (`Tombstoning → Removed`), legt der
211+
nächste `GetOrAdd` eine frische `Live`-Instanz an; alte
212+
Referenzen werden in Schritt 4 oder Schritt 2 verworfen.
213+
Generation-Wechsel und Critical-Section-Parallelität sind
214+
ausgeschlossen.
215+
216+
**Vier Pflicht-Unit-Tests namentlich (Race-Coverage):**
217+
(i) **„Acquire racing with Eviction-Removal":** Caller hat
218+
Instanz-Referenz, Eviction führt `Tombstoning → Removed` durch
219+
+ neuer `GetOrAdd` legt frische Instanz an, Caller versucht
220+
Lease-Reservierung — beweist dass Caller entweder auf die
221+
frische Instanz wechselt (Removed-Pfad in Schritt 2) oder Lease
222+
zurücknimmt und neu lädt.
223+
(ii) **„Sweep während paralleler Calls":** Eviction-Sweep
224+
parallel zu zwei aktiv haltenden Callern — beweist dass die
225+
CAS `Live → Tombstoning` in Schritt 2 des Sweeps oder der
226+
Final-Check `leaseCount == 0` in Schritt 3 keinen Eintrag mit
227+
aktivem Lease entfernt.
228+
(iii) **„Cancellation zwischen Lease-Reservierung und
229+
Semaphore-Akquise":** Caller-`CancellationToken` feuert nach
230+
Schritt 3 (Lease reserviert) aber vor `WaitAsync`-Erfolg —
231+
beweist dass `leaseCount == 0` zurückbleibt und kein
232+
`Release()` aufgerufen wird.
233+
(iv) **„Sweep-Abort bei Lease-Race":** Eviction setzt
234+
Tombstoning, parallel reserviert Caller Lease und sieht
235+
Tombstoning im Verify (Schritt 4) → Decrement; Sweep sieht
236+
trotzdem im Final-Check kurz `leaseCount > 0` → CAS
237+
`Tombstoning → Live` — beweist dass derselbe Semaphore weiter
238+
benutzt wird und Serialisierung erhalten bleibt.
239+
240+
- **Aufwand:** ~4-5 PT (Slice + gemeinsamer Lease-Helper mit
241+
State-Machine + Metrik mit Label + **vier** Concurrency-Tests
242+
+ Aktualisierung der Ursprungs-Followup-Notiz + Dokumentation).
243+
**Doku-Ort:** wird im Slice-Plan festgelegt; `quality.md` §6 ist
244+
Native-/.NET-Parity (nicht passend), Persistenz-Doku ist
245+
ebenfalls fachfremd — vermutlich neuer Operations-/Metrik-
246+
Abschnitt in `quality.md` oder kurze Betriebsnotiz im
247+
Application-User-Doc.
229248
- **Slice-Plan:** `plan-RM-M3-FUP-03.md` (entsteht in `open/`
230249
`in-progress/``done/`).
231250
- **Offene Entscheidungen** (Coverage **nicht** mehr offen —
@@ -375,8 +394,14 @@ Diese Notiz fixiert pro Kandidat:
375394
Workflow-Datei + ggf. `scripts/helm-cluster-smoke*`) plus
376395
**unconditional** nightly auf `main` (kein Path-Filter, weil
377396
Scheduled Runs keinen PR-Diff haben und die Stabilitäts-
378-
Serie sonst nicht beweisbar ist). Failures werden im Issue-
379-
Tracker erfasst, blocken aber keine PRs.
397+
Serie sonst nicht beweisbar ist). **Failure-Behandlung:**
398+
Workflow-Run zeigt Fehler im GitHub-Actions-UI; manuelle
399+
Triage durch das Team beim Stand-up oder per
400+
Workflow-Notification (kein PR-Block). Automatische
401+
Issue-Anlage ist **explizit nicht** Bestandteil dieses Slices —
402+
falls gewünscht, ist das ein eigener Folge-Slice
403+
(z. B. `peter-evans/create-issue-from-file` oder ähnlich),
404+
der den v1.1.0-Cluster-Smoke-Scope nicht aufbläht.
380405
2. **Nach 4 Wochen ununterbrochen grünem Nightly:** Promotion
381406
zu `required`. **Pflicht-Begleitänderung:** Trigger wird auf
382407
`pull_request` ohne `paths`-Filter umgestellt und der Job

0 commit comments

Comments
 (0)