[P2][BUG] Speicherleck: _UPDATE_RUNS und partial_outputs werden nur bei vollständigem Client-Poll aufgeräumt #228

Open
opened 2026-07-27 10:44:23 +00:00 by chinux · 1 comment
Owner

Befund

Zwei In-Memory-Strukturen halten Kommando-Ausgaben und werden nur aufgeräumt, wenn der Client bis zum Ende pollt.

_UPDATE_RUNS (routers/docker_router.py:21) wird in docker_router.py:185 nur entfernt, wenn der Client die Ausgabe vollständig abgeholt hat:

if run["done"]:
    if offset + len(chunk) >= len(full):
        _UPDATE_RUNS.pop(cmd_id, None)

Schließt der Nutzer den Tab, verliert das Frontend die Verbindung oder bricht der Poll ab, bleibt der Eintrag samt vollständiger Ausgabe für die Lebensdauer des Prozesses liegen. Kein TTL, kein GC.

manager.partial_outputs (websocket/manager.py:40) ist laut Kommentar in Zeile 200 bewusst nicht in disconnect() aufgeräumt. Entfernt wird nur an sechs Stellen in den Routern — jeweils im Erfolgspfad. Bricht der Pfad vorher ab, bleibt der Eintrag.

Auswirkung

Kein Absturz, sondern langsames Wachstum: jeder abgebrochene Docker-Update-, Host-Update- oder Bulk-Update-Lauf hinterlässt seine Ausgabe im Speicher. Bei mehr Nodes und mehr Läufen wächst das monoton bis zum nächsten Neustart. Da Updates typischerweise die längsten und ausgabestärksten Kommandos sind, sind die Einträge nicht klein.

_GUAC_SESSIONS (rdp_router.py:27) macht es richtig — TTL von 60 s plus _gc_guac_sessions() bei jedem Zugriff. Das Muster existiert im Projekt also schon und wird hier nur nicht angewandt.

Aufwand

Über 30 Minuten, weil an mehreren Stellen eingegriffen wird und die Aufräumbedingung nicht ohne Blick auf die Poll-Logik geändert werden kann — deshalb ein Issue statt eines Nebenfixes.


Prompt für Claude Code

Beseitige das unbegrenzte Wachstum der In-Memory-Ausgabepuffer in theProx.
Vorlage für das Muster: _GUAC_SESSIONS / _gc_guac_sessions() in
routers/rdp_router.py — dort ist es bereits korrekt gelöst.

--- 1. GEMEINSAMER PUFFER MIT TTL ---
Neu: server/util/run_buffer.py

    class RunBuffer:
        """cmd_id -> {payload, done, ts}. TTL-basiert, kein Verlass auf Clients."""
        def put(self, cmd_id, value): ...
        def get(self, cmd_id): ...
        def mark_done(self, cmd_id): ...
        def pop(self, cmd_id): ...
        def gc(self): ...          # bei jedem Zugriff aufrufen
        def stats(self) -> dict    # {entries, bytes} für Diagnose

TTL zweistufig, als Konstanten:
    TTL_RUNNING = 4 h    -- laufender Lauf, Client könnte noch pollen
    TTL_DONE    = 15 min -- fertig; nach dieser Zeit holt niemand mehr ab

Zusätzlich eine Obergrenze MAX_ENTRIES (Default 200): beim Überschreiten die
ältesten FERTIGEN Einträge zuerst verwerfen, laufende nie. Wird die Grenze
durch laufende Einträge erreicht, WARN loggen — dann liegt ein echtes Problem
vor und stilles Verwerfen würde es verdecken.

--- 2. _UPDATE_RUNS UMSTELLEN ---
docker_router.py auf RunBuffer umstellen. Die bestehende Aufräumbedingung
(Client hat alles abgeholt) BLEIBT als Schnellpfad — sie ist nicht falsch, nur
unzureichend. Der TTL ist das Netz darunter.

Die Prüfung aus #186 (cmd_id muss zu node_name/vmid passen) muss erhalten
bleiben — sie verhindert Cross-VM-Lesen und darf beim Umbau nicht verloren
gehen.

