diff --git a/features/INDEX.md b/features/INDEX.md index 897767e..fd654c8 100644 --- a/features/INDEX.md +++ b/features/INDEX.md @@ -90,7 +90,7 @@ | PROJ-72 | Fix Superadmin kann Passwort/Rolle von Superadmin-Peers nicht ändern (Sicherheitsbug) | Deployed | [PROJ-72](PROJ-72-fix-superadmin-peer-patch.md) | 2026-07-27 | | PROJ-73 | Restliche Crash-Härtung (Upload-Job-Status bei Panic, fehlende nil-Checks) | In Review | [PROJ-73](PROJ-73-restliche-crash-haertung.md) | 2026-08-05 | | PROJ-74 | Vorbestehende Test-/Vet-Signatur-Drift beheben (go vet/test wieder komplett grün) | In Review | [PROJ-74](PROJ-74-test-suite-signatur-drift.md) | 2026-08-05 | -| PROJ-75 | DB-Performance-Audit (Query-Index-Nutzung, Fan-out, pgxpool-Tuning) | Planned | [PROJ-75](PROJ-75-db-performance-audit.md) | 2026-08-05 | +| PROJ-75 | DB-Performance-Audit (Query-Index-Nutzung, Fan-out, pgxpool-Tuning) | In Review | [PROJ-75](PROJ-75-db-performance-audit.md) | 2026-08-05 | | PROJ-76 | Mail-HTML-Sanitizing schließt CSS-url()/link/srcset nicht ein (Tracking-Pixel-Umgehung) | Planned | [PROJ-76](PROJ-76-mail-html-sanitizing-luecken.md) | 2026-08-05 | | PROJ-77 | Admin-Tab-Bundle-Optimierung (dynamic import statt 19 statische Imports) | Planned | [PROJ-77](PROJ-77-admin-tabs-dynamic-import.md) | 2026-08-05 | | PROJ-78 | SearchResultsTable re-rendert bei jedem Tastenanschlag im Suchfeld | Planned | [PROJ-78](PROJ-78-search-results-rerender-perf.md) | 2026-08-05 | diff --git a/features/PROJ-74-test-suite-signatur-drift.md b/features/PROJ-74-test-suite-signatur-drift.md index b420a1f..5f88639 100644 --- a/features/PROJ-74-test-suite-signatur-drift.md +++ b/features/PROJ-74-test-suite-signatur-drift.md @@ -145,6 +145,8 @@ Zwei zusätzliche Fixes waren nötig, über die ursprüngliche Analyse hinaus: Auth-Contract-Drift, unabhängig von PROJ-73/74. Test liest den Token jetzt aus `w.Result().Cookies()`. +Deployed auf 132 am 2026-08-05. + ## Acceptance Criteria - [x] `CGO_ENABLED=0 go build ./...` auf 132 fehlerfrei (alle Pakete diff --git a/features/PROJ-75-db-performance-audit.md b/features/PROJ-75-db-performance-audit.md new file mode 100644 index 0000000..1b99db1 --- /dev/null +++ b/features/PROJ-75-db-performance-audit.md @@ -0,0 +1,211 @@ +--- +id: PROJ-75 +title: DB-Performance-Audit (Query-Index-Nutzung, Fan-out, pgxpool-Tuning) +status: In Review +created: 2026-08-05 +--- + +## Problem + +Der Performance-Check der Crash-Härtung-Session (PROJ-73-Vorarbeiten) wurde +aus Zeitgründen nicht systematisch für die Datenbankschicht abgeschlossen. +Beim Überfliegen auffällig: + +- `internal/storage/storage.go:1094` und `internal/storage/mark_deletion.go:101` + — dynamisch zusammengebaute SQL-Queries, Index-Nutzung nicht verifiziert. +- `internal/api/search_handlers.go:209` und `:491` — Parsing/Verarbeitung + pro Treffer innerhalb der Ergebnis-Schleife, potenzieller Fan-out bei + großen Trefferzahlen. +- `internal/reconciliation/` — noch nicht geprüft. +- Kein explizites `pgxpool.Config`-Tuning (`MaxConns`, `MinConns`, + `MaxConnLifetime`) — läuft aktuell auf pgx-Defaults, unklar ob für die + Archivgröße auf 131 (Produktiv) und 132 (teilproduktiv, echte Daten) + passend. + +Kein akuter Crash-Bezug, aber relevant für Skalierung bei wachsendem +Archiv-Volumen. + +## Lösung (Vorschlag) + +1. `EXPLAIN ANALYZE` für die Queries in `storage.go:1094` und + `mark_deletion.go:101` auf 132 gegen realistische Datenmenge laufen + lassen, fehlende Indizes identifizieren. +2. `search_handlers.go` Fan-out-Pfad profilen (z.B. mit `pprof` bei + großer Trefferzahl) — prüfen ob Batch-Verarbeitung statt Pro-Treffer- + Parsing möglich ist. +3. `internal/reconciliation/` auf N+1-Muster durchsehen. +4. `pgxpool.Config` explizit setzen basierend auf Serverkapazität (131/132) + und beobachteter Connection-Auslastung. + +## Implementation Notes + +Audit durchgeführt am 2026-08-05, rein lesend gegen 192.168.1.132 (teilproduktiv, +echte Daten). Keine Schema-Änderungen vorgenommen — Vorschläge unten stehen zur +Rücksprache. + +**Servereckdaten 132:** 5 vCPU, 4 GiB RAM, `max_connections = 100`, aktuell 12 +aktive Verbindungen auf `archivmail`. Tabellengrößen (n_live_tup): +`storage_objects` 84139, `emails` 52895, `email_refs` 51209, +`email_attachments` 14712, `attachments` 9102. + +### 1. Query-Pläne (storage.go:1094, mark_deletion.go:101) + +- `GetUnindexedMails` (storage.go, `SELECT id, tenant_id FROM emails WHERE + indexed_at IS NULL ORDER BY received_at DESC LIMIT $1`): + **Seq Scan bestätigt.** `EXPLAIN ANALYZE` zeigt vollen Scan über alle 52895 + Zeilen (`Rows Removed by Filter: 52895`), aktuell 13.3ms weil alles bereits + indexiert ist (0 Treffer) — d.h. der Scan berührt bei jedem Cron-Lauf + (`index-pending`, PROJ-58) immer die komplette Tabelle, unabhängig davon wie + wenige unindizierte Mails es gibt. Auf `emails` existiert kein Index auf + `indexed_at`. Mit wachsendem Archiv (aktuell 53k, Zielgröße deutlich größer) + skaliert das linear mit — **Index-Vorschlag** (partial index, da fast immer + `indexed_at IS NOT NULL`): + ```sql + CREATE INDEX IF NOT EXISTS idx_emails_unindexed + ON emails (received_at DESC) WHERE indexed_at IS NULL; + ``` + Partial Index hält den Index winzig (nur die tatsächlich unindizierten + Zeilen), passt exakt zur WHERE- und ORDER-BY-Klausel der Query. + +### Umsetzung Index (2026-08-05) + +Index in `initSchema` ergänzt (`internal/storage/storage.go`, direkt nach +`idx_emails_retain_until`, idempotent via `CREATE INDEX IF NOT EXISTS`): +```sql +CREATE INDEX IF NOT EXISTS idx_emails_unindexed + ON emails (received_at DESC) WHERE indexed_at IS NULL; +``` +Ausgeführt direkt via `psql` auf 192.168.1.132 (nicht auf 131 — nur Testserver +laut Auftrag). Kein Backend-Restart nötig, `CREATE INDEX IF NOT EXISTS` ohne +`CONCURRENTLY` war bei 52.895 Zeilen unkritisch und ohne spürbare Downtime. + +**Validierung — `EXPLAIN ANALYZE` gegen die Query aus `GetUnindexedMails`:** + +Vorher (Seq Scan, aus dem ursprünglichen Audit oben): `Rows Removed by Filter: +52895`, 13.3ms. + +Nachher: +``` +Limit (cost=0.12..12.10 rows=2 width=81) (actual time=0.069..0.070 rows=1 loops=1) + -> Index Scan using idx_emails_unindexed on emails (cost=0.12..12.10 rows=2 width=81) (actual time=0.068..0.069 rows=1 loops=1) +Planning Time: 1.012 ms +Execution Time: 0.086 ms +``` +Seq Scan → Index Scan bestätigt, Ausführungszeit von 13.3ms auf 0.086ms +gesunken (~150x). Migration erfolgreich, nur auf 132 angewendet — 131 bewusst +nicht angefasst. + +- `ListExpiredMails` (mark_deletion.go, `WHERE retain_until IS NOT NULL AND + retain_until < NOW() [AND tenant_id = $N] ORDER BY retain_until ASC LIMIT + 500`): **Index Scan bestätigt, keine Änderung nötig.** Nutzt bereits + `idx_emails_retain_until` (partial index `WHERE retain_until IS NOT NULL`), + 0.4ms Ausführungszeit. Der optionale `tenant_id`-Filter wird nicht separat + geprüft, ist aber bei der geringen Selektivität (max. 500 Zeilen via LIMIT) + unkritisch — `idx_emails_tenant` existiert als Fallback. + +### 2. Fan-out in search_handlers.go (:196–235, :460–508) + +**Kein N+1 gegen PostgreSQL.** Der Treffer-Loop ab Zeile 205 macht pro Treffer: +- `s.store.Load(h.ID)` — Dateisystem-Read (verschlüsselter Store), keine DB-Query. +- `mailparser.Parse(raw)` — reines In-Memory-Parsing. + +`GetThreadInfo(ctx, hitIDs)` und `GetReceivedAts(ctx, hitIDs)` werden **einmal +vor der Schleife** mit der vollen `hitIDs`-Liste aufgerufen und intern per +`WHERE id = ANY($1)` batched (storage.go:600ff, :796) — kein Pro-Treffer-Query. +Bei `page_size=100` sind das weiterhin 2 DB-Queries gesamt, nicht 100+2. Der +eigentliche Kostenfaktor bei großer Trefferzahl ist der Datei-I/O +(`store.Load` pro Treffer) und CPU-gebundenes Parsing, nicht die Datenbank — +das ist außerhalb des DB-Audit-Scopes (ggf. separates Ticket für Store-I/O- +Profiling, falls das in der Praxis auffällt). Für PROJ-75 (DB-Fokus): **kein +Handlungsbedarf.** + +### 3. internal/reconciliation/ — N+1 gefunden + +`DashboardData` (reconciliation.go:74–113) iteriert über alle Source-Buckets +(`agg`, typischerweise wenige Quellen: SMTP, IMAP-Accounts, POP3-Accounts, +Import — Größenordnung einstellig bis niedrig zweistellig pro Tenant) und ruft +pro Bucket `s.trailingAverage(ctx, ...)` auf (Zeile 102), was **eine eigene +DB-Query pro Source** ist (compute.go:287–320, `SELECT ... LIMIT 7`). Gleiches +Muster in `ComputeForDate` (compute.go:116–139) — ein `trailingAverage`-Call +pro Report-Zeile. + +Das ist klassisches N+1, aber bei der aktuellen Bucket-Kardinalität (Anzahl +Quellen pro Tenant, nicht Anzahl Mails) unkritisch — 504 Zeilen in +`reconciliation_reports` insgesamt, die Query selbst ist ein günstiger +Index-Scan mit `LIMIT 7` auf `idx_recon_reports_lookup`. Bei sehr vielen +Tenants mit vielen IMAP/POP3-Accounts könnte das spürbar werden (Dashboard- +Aufruf mit z.B. 200 Quellen = 200 Queries), aktuell aber kein akuter Engpass. +**Empfehlung: nicht jetzt umsetzen**, aber im Auge behalten falls +Multi-Tenant-Wachstum die Quellenzahl deutlich erhöht. Bei Bedarf ließe sich +`trailingAverage` zu einer gebatchten Query (ein `SELECT` mit +`ROW_NUMBER() OVER (PARTITION BY source ...)` oder ein Window-Function-Ansatz) +umbauen. + +### 4. pgxpool-Setup + +**Kein zentraler Pool — jeder Store öffnet seinen eigenen `pgxpool.New(dsn)` +ohne explizites `pgxpool.Config`:** `internal/storage/storage.go:86`, +`internal/tenantstore/store.go:93`, `internal/tokenstore/store.go` (über +übergebenen Pool, s.u.), `internal/smtpoutconfig/store.go:62`, +`internal/ldapconfig/store.go:73` + `tenant_store.go:54`, +`internal/pop3/store.go:71`, `internal/imap/store.go:105`, +`internal/audit/audit.go:105`, `internal/userstore/userstore.go:73`, +`internal/reconciliation/reconciliation.go:47`, plus +`cmd/archivmail/main.go:372` (separater `tokenPool`) und zwei weitere in +`cmd_status.go`/`cmd_migrate_tenants.go` für CLI-Subkommandos. Das sind **~10 +unabhängige Pools** im laufenden Backend-Prozess, jeder mit pgx-Default +(`MaxConns = max(4, runtime.NumCPU())` → auf 132 mit 5 vCPU: 5 Conns/Pool, +theoretisches Maximum ~50 gleichzeitige Verbindungen aus einem einzigen +archivmail-Prozess). Aktuell unkritisch (`max_connections=100`, 12 aktive +Verbindungen beobachtet), aber: +- Kein `MinConns` gesetzt → Pools bauen Connections erst bei Bedarf auf statt + vorzuhalten, kostet Cold-Start-Latenz auf selten genutzten Stores (z.B. + `ldapconfig`, `smtpoutconfig`). +- Kein `MaxConnLifetime`/`MaxConnIdleTime` → Connections werden nie + proaktiv recycelt (relevant falls PgBouncer/Connection-Pooler später + vorgeschaltet wird oder Postgres restart-bedingt Verbindungen killt). +- 10 separate Pools statt einem gemeinsamen ist strukturell unnötig (jeder + Pool reserviert eigene Health-Check-Goroutinen, eigenes Idle-Handling) — + architektonisch sauberer wäre ein gemeinsamer `*pgxpool.Pool`, der an alle + Stores durchgereicht wird (Ansatz existiert bereits teilweise: + `tokenstore.New(pool)` nimmt einen fertigen Pool entgegen). Das ist aber ein + größerer Refactor, kein reines Tuning — separat einplanen, nicht Teil dieses + Audits. + +**Empfohlene Werte** (pro Pool, bis der Pool-Zusammenlegung-Refactor +ansteht) für 132 (5 vCPU/4 GiB) und 131 (Produktiv, vermutlich vergleichbare +oder größere Kapazität — vor Umsetzung auf 131 `nproc`/`free -h` gegenchecken): +```go +cfg, _ := pgxpool.ParseConfig(dsn) +cfg.MaxConns = 8 // statt Default 5 auf 132 — Headroom für Backend + Cron-Jobs +cfg.MinConns = 2 // 2 warme Connections je Store statt Cold-Start +cfg.MaxConnLifetime = 30 * time.Minute +cfg.MaxConnIdleTime = 5 * time.Minute +pool, err := pgxpool.NewWithConfig(ctx, cfg) +``` +Summe über alle ~10 Pools bei MaxConns=8: bis zu 80 Verbindungen möglich — +noch unter `max_connections=100`, aber knapper. **Vor Umsetzung**: entweder +`max_connections` in `postgresql.conf` moderat erhöhen (z.B. 150) oder die +MaxConns pro Pool niedriger ansetzen (z.B. 6) — Vorschlag zur Rücksprache, +keine Änderung vorgenommen. + +## Acceptance Criteria + +- [x] Query-Pläne für die genannten Storage-Queries dokumentiert, fehlende + Indizes identifiziert oder Fehlanzeige begründet. → `GetUnindexedMails` + Seq-Scan bestätigt, Index umgesetzt und auf 132 validiert (Seq Scan → + Index Scan, 13.3ms → 0.086ms). `ListExpiredMails` nutzt bereits einen + passenden Index, kein Handlungsbedarf. +- [x] Fan-out-Verhalten in `search_handlers.go` bei großen Trefferzahlen + gemessen, Optimierung umgesetzt falls Engpass bestätigt. → Kein + DB-seitiger N+1 (Batch-Queries bereits vorhanden), Engpass falls + vorhanden liegt bei Datei-I/O/Parsing, nicht DB — kein DB-seitiger + Optimierungsbedarf, daher keine Umsetzung nötig. +- [ ] `pgxpool.Config`-Werte explizit gesetzt und begründet (nicht mehr + stillschweigend Default). → Empfehlung dokumentiert (siehe oben), + **Umsetzung steht noch aus** (Folge-Schritt, nicht Teil dieses Audits). + +Zusätzlicher Befund außerhalb der ursprünglichen Lösung-Skizze: +`internal/reconciliation/DashboardData`/`ComputeForDate` haben ein N+1-Muster +(`trailingAverage` pro Source-Bucket), aktuell bei geringer Quellenzahl +unkritisch — dokumentiert, keine Umsetzung empfohlen. diff --git a/internal/storage/storage.go b/internal/storage/storage.go index 3ac6fbb..2cdfe59 100644 --- a/internal/storage/storage.go +++ b/internal/storage/storage.go @@ -100,6 +100,9 @@ func New(cfg Config) (*Store, error) { // PROJ-34: GoBD retention lock _, _ = s.db.Exec(ctx, `ALTER TABLE emails ADD COLUMN IF NOT EXISTS retain_until TIMESTAMPTZ`) _, _ = s.db.Exec(ctx, `CREATE INDEX IF NOT EXISTS idx_emails_retain_until ON emails (retain_until) WHERE retain_until IS NOT NULL`) + // PROJ-75: partial index for GetUnindexedMails (index-pending cron, PROJ-58) — avoids + // full seq scan over emails on every run once indexed_at is set for most rows. + _, _ = s.db.Exec(ctx, `CREATE INDEX IF NOT EXISTS idx_emails_unindexed ON emails (received_at DESC) WHERE indexed_at IS NULL`) // PROJ-33: Stable IMAP UIDs _, _ = s.db.Exec(ctx, `ALTER TABLE emails ADD COLUMN IF NOT EXISTS uid BIGSERIAL`) _, _ = s.db.Exec(ctx, `CREATE UNIQUE INDEX IF NOT EXISTS idx_emails_uid ON emails (uid)`)