docs: document N+1 performance bug findings and prevention rules
This commit is contained in:
@@ -4,6 +4,61 @@
|
||||
|
||||
---
|
||||
|
||||
## Session — 2026-02-27 late night #2 (GitHub Copilot - Claude Opus 4.6)
|
||||
|
||||
### Context
|
||||
|
||||
Performance investigation: Registratura loading extremely slowly with only 6 entries. Rack numbering inverted.
|
||||
|
||||
### Root Cause Analysis — Findings
|
||||
|
||||
**CRITICAL BUG: N+1 Query Pattern in ALL Storage Hooks**
|
||||
|
||||
The `DatabaseStorageAdapter.list()` method fetches ALL items (keys + values) from PostgreSQL in one HTTP request, but **discards the values** and returns only the key names. Then every hook calls `storage.get(key)` for EACH key individually — making a separate HTTP request + DB query per item.
|
||||
|
||||
With 6 registry entries + ~10 contacts + ~20 tags, the Registratura page fired **~40 sequential HTTP requests** on load (**1 list + N gets per hook × 3 hooks**). Each request goes through: browser → Next.js API route → Prisma → PostgreSQL → back. This is a textbook N+1 query problem.
|
||||
|
||||
**Additional issues found:**
|
||||
|
||||
- `addEntry()` called `getAllEntries()` for number generation, then `refresh()` called `getAllEntries()` again → double-fetch
|
||||
- `closeEntry()` called `updateEntry()` (which refreshes), then manually called `refresh()` again → double-refresh
|
||||
- Every single module hook had the same pattern (11 hooks total)
|
||||
- Rack visualization rendered U1 at top instead of bottom
|
||||
|
||||
### Fix Applied
|
||||
|
||||
**Strategy:** Replace `list()` + N × `get()` with a single `exportAll()` call that fetches all items in one HTTP request and filters client-side.
|
||||
|
||||
**Files fixed (13 total):**
|
||||
|
||||
- `src/modules/registratura/services/registry-service.ts` — added `exportAll` to `RegistryStorage` interface, rewrote `getAllEntries`
|
||||
- `src/modules/registratura/hooks/use-registry.ts` — `addEntry` uses optimistic local state update instead of double-refresh; `closeEntry` batches saves with `Promise.all` + single refresh
|
||||
- `src/modules/address-book/hooks/use-contacts.ts` — `exportAll` batch load
|
||||
- `src/modules/it-inventory/hooks/use-inventory.ts` — `exportAll` batch load
|
||||
- `src/modules/password-vault/hooks/use-vault.ts` — `exportAll` batch load
|
||||
- `src/modules/word-templates/hooks/use-templates.ts` — `exportAll` batch load
|
||||
- `src/modules/prompt-generator/hooks/use-prompt-generator.ts` — `exportAll` batch load
|
||||
- `src/modules/hot-desk/hooks/use-reservations.ts` — `exportAll` batch load
|
||||
- `src/modules/email-signature/hooks/use-saved-signatures.ts` — `exportAll` batch load
|
||||
- `src/modules/digital-signatures/hooks/use-signatures.ts` — `exportAll` batch load
|
||||
- `src/modules/ai-chat/hooks/use-chat.ts` — `exportAll` batch load
|
||||
- `src/core/tagging/tag-service.ts` — uses `storage.export()` instead of N+1
|
||||
- `src/modules/it-inventory/components/server-rack.tsx` — reversed slot rendering (U1 at bottom)
|
||||
|
||||
**Performance impact:** ~90% reduction in HTTP requests on page load. Registratura: from ~40 requests to 3 (one per namespace: registratura, address-book, tags).
|
||||
|
||||
### Prevention Rules (for future development)
|
||||
|
||||
> **NEVER** use the `storage.list()` + loop `storage.get()` pattern.
|
||||
> Always use `storage.exportAll()` to load all items in a namespace, then filter client-side.
|
||||
> This is the #1 performance pitfall in the storage layer.
|
||||
|
||||
### Commits
|
||||
|
||||
- `c45a30e` perf: fix N+1 query pattern across all modules + rack numbering
|
||||
|
||||
---
|
||||
|
||||
## Session — 2026-02-27 late night (GitHub Copilot - Claude Opus 4.6)
|
||||
|
||||
### Context
|
||||
|
||||
Reference in New Issue
Block a user