Gleiches Vorgehen für die Poll-Pfade in update_router.py (Zeilen ~81, ~112,
~486, ~549) und deploy_router.py (~1380), soweit sie dieselbe Struktur nutzen.

--- 3. partial_outputs ---
websocket/manager.py: partial_outputs auf RunBuffer umstellen.

Der Kommentar in Zeile 200 begründet, warum in disconnect() NICHT aufgeräumt
wird (der Client soll die Ausgabe eines abgebrochenen Laufs noch lesen können).
Diese Absicht beibehalten — der TTL löst es, nicht ein Aufräumen bei
disconnect. Kommentar entsprechend aktualisieren, statt ihn zu entfernen.

--- 4. GC ANSTOSSEN ---
gc() bei jedem put/get aufrufen (wie _gc_guac_sessions). Kein zusätzlicher
Hintergrund-Task — bei völliger Inaktivität wächst auch nichts.

--- 5. DIAGNOSE ---
Den bestehenden Superadmin-Diagnosepfad um RunBuffer.stats() erweitern
(Anzahl Einträge, geschätzte Bytes). Ohne Sichtbarkeit ist ein Leck erst am
Speicherverbrauch des Containers erkennbar.

--- 6. TESTS ---
tests/test_run_buffer.py:
  - fertiger Eintrag verschwindet nach TTL_DONE
  - laufender Eintrag überlebt TTL_DONE, verschwindet nach TTL_RUNNING
  - MAX_ENTRIES: ältester FERTIGER wird verworfen, laufender nie
  - nur laufende Einträge über der Grenze -> WARN, kein Verwerfen
  - Client bricht Poll ab -> Eintrag ist nach TTL_DONE weg
  - #186-Prüfung greift weiterhin (falsche node/vmid -> 404)

Definition of Done

  • Kein Puffer-Eintrag überlebt seinen TTL
  • Abgebrochener Client-Poll hinterlässt nichts
  • Obergrenze verwirft nie laufende Läufe
  • #186-Zugriffsprüfung unverändert wirksam
  • Füllstand über Diagnose sichtbar
## Befund Zwei In-Memory-Strukturen halten Kommando-Ausgaben und werden nur aufgeräumt, wenn der Client bis zum Ende pollt. **`_UPDATE_RUNS`** (`routers/docker_router.py:21`) wird in `docker_router.py:185` nur entfernt, wenn der Client die Ausgabe vollständig abgeholt hat: ```python if run["done"]: if offset + len(chunk) >= len(full): _UPDATE_RUNS.pop(cmd_id, None) ``` Schließt der Nutzer den Tab, verliert das Frontend die Verbindung oder bricht der Poll ab, **bleibt der Eintrag samt vollständiger Ausgabe für die Lebensdauer des Prozesses liegen**. Kein TTL, kein GC. **`manager.partial_outputs`** (`websocket/manager.py:40`) ist laut Kommentar in Zeile 200 bewusst nicht in `disconnect()` aufgeräumt. Entfernt wird nur an sechs Stellen in den Routern — jeweils im Erfolgspfad. Bricht der Pfad vorher ab, bleibt der Eintrag. ## Auswirkung Kein Absturz, sondern langsames Wachstum: jeder abgebrochene Docker-Update-, Host-Update- oder Bulk-Update-Lauf hinterlässt seine Ausgabe im Speicher. Bei mehr Nodes und mehr Läufen wächst das monoton bis zum nächsten Neustart. Da Updates typischerweise die längsten und ausgabestärksten Kommandos sind, sind die Einträge nicht klein. `_GUAC_SESSIONS` (`rdp_router.py:27`) macht es richtig — TTL von 60 s plus `_gc_guac_sessions()` bei jedem Zugriff. Das Muster existiert im Projekt also schon und wird hier nur nicht angewandt. ## Aufwand Über 30 Minuten, weil an mehreren Stellen eingegriffen wird und die Aufräumbedingung nicht ohne Blick auf die Poll-Logik geändert werden kann — deshalb ein Issue statt eines Nebenfixes. --- ## Prompt für Claude Code ``` Beseitige das unbegrenzte Wachstum der In-Memory-Ausgabepuffer in theProx. Vorlage für das Muster: _GUAC_SESSIONS / _gc_guac_sessions() in routers/rdp_router.py — dort ist es bereits korrekt gelöst. --- 1. GEMEINSAMER PUFFER MIT TTL --- Neu: server/util/run_buffer.py class RunBuffer: """cmd_id -> {payload, done, ts}. TTL-basiert, kein Verlass auf Clients.""" def put(self, cmd_id, value): ... def get(self, cmd_id): ... def mark_done(self, cmd_id): ... def pop(self, cmd_id): ... def gc(self): ... # bei jedem Zugriff aufrufen def stats(self) -> dict # {entries, bytes} für Diagnose TTL zweistufig, als Konstanten: TTL_RUNNING = 4 h -- laufender Lauf, Client könnte noch pollen TTL_DONE = 15 min -- fertig; nach dieser Zeit holt niemand mehr ab Zusätzlich eine Obergrenze MAX_ENTRIES (Default 200): beim Überschreiten die ältesten FERTIGEN Einträge zuerst verwerfen, laufende nie. Wird die Grenze durch laufende Einträge erreicht, WARN loggen — dann liegt ein echtes Problem vor und stilles Verwerfen würde es verdecken. --- 2. _UPDATE_RUNS UMSTELLEN --- docker_router.py auf RunBuffer umstellen. Die bestehende Aufräumbedingung (Client hat alles abgeholt) BLEIBT als Schnellpfad — sie ist nicht falsch, nur unzureichend. Der TTL ist das Netz darunter. Die Prüfung aus #186 (cmd_id muss zu node_name/vmid passen) muss erhalten bleiben — sie verhindert Cross-VM-Lesen und darf beim Umbau nicht verloren gehen. Gleiches Vorgehen für die Poll-Pfade in update_router.py (Zeilen ~81, ~112, ~486, ~549) und deploy_router.py (~1380), soweit sie dieselbe Struktur nutzen. --- 3. partial_outputs --- websocket/manager.py: partial_outputs auf RunBuffer umstellen. Der Kommentar in Zeile 200 begründet, warum in disconnect() NICHT aufgeräumt wird (der Client soll die Ausgabe eines abgebrochenen Laufs noch lesen können). Diese Absicht beibehalten — der TTL löst es, nicht ein Aufräumen bei disconnect. Kommentar entsprechend aktualisieren, statt ihn zu entfernen. --- 4. GC ANSTOSSEN --- gc() bei jedem put/get aufrufen (wie _gc_guac_sessions). Kein zusätzlicher Hintergrund-Task — bei völliger Inaktivität wächst auch nichts. --- 5. DIAGNOSE --- Den bestehenden Superadmin-Diagnosepfad um RunBuffer.stats() erweitern (Anzahl Einträge, geschätzte Bytes). Ohne Sichtbarkeit ist ein Leck erst am Speicherverbrauch des Containers erkennbar. --- 6. TESTS --- tests/test_run_buffer.py: - fertiger Eintrag verschwindet nach TTL_DONE - laufender Eintrag überlebt TTL_DONE, verschwindet nach TTL_RUNNING - MAX_ENTRIES: ältester FERTIGER wird verworfen, laufender nie - nur laufende Einträge über der Grenze -> WARN, kein Verwerfen - Client bricht Poll ab -> Eintrag ist nach TTL_DONE weg - #186-Prüfung greift weiterhin (falsche node/vmid -> 404) ``` ## Definition of Done - [ ] Kein Puffer-Eintrag überlebt seinen TTL - [ ] Abgebrochener Client-Poll hinterlässt nichts - [ ] Obergrenze verwirft nie laufende Läufe - [ ] #186-Zugriffsprüfung unverändert wirksam - [ ] Füllstand über Diagnose sichtbar
Author
Owner

Aus dem Code-Review vom 2026-07-27 (Commit 4152c4d). Gesamtblock: #226, #227, #228, #229, #230, #231.

Abarbeitung niedrig → hoch: #231#230#229#228#227#226.

Aus dem Code-Review vom 2026-07-27 (Commit 4152c4d). Gesamtblock: #226, #227, #228, #229, #230, #231. Abarbeitung niedrig → hoch: #231 → #230 → #229 → #228 → #227 → #226.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
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
chinux/theProx#228
No description provided.