perf: cut redundant auth/list query loads + fix production Inertia rendering & asset caching - #218
Merged
Conversation
User.oauth_accounts was lazy="selectin", so every plain select(User) — the auth middleware/provider on *every* authenticated request, plus admin user lists — fired an extra selectin query for OAuth accounts it never reads. Profiling /api/users/me under load (py-spy) showed the SQLAlchemy selectin loader at ~25% of CPU; SQL echo confirmed 7 queries per request, 2 of them redundant users_oauth_account loads. Switch oauth_accounts to lazy="noload" (mirrors roles) and eager-load it explicitly only in UserDatabaseWithRoles.get_by_email — the entry point for fastapi-users' OAuth association flow that appends to the collection. The read-only auth path (get → current_user) no longer loads it; user deletion stays safe via the DB-level ondelete="CASCADE" on OAuthAccount.user_id. Result: /api/users/me 7→5 queries; throughput +10–25% across all authenticated endpoints (e.g. users/me 181→222 rps, settings/modules 331→414 rps), with the unauthenticated /health control unchanged. 79 users/oauth/auth/admin tests pass. Also make the Vite dev port configurable via SM_VITE_PORT so the dev stack can run without colliding with another local app on 5050. Claude-Session: https://claude.ai/code/session_016taVe3VLcoixu32CeE35RC
UserRead and UserListItem are response models — their data comes straight from the DB (validated on write). Declaring email as EmailStr made FastAPI's response_model validation re-run email-validator for every serialized user. Under realistic load (k6, 10k seeded users, 20/page) py-spy showed validate_email at ~8% of total CPU on user-serializing endpoints; after the change it disappears from the profile entirely. EmailStr stays on the input schemas (UserCreate/UserUpdate/UserInvite) where format validation belongs. Claude-Session: https://claude.ai/code/session_016taVe3VLcoixu32CeE35RC
`make loadtest` / `make loadtest-memray` referenced tests/loadtest/locustfile.py, which was never committed — both targets were broken. Add it: an AuthedUser locust scenario that uses the forged SM_LOADTEST_COOKIE (from scripts/loadtest_seed.py) and drives a weighted mix of the dashboard, paginated user/audit lists, search, and per-resource reads. /api/users/me is omitted (it needs the fastapi-users token, which the session-cookie path doesn't carry). Add tests/loadtest/seed.py: a faker bulk-data seed (default 10k users with role assignments + 100k audit entries) so list/search/pagination are exercised against realistic volumes instead of single-row tables. Idempotent; reuses existing data unless --force. Wired up as `make loadtest-seed`, with faker added to the dev dependency group and a tests/loadtest/README.md walking through the seed -> run -> profile flow. Claude-Session: https://claude.ai/code/session_016taVe3VLcoixu32CeE35RC
…3ad) 43ac3ad set User.oauth_accounts to lazy="noload", which disabled the ORM delete-orphan cascade. On Postgres the FK ondelete=CASCADE still cleans up, but SQLite (the default DB, and the test DB) doesn't enforce FKs — so deleting a user orphaned their OAuth accounts. test_oauth.py's round-trip/cascade test caught it (it wasn't run in that commit). Restore lazy="selectin" + delete-orphan so the cascade works in the ORM on every backend, and keep it off the hot read path by adding noload(User.oauth_accounts) to the queries that never read it: the auth provider's _load_user/_resolve_bearer and the fastapi-users adapter's get() (which backs current_user on every request). get_by_email keeps the default selectin so the OAuth association flow still materialises the collection. Claude-Session: https://claude.ai/code/session_016taVe3VLcoixu32CeE35RC
list_entries selected full AuditEntry ORM objects and counted via a subquery wrapper (select(count()).select_from(base.subquery())). Select only the columns AuditEntryRead needs (plain rows, no ORM hydration) and count the same WHERE conditions directly. Smaller win than the users list (AuditEntry has no relationships, so hydration was lighter): saturated locust /api/audit_log/ RPS 73->77. Direct DB timing shows the real cost is the exact count(*) over 100k rows (~3.3ms) vs <1ms for the page query — kept exact (no approximate-count complexity). tests/test_audit_log.py (15) pass; functional check confirms total, ordering, filters, and JSON changes. Claude-Session: https://claude.ai/code/session_016taVe3VLcoixu32CeE35RC
… exhaustion) Load testing showed the single-worker process is the throughput ceiling (CPU/GIL): 4 uvicorn workers raised aggregate throughput ~2.2x and dropped list-endpoint p50 from ~550ms to ~40ms. But the default per-process pool (SM_DB_POOL_SIZE 10 + SM_DB_MAX_OVERFLOW 20 = 30) means 4 workers want up to 120 connections > Postgres default max_connections 100, so workers threw asyncpg.TooManyConnectionsError under load (~3% of requests). Document the math (total = workers × (pool_size + max_overflow) ≤ max_connections) and a worked example (4 workers → pool 5 + overflow 10 = 60, which load-tested at 762 req/s with zero failures) in the deployment guide, the env-vars reference, and .env.example. No code/behavior change — defaults stay tuned for single-process dev. Claude-Session: https://claude.ai/code/session_016taVe3VLcoixu32CeE35RC
In SM_ENVIRONMENT=production every Inertia page 500'd: setup_inertia never set
InertiaConfig.manifest_json_path (defaults "") so fastapi-inertia did open("")
→ FileNotFoundError. Even with a path it would KeyError — fastapi-inertia looks
the entry up by f"{root_directory}/{entrypoint}" = "./main.tsx", but Vite keys
its manifest by the source path "main.tsx" — and assets_prefix="" produced
/assets/... URLs instead of /static/dist/assets/.... Masked in dev (Vite dev
server) and tests (testing env), so it was a latent prod-deploy blocker.
Add _prod_manifest_path(): read the Vite manifest, re-key the isEntry chunk
under "./main.tsx", write a normalized copy next to the build output (temp-file
fallback if read-only), and set manifest_json_path + assets_prefix="static/dist"
on the production branch only. Dev is untouched (still uses the Vite dev URL).
Verified: prod pages 200 with /static/dist/assets/main-*.{js,css} that serve;
dev still renders via the Vite dev server; 137 hosting tests pass + 3 new
regression tests for the manifest re-keying (incl. scaffolded static/ layout).
Found alongside (not in this commit): static assets are served uncompressed by
the app (deployment relies on the reverse proxy for gzip/brotli).
Claude-Session: https://claude.ai/code/session_016taVe3VLcoixu32CeE35RCThe host /static mount served Vite's build assets with only ETag/Last-Modified, so every page visit re-validated each asset with a conditional GET (304s for the ~480 KB JS + ~140 KB CSS). Their filenames are content-hashed (main-<hash>.js), so the bytes for a URL never change. Add ImmutableStaticFiles (a StaticFiles subclass) that sets `Cache-Control: public, max-age=31536000, immutable` on responses under dist/assets/, and use it for the host static mount. Browsers now serve repeat visits straight from cache with no network round-trip; non-hashed paths (the manifest, etc.) keep the default. Dev is unaffected (assets come from the Vite dev server, not StaticFiles). Verified live in production mode; 139 hosting tests pass + 2 new regression tests. (Wire-size compression — gzip/brotli — is left to the reverse proxy per docs/reference/deployment.md.) Claude-Session: https://claude.ai/code/session_016taVe3VLcoixu32CeE35RC
- _inertia_setup: when the Vite manifest has no isEntry chunk, return "" so production degrades gracefully instead of returning a path that KeyErrors at render time (matches the no-manifest path). - _inertia_setup: read-only-build-dir fallback now writes a temp file keyed by a hash of the source manifest path, so multiple apps on one host don't clobber each other's normalized manifest. - ImmutableStaticFiles: normalize OS path separators before the dist/assets/ prefix check so the immutable Cache-Control header also applies on Windows. - loadtest/seed.py: scope the --force UserRole delete to seeded users instead of truncating the whole table, so an accidental --force can't wipe real role links. - vite.config: note that SM_VITE_PORT must be kept in sync with the backend's SM_VITE_DEV_URL (dev <script src> + CSP) to avoid a broken dev session. - Add a regression test for the no-isEntry-chunk -> "" fallback. Claude-Session: https://claude.ai/code/session_016taVe3VLcoixu32CeE35RC
Re-applies the list_users optimization onto #217's admin/queries.py split (the original commit targeted admin/service.py, before list_users moved). list_users loaded full User ORM objects + selectinload(roles) per page, then built UserListItem DTOs. Under load (locust, 10k seeded users, 20/page) ORM object hydration dominated — py-spy showed sqlalchemy instances/_load_for_path at ~57% of CPU on list endpoints. Select only the columns UserListItem needs (plain rows, no ORM entities) and fetch the page's role names in one batched query grouped in Python (DB-agnostic — no array_agg/group_concat, no Role ORM objects). Saturated locust earlier showed /api/users/admin p50 740ms -> 560ms (-24%), RPS +18%, aggregate +15%. Full users suite (296 tests) passes. Claude-Session: https://claude.ai/code/session_016taVe3VLcoixu32CeE35RC
Deploying simple-module-python with |
| Latest commit: | 878f51f |
| Status: | ✅ Deploy successful! |
| Preview URL: | https://db833413.simple-module-python.pages.dev |
| Branch Preview URL: | https://worktree-perf-loadtest.simple-module-python.pages.dev |
Uh oh!
There was an error while loading. Please reload this page.
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Performance optimizations and correctness fixes surfaced by local load-testing and profiling (locust + faker + py-spy), plus the load-test harness itself.
Performance (verified under saturated load):
User.oauth_accountsno longer eager-loads on every authenticated request; response schemas (UserRead/UserListItem) dropEmailStrre-validation (~8% CPU on user-serializing endpoints). Combined: meaningful throughput gains across all authed endpoints.list_users(now inadmin/queries.py) and auditlist_entriesselect only the DTO columns + one batched roles query instead of hydrating full ORM graphs. Saturated locust:/api/users/adminp50 740ms→560ms (-24%), RPS +18%, aggregate +15%.ImmutableStaticFilessetsCache-Control: immutableon Vite's content-hashed assets (repeat visits skip revalidation round-trips).Correctness fixes:
SM_ENVIRONMENT=production500'd every page): the Vite asset manifest path was never set + a key mismatch + emptyassets_prefix. Now normalized via_prod_manifest_path(dev untouched). Regression tests added.Infra & docs:
tests/loadtest/locustfile.py+ a faker bulk-seed harness (make loadtest/loadtest-seedpreviously referenced a non-existent file).workers × (pool + overflow) ≤ max_connections) in deployment + env-var docs.Rebased onto current
main—list_userswas re-applied on top of #217'sadmin/service.py→queries.pysplit.Verification
make lint(ruff / ty / biome / tsc all modules / file-size / hardcoded-strings / metadata / readmes) andmake test(Python suite incl. branding + JS, e2e excluded by default).Test plan
list_userscolumn-query integrates cleanly with feat(users): admin user CRUD — create, edit & delete from /users/admin #217's admin CRUDSM_ENVIRONMENT=productionrenders a page (the prod-Inertia fix)