toolctrl-labs/HiveGrid-Wireframes
9
0 Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity
Files
3cb93a81bea50927864585d1fafe0af4c4772a09
HiveGrid-Wireframes/hivegrid-cloud/docs/improvement-plan/02-observability/performance-quickwins.md
T

16 KiB
Raw Blame History

Performance Quickwins — 8 Audit-Items in einem PR

Status: completed Task-ID: #36 Phase: 02 Estimated Effort: 1-2 Tage Owner: unassigned

Problem

Aus dem Code-Audit (Phase 00) sind 8 kleine, low-risk-Hot-Path-Optimierungen aufgefallen, die einzeln keinen eigenen Task rechtfertigen, in Summe aber spürbar sind. Wir bündeln sie in einem PR mit atomic Commits pro Fix, weil:

  • Vor/Nachher-Messung sauberer ist wenn die Items zeitnah deployed werden
  • Jeder Fix isoliert revertierbar bleibt
  • Code-Review konzentriert auf "8 kleine Performance-Patches" statt verteilt über 8 PRs

Alle 8 Items sind lokal, ohne Architektur-Auswirkungen, ohne Schema-Änderungen.

Depends on

  • 02-observability/metrics-grafana.md — wir wollen Vorher/Nachher mit /metrics messen (Latency-p95, Prisma-Query-Counts, Redis-Op-Counts)

Blocks

  • Keine. Reine Optimierung.

Approach

Gewählt: Ein Branch agent/phase-02-perf-quickwins, 8 Commits (einer pro Item), Vorher/Nachher-Snapshot der /metrics-relevanten Werte in docs/observability/performance-baseline.md. Falls eine Optimierung Tests bricht oder Regression triggert: nur dieser Commit revertiert, Rest bleibt.

Mess-Methodik:

  • Vorher: Auf Staging oha-Last-Test (z.B. oha -n 5000 -c 50 https://app.<staging-domain>/app/<tenant>/dashboard) → /metrics-Snapshot
  • Nachher: Gleicher Test → Snapshot
  • Vergleich: p50/p95 Latency, total Prisma-Queries, total Redis-Ops, Memory-Peak
  • Dokumentiert in docs/observability/performance-baseline.md mit Tabelle

Implementation Steps

Item 1 — N+1 in Users-Admin-View

Datei: apps/web/app/routes/admin/accounts/users.tsx:119-129 Problem: Pro User in der Liste wird getLastUserLog(user.id) separat aufgerufen → N+1 (bei 100 Users = 101 DB-Queries). Fix: Batch-Query mit IN-Clause oder Prisma groupBy({ by: ['userId'], _max: { createdAt: true } }). Eine Query für alle. Tests: Bestehende Tests für die Route müssen grün bleiben; neuer Test prüft, dass nur 1 Query für User-Logs abgesetzt wird (Prisma-Query-Spy).

  • Files: apps/web/app/routes/admin/accounts/users.tsx, apps/web/app/utils/services/.server/userLogService.ts (oder analog), Tests
  • Commit: 02-observability/perf-quickwins: fix N+1 in admin users list (batched getLastUserLog)

Item 2 — currentTenant Doppel-Fetch

Datei: apps/web/app/utils/data/.server/rootData.ts:30 Problem: currentTenant wird zweimal aus der DB geladen — einmal explizit, einmal innerhalb von setLoggedUser. Doppelte Latenz. Fix: Tenant einmal fetchen, an setLoggedUser als Parameter durchreichen. Tests: Existing root-data Tests; neuer Test asserts 1 Tenant-Query pro Request.

  • Files: apps/web/app/utils/data/.server/rootData.ts, apps/web/app/utils/session.server.ts (für setLoggedUser-Signatur), Tests
  • Commit: 02-observability/perf-quickwins: avoid duplicate tenant fetch in rootData

Item 3 — Storage Signed-URL-Adapter Caching

Datei: apps/api/src/storage.ts:56-80, 82-131 Problem: Pro Signed-URL-Call wird ein neuer S3-Client + Signing-Key konstruiert (Config-Parsing, Crypto-Init). Bei vielen Uploads/Downloads hot loop. Fix: S3-Client + SignerV4-Instance per Config-Hash in Map<string, S3Client> cachen. Cache-Key = SHA256 von {endpoint, region, accessKeyId, secretKeyHash}. Tests: Cache-Hit-Test (zweiter Call mit gleichem Config liefert gecachte Instanz); Cache-Miss-Test (anderer Config → neue Instanz).

  • Files: apps/api/src/storage.ts, apps/api/src/storage.test.ts
  • Commit: 02-observability/perf-quickwins: cache s3 client and signer per config hash

Item 4 — clearSubscriptionsCache Thrash

Datei: apps/web/app/utils/services/.server/subscriptionService.ts:543-548 Problem: clearSubscriptionsCache() invalidiert den gesamten Cache bei jeder Subscription-Änderung, auch wenn nur ein Tenant betroffen ist. Tritt bei Webhook-Bursts massiv auf. Fix: Nur betroffenen Tenant invalidieren via cache.delete(tenantId). Global-Invalidate als separate, expliziter Funktion behalten (für Bulk-Migrations). Tests: Cache mit 3 Tenants befüllen, einen invalidieren → 2 bleiben drin.

  • Files: apps/web/app/utils/services/.server/subscriptionService.ts, Tests
  • Commit: 02-observability/perf-quickwins: invalidate subscription cache per tenant instead of global

Item 5 — CSRF Body-Clone reduzieren

Datei: apps/web/app/utils/session.server.ts:230-268 Problem: CSRF-Token-Check klont und parsed den Request-Body, auch wenn der Token bereits im Header vorhanden ist (häufigster Fall). Body-Parse ist teuer bei Multipart-Uploads. Fix: Header-First-Lookup. Nur wenn Header fehlt: Body-Parse als Fallback (für Forms ohne JS). Tests: Token im Header → Body wird nicht gelesen (Mock-Body-Reader-Spy = 0 calls). Token im Body → Body wird gelesen.

  • Files: apps/web/app/utils/session.server.ts, Tests
  • Commit: 02-observability/perf-quickwins: csrf check reads header first to skip body parse

Item 6 — Recovery-Code Rejection-Sampling

Datei: apps/web/app/utils/auth/mfa/recovery-codes.server.ts:80-83 Problem: While-Loop generiert Codes und prüft auf Collision via DB-Lookup → unnötige Iterations bei sehr unwahrscheinlichen Collisions (Code-Space ist ~10^12). Fix: Direkt generieren ohne Loop. Collision auf DB-Level via UNIQUE-Constraint abfangen (Prisma wirft P2002, dann einmal retry). Spart bei 99.999% der Aufrufe einen DB-Roundtrip. Tests: Standard-Path: 1 Insert. Collision-Path (Mock P2002): 2 Inserts. Mehr-als-1-Collision: throw nach 2 Retries (Rate-Limit-Verdacht).

  • Files: apps/web/app/utils/auth/mfa/recovery-codes.server.ts, Tests
  • Commit: 02-observability/perf-quickwins: drop rejection sampling for recovery codes

Item 7 — UUID-Collision-Check entfernen

Datei: apps/api/src/auth/session/session-store.ts:47-61 Problem: While-Loop prüft, ob ein neu generiertes Session-UUID schon in DB existiert. Bei UUID-v4 ist die Collision-Wahrscheinlichkeit ~10^-37 → Loop ist Aberglaube und macht jeden Session-Create teurer. Fix: UUID generieren, direkt insert. UNIQUE-Constraint fängt theoretische Collision. Tests: Happy-Path: 1 Insert. Collision (Mock): Error wird geworfen (kein Retry — bei UUID-v4-Collision ist was anderes falsch).

  • Files: apps/api/src/auth/session/session-store.ts, Tests
  • Commit: 02-observability/perf-quickwins: drop uuid collision loop in session store

Item 8 — CSRF-Token-Größe 100 → 32 Bytes

Datei: apps/web/app/utils/session.server.ts:212-219 Problem: CSRF-Token wird mit 100 Random-Bytes generiert. 32 Bytes (256 Bit) reichen kryptographisch komplett. Größere Tokens = mehr Bytes pro Request/Response, mehr DOM-Größe. Fix: randomBytes(32) statt randomBytes(100). Tests: Token-Output-Länge = 64 Hex-Chars (32 Bytes Hex).

  • Files: apps/web/app/utils/session.server.ts, Tests
  • Commit: 02-observability/perf-quickwins: reduce csrf token size from 100 to 32 bytes

Item 9 (Wrap-Up) — Baseline-Dokumentation

Datei: docs/observability/performance-baseline.md Inhalt: Tabelle mit Vorher/Nachher-Werten der wichtigsten /metrics-Werte aus den Last-Tests.

  • Files: docs/observability/performance-baseline.md
  • Commit: 02-observability/perf-quickwins: document before/after performance baseline

Test Requirements

  • Unit-Tests: wie pro Item oben gelistet
  • Integration-Tests: keine neuen — die bestehenden Test-Suites pro Service müssen grün bleiben
  • Regression-Tests:
    • Existing auth/session/storage-tests müssen alle grün bleiben (8 Items touchen sicherheitsrelevante Pfade)
    • Existing CSRF-E2E-Tests grün
  • Load-Test (vorher/nachher):
    • oha -n 5000 -c 50 https://app.<staging-domain>/<tenant>/dashboard
    • oha -n 5000 -c 50 https://api.<staging-domain>/v1/<tenant>/something-with-many-queries
  • Coverage-Target: keine Senkung der bestehenden Coverage, neue Tests addieren
  • Test-Files: wie in Implementation Steps pro Item

NICHT als completed markieren bevor:

  • alle Tests grün
  • Last-Test-Vergleich zeigt KEINE Regression in irgendeinem Wert (Optimierungen müssen netto ≥ 0 sein)
  • docs/observability/performance-baseline.md ausgefüllt

Acceptance Criteria

  • Alle 8 Items implementiert, jeder als atomic Commit
  • Existing Tests grün (besonders auth, csrf, session, storage)
  • Neue Tests pro Item, alle grün
  • Last-Test-Vergleich Vorher/Nachher in docs/observability/performance-baseline.md
  • Keine Regression (jeder Metric-Wert nachher ≤ vorher; mind. 3 Items messbar besser)
  • pnpm lint, pnpm typecheck, pnpm test grün
  • Detail-MD Progress Log aktualisiert
  • PROGRESS.md Status auf completed

Rollback Strategy

Pro-Item-Granularität:

  • Item 1 (N+1-Fix) revertierbar via Commit-Revert; im Worst-Case sind wir wieder bei N+1, kein Funktionsbruch
  • Item 2 (Tenant-Fetch) — Risiko: setLoggedUser-Signatur-Änderung kann andere Callsites brechen. Vor Commit: alle Callsites prüfen.
  • Item 3 (S3-Client-Cache) — Risiko: Cache hält veralteten Client wenn Config rotiert. Cache-Invalidate-Hook (z.B. SIGHUP) als Sicherheitsnetz einbauen.
  • Item 4 (per-Tenant-Cache-Invalidate) — Risiko: wenn Subscription-Change Cross-Tenant-Auswirkungen hat (sollte nicht, aber Cross-Check im Service-Code). Bei Zweifel: behalten der Global-Invalidate-Funktion für gewisse Triggers.
  • Item 5 (CSRF-Header-First) — kritisch: muss equivalent zur alten Logik sein. Tests entscheiden.
  • Item 6 (Recovery-Code-Loop entfernen) — Risiko: bei Bug im Unique-Constraint-Handler wird Code-Generation Endlos-Retry. Hard-Limit von 2 Retries einbauen.
  • Item 7 (UUID-Loop entfernen) — quasi risikofrei, UUID-Collision = Hardware-Bug.
  • Item 8 (Token-Größe) — kritisch: bestehende Sessions/Cookies haben noch 100-Byte-Tokens. Migrations-Hinweis: Token-Validierung sollte beide Längen akzeptieren bis alle Sessions abgelaufen sind (max. Session-TTL). Wenn das nicht geht: nur in einem späteren Rollout deployen.

Security Notes

  • Item 5 (CSRF) — keine Verschwächung der CSRF-Logik. Header-First ist standard, Double-Submit-Cookie-Pattern hat den Token immer im Header. Body-Fallback bleibt für No-JS-Forms.
  • Item 6 (Recovery-Codes) — UNIQUE-Constraint ist die richtige Quelle der Wahrheit für Collision-Schutz, nicht ein DB-Pre-Check.
  • Item 7 (UUID-Session) — UUID-v4 hat 122 Bits Entropy. Collision-Probabilistik (Birthday-Paradox) wird erst bei ~10^18 Sessions relevant. Loop ist Cargo-Cult.
  • Item 8 (Token-Größe 32 Bytes) — 256 Bit Entropy ist NIST-empfohlen für CSRF-Tokens. 100 Bytes ist Overkill. Sicherheits-Niveau bleibt identisch.
  • Item 3 (S3-Client-Cache) — Cache-Key MUSS Secret-Hash beinhalten, nicht Secret-Klartext (auch nicht im Hash-Input direkt — sha256(${endpoint}:${region}:${accessKeyId}:${sha256(secret)})).
  • Item 4 (per-Tenant-Cache) — Cache-Key MUSS tenant-scoped sein, sonst Cross-Tenant-Leakage. Test explizit dafür.

Subagent Recommendations

  • Planning: Plan-Subagent mit dieser Detail-MD — soll prüfen, ob Item 8 wirklich rollout-safe ist (Token-Längen-Backwards-Compat) oder ob es ggf. später in eigenen Task muss
  • Research: Explore-Subagent für: "Welche existierenden Sessions/Cookies hätten Probleme mit Token-Größen-Change? Wie ist Session-TTL aktuell?"
  • Implementation: EIN general-purpose-Subagent für alle 8 Items in Sequenz — atomare Commits, Subagent commited nach jedem Item und läuft Tests
  • Tests: kann Implementation-Subagent direkt mitmachen (kleine Test-Files)
  • Load-Test: general-purpose-Subagent mit Brief: "Auf Staging oha-Tests vorher fahren, dann Items 1-8 deployen, dann nachher fahren, Werte tabellieren in docs/observability/performance-baseline.md"
  • Verification: general-purpose-Subagent für Acceptance-Check
  • Code-Review: gsd-code-reviewer nach Task-Completion mit Fokus auf Items 5-8 (Security-relevant)

Staging-Deployment-Steps

  1. Baseline-Messung VOR Deployment:
    • Auf Staging aktuell deployt → oha-Tests fahren, /metrics-Snapshot speichern
    • Werte in docs/observability/performance-baseline.md Spalte "Vorher" eintragen
  2. Branch agent/phase-02-perf-quickwins pushen → Coolify-Deploy
  3. Smoke-Tests:
    • Login funktioniert (Items 5, 8 touchen Session-Pfad)
    • Admin-Users-List rendert (Item 1)
    • File-Upload + Signed-URL funktionieren (Item 3)
    • MFA-Enrollment + Recovery-Codes-Generation (Item 6)
    • Subscription-Update via Stripe-Webhook (Item 4)
  4. Nachher-Messung:
    • Gleiche oha-Tests
    • Werte in "Nachher"-Spalte eintragen
  5. Diff prüfen:
    • Wenn ein Wert schlechter geworden: betroffenen Commit revertieren, neu testen
    • Wenn netto besser: PR → main
  6. Notiz im docs/observability/performance-baseline.md-Footer: Datum, Tester, Last-Test-Hardware (für spätere Reproduzierbarkeit)

Deviations from Plan

(Wird vom ausführenden Agent gefüllt — z.B. wenn Item 8 wegen Backwards-Compat-Issues in einen separaten Task ausgegliedert wird.)

Progress Log

  • 2026-05-17 — Item 1 — N+1 in admin users list resolved via batched getLastUserLogsByUserIds (1 groupBy + 1 findMany).
  • 2026-05-17 — Item 2 — setLoggedUser now returns defaultTenant on the happy path; admin impersonation + GitHub OAuth callsites reuse it (saves one getTenant per sign-in). Path-correction: rootData.ts:30 was a single fetch already; the actual duplicate lived at setLoggedUser callsites in admin/accounts/users.tsx and oauth/github/callback.tsx.
  • 2026-05-17 — Item 3 — S3 storage adapter, signed-URL adapter and SigV4 signing key now cached per SHA256-of-config in apps/api/src/storage.ts. Secret key is fingerprinted, never stored raw. Worker storage skipped (single-use at startup).
  • 2026-05-17 — Item 4 — added clearTenantSubscriptionCache(tenantId) + clearAllSubscriptionsCache(); per-tenant subscription mutations in cancelTenantSubscriptionProduct no longer wipe every tenant's cache. Old clearSubscriptionsCache kept as deprecated alias to avoid breaking plan-CRUD callers in pricingService.
  • 2026-05-17 — Item 5 — already-resolved. getCSRFTokenFromRequest in apps/web/app/utils/session.server.ts:221-258 already reads x-csrf-token/csrf-token headers first and only falls back to body parsing when the header is absent. The header-first path was introduced in 83a57f1c fix: enforce csrf on settings mutations. Existing tests cover header, JSON-body and form-body branches.
  • 2026-05-17 — Item 6 — generateRecoveryCodes replaced rejection-sampling while-loop with a bounded (max 2 attempts) batch-generate-and-verify. Spec referenced lines 80-83 (normalizeRecoveryCode) but the actual rejection loop was in generateRecoveryCodes (lines ~132-134 pre-fix). No DB layer involved — uniqueness is enforced in-memory only.
  • 2026-05-17 — Item 7 — apps/api/src/auth/session/session-store.ts no longer loops on UUID collision in createSession; a duplicate now throws (cargo-cult retry removed). Test suite updated to assert throw instead of overwrite-skip behaviour.
  • 2026-05-17 — Item 8 — generateCSRFToken now emits 32 bytes hex-encoded (64 chars) instead of 100 bytes base64. 256-bit entropy is NIST-recommended; previous token was overkill and bloated every cookie. Backwards-compat: existing sessions retain their longer tokens until natural expiry (session TTL = 30 days) — validateCSRFToken uses timingSafeStringEqual which compares by byte length, so old + new tokens both validate against their own session entry without coercion.
  • 2026-05-17 — Item 9 — wrote docs/observability/performance-baseline.md with per-item code-level expectations + a placeholder "Live baseline" table flagged TBD after #35 ships. Task status flipped to completed.

Lessons Learned

(Optional — z.B. "Item 3 S3-Cache hat in einer Tenant-Bucket-Rotation-Edge-Case ein Stale-Client-Problem gezeigt → Cache-TTL ergänzt".)

Reference in new issue View Git Blame Copy Permalink