Files
gpu-rent/docs/reviews/2026-08-21-review-3.md
T
Leonid Pershin 3c8225a69e Implement GPU environment verification in session management
- Added a new function `verify_gpu_env` to check GPU stack readiness, including nvidia-smi, CUDA, and torch in the Comfy virtual environment when SwarmUI is enabled.
- Updated the session management to call `verify_gpu_env`, capturing GPU environment status and errors in the state notes.
- Enhanced documentation in `cli.md` to reflect the new GPU environment verification process.
- Added tests for `verify_gpu_env` to ensure proper functionality and error handling during GPU checks.
2026-08-21 06:58:23 +03:00

3.9 KiB
Raw Blame History

Project Review — 2026-08-21 (3)

Critical-only pass (мелочи / style / UX polish ignored). Focus: billing safety, silent GPU left on, security of secrets, broken stop/auto-stop paths.

Prior Reviews Summary

Based on 2026-08-21-review-2.md and 2026-08-21-review-1.md.

Still Open (carried forward)

None. All tasks in review-1 and review-2 are [x].

Resolved Since Last Review

  • Prior review-2 closed the fail-closed idle-killer access_rules + revoke on stop, Ollama pull exact-tag, provision_llm not swallowed, etc. Those remain in place; this review finds new regressions/gaps around arm false-positive and unreachable Swarm.

Phase 1: Code Quality

SOLID

No critical issues (god-flow in session/provision is known debt, not a break).

Performance

No critical issues.

Correctness & Bugs

  1. arm_idle_killer soft-fails on missing app cred, but provision still marks armed.
    create_application_credential failure is caught, logged (idle-killer слеп), and returns without raising. Caller always sets notes["idle_killer"] = "armed". Access card only warns on "failed". → User thinks auto-stop works; GPU bills forever.

  2. Remote idle-killer treats any Swarm HTTP error as busy forever.
    swarm_busy: unreachable → (True, "swarm unreachable…"), which resets idle timer every tick. No grace/timeout to delete when Swarm is down for hours. → Crashed Swarm / bad Comfy ExtraArgs → never delete.

  3. provision_llm / seed failures can leave ACTIVE compute before killer arm.
    Killer is armed last in provision_vm. Earlier CloudError aborts up with server already created and no timer. Known risk amplified by LLM/extensions paths.

  4. Perf tune sets pip_ok=True even when pip install triton/sageattention fails, then may still patch --use-sage-attention and restart Swarm. Marker prevents retry. → Can leave Swarm broken → compounds (2).

Code Quality

No critical issues.


Phase 2: Logical Consistency

Domain & Application Layer

No critical issues.

Data Flow

Requires-filter for extensions / Ollama install paths / balance notify (notify-only) — no critical billing harm found.

State Management

notes.idle_killer = "armed" does not match actual arm success (see bug 1).

Consistency

No critical issues.


Phase 3: UI/UX

Usability

Misleading “armed” status when killer is blind — treated as Bug/Logic above, not UX polish.

Visual / Interaction / Accessibility

N/A for CLI critical pass (or no critical issues).


Tasks

Critical only:

  • 1. [Bug] arm_idle_killer: on cred create failure raise or return False; never set notes.idle_killer=armed; surface same ⚠ as "failed"src/gpu_rent/idle_killer.py line 151, src/gpu_rent/provision.py line 516
  • 2. [Bug] Idle-killer: if Swarm unreachable longer than N minutes (e.g. 2× idle or fixed 60m), treat as idle/allow delete (llm-only already bypasses) — src/gpu_rent/remote/idle_killer.py line 75
  • 3. [Logic] On mid-up failure after server create, arm killer anyway or fail loudly and refuse to leave session without killer / document mandatory stopsrc/gpu_rent/provision.py / session.py
  • 4. [Bug] Perf tune: set pip_ok only if pip succeeded; do not write --use-sage-attention ExtraArgs unless install OK (or allow retry when pip_ok false) — src/gpu_rent/remote/tune_swarm_perf.py line 109
  • 5. [Security] Write idle-killer creds JSON with mode=0o600 before mv (same as GIT_TOKEN) — src/gpu_rent/idle_killer.py line 156

Verified OK (critical): cmd_stop delete path, tunnel Ctrl+C detach, local-watchdog stop on stale lease, requires: ollama filter, balance notify (toast only), Ollama unit bind to data dir, 127 tests green at review time.