Merge pull request #4 from bsiebens/development

Memory saving changes

DEPLOYMENT.md

@@ -35,12 +35,14 @@ Caddy terminates TLS, so without it Django believes every request is plain HTTP:
 
 **4. Uploads must move to object storage before the second app server.**
 Club logos go to `MEDIA_ROOT` on local disk by default. `compose.yaml` mounts a `media_data`
-volume so that survives a rebuild, and `rosterchief/urls.py` serves `/media/*` itself whenever
-`AWS_STORAGE_BUCKET_NAME` is unset — Caddy only reverse-proxies, it never serves media on its
-own, so without that route every logo 404s even on one box. On two boxes local disk stops
-working regardless: a logo uploaded to node A is still a 404 on node B, since nothing shares
-the volume between them. Setting `AWS_STORAGE_BUCKET_NAME` switches the default storage to S3
-— do it *before* you scale, not during.
+volume, shared read-write with `web` and read-only with `caddy`, so uploads both survive a
+rebuild and get served by Caddy directly (`handle_path /media/*` in the Caddyfile) rather than
+round-tripping through a gunicorn worker. `rosterchief/urls.py` still serves `/media/*` itself
+as a fallback whenever `AWS_STORAGE_BUCKET_NAME` is unset — needed for `compose.behind-proxy.yaml`
+(no bundled Caddy there) and for `runserver`. On two boxes local disk stops working regardless
+of any of this: a logo uploaded to node A is still a 404 on node B, since nothing shares the
+volume between them. Setting `AWS_STORAGE_BUCKET_NAME` switches the default storage to S3 — do
+it *before* you scale, not during.
 
 **5. PDF invoices need native libraries.**
 WeasyPrint binds to pango/cairo. The image installs them; a bare-metal deploy would need
@@ -421,7 +423,8 @@ So do not size for the data. Size for the **processes**.
 
 ### What actually consumes the box
 
-Measured, running this app under gunicorn with `DEBUG=False`:
+Measured, running this app under gunicorn with `DEBUG=False`, before the tuning below —
+`--workers 3`, no `--preload`, Postgres and Redis on their image defaults:
 
 | | memory |
 |---|---|
@@ -432,6 +435,13 @@ Measured, running this app under gunicorn with `DEBUG=False`:
 | OS + Docker daemon | ~400 MB |
 | **steady state** | **~1.0–1.2 GB** |
 
+Since then, `Dockerfile`/`compose.yaml` were tuned for smaller boxes: `--workers 2 --preload`
+(one fewer duplicated Django process, and `--preload` shares immutable memory across workers
+via copy-on-write instead of each worker importing Django independently), plus trimmed Postgres
+`shared_buffers`/`max_connections` and a Redis `--maxmemory` cap. Expect the gunicorn and
+Postgres rows to come in lower than above — not yet re-measured, so treat the table as the
+shape of where memory goes rather than exact numbers on the current config.
+
 2 GB would run it. 4 GB is the recommendation for three reasons, all of which are the kind of
 thing that bites at the worst moment:
 

Dockerfile

@@ -77,7 +77,13 @@ RUN DJANGO_SECRET_KEY=build-only-not-a-secret \
     DJANGO_STATICFILES_BACKEND=whitenoise.storage.CompressedManifestStaticFilesStorage \
     python manage.py collectstatic --noinput
 
-RUN useradd --system --uid 1000 rosterchief && chown -R rosterchief /app
+# mkdir before chown, and before the volume ever mounts: media_data has nothing to copy from
+# at /app/media otherwise, so Docker creates the mount point itself, owned by root — and the
+# app runs as rosterchief, not root. Existing image content (even an empty, correctly-owned
+# dir) is what a named volume copies its initial ownership from on first use.
+RUN useradd --system --uid 1000 rosterchief \
+    && mkdir -p /app/media \
+    && chown -R rosterchief /app
 USER rosterchief
 
 EXPOSE 8000
@@ -85,10 +91,19 @@ EXPOSE 8000
 # Migrations are NOT run here. With more than one app container they would race, and a failed
 # migration inside a starting web process is a bad place to find out — deploy runs them once,
 # explicitly (see DEPLOYMENT.md).
+# 2 workers, not 3: DEPLOYMENT.md's own sizing says this workload isn't CPU-bound, and each
+# worker duplicates a full Django process — the single biggest lever on a memory-limited box.
+# --preload imports the app once in the master and forks workers via copy-on-write instead of
+# each re-importing Django independently (safe here: no app's ready() touches DB/Redis eagerly,
+# checked club/features/news/events). --max-requests recycles a worker periodically so the one
+# that happens to render a WeasyPrint invoice doesn't carry that +50-100MB forever.
 CMD ["gunicorn", "rosterchief.wsgi:application", \
      "--bind", "0.0.0.0:8000", \
-     "--workers", "3", \
+     "--workers", "2", \
      "--threads", "4", \
+     "--preload", \
+     "--max-requests", "500", \
+     "--max-requests-jitter", "50", \
      "--timeout", "60", \
      "--access-logfile", "-", \
      "--error-logfile", "-"]

compose.behind-proxy.yaml

@@ -43,6 +43,8 @@ services:
       POSTGRES_DB: ${POSTGRES_DB:-rosterchief}
       POSTGRES_USER: ${POSTGRES_USER:-rosterchief}
       POSTGRES_PASSWORD: ${POSTGRES_PASSWORD:?set a database password}
+    # See compose.yaml for why these are trimmed from the defaults.
+    command: ["postgres", "-c", "shared_buffers=64MB", "-c", "max_connections=20"]
     volumes:
       - pgdata:/var/lib/postgresql/data
     healthcheck:
@@ -54,7 +56,7 @@ services:
   redis:
     image: redis:7-alpine
     restart: unless-stopped
-    command: ["redis-server", "--save", "", "--appendonly", "no"]
+    command: ["redis-server", "--save", "", "--appendonly", "no", "--maxmemory", "32mb", "--maxmemory-policy", "allkeys-lru"]
 
 volumes:
   pgdata:

compose.yaml

@@ -25,6 +25,9 @@ services:
       - ./deploy/caddy/Caddyfile:/etc/caddy/Caddyfile:ro
       - caddy_data:/data
       - caddy_config:/config
+      # Read-only: Caddy serves club logos straight off disk instead of round-tripping every
+      # image request through a gunicorn worker. Same volume `web` writes uploads into.
+      - media_data:/srv/media:ro
     depends_on:
       - web
 
@@ -56,6 +59,11 @@ services:
       POSTGRES_DB: ${POSTGRES_DB:-rosterchief}
       POSTGRES_USER: ${POSTGRES_USER:-rosterchief}
       POSTGRES_PASSWORD: ${POSTGRES_PASSWORD:?set a database password}
+    # shared_buffers/max_connections default to 128MB / 100 — sized for a much bigger database
+    # than this app's (DEPLOYMENT.md: ~0.2GB after 5 years). 20 connections is comfortably above
+    # 2 gunicorn workers x 4 threads plus the odd `manage.py` one-off; trimmed both for the box,
+    # not for the data.
+    command: ["postgres", "-c", "shared_buffers=64MB", "-c", "max_connections=20"]
     volumes:
       - pgdata:/var/lib/postgresql/data
     healthcheck:
@@ -67,9 +75,11 @@ services:
   redis:
     image: redis:7-alpine
     restart: unless-stopped
-    command: ["redis-server", "--save", "", "--appendonly", "no"]
     # Cache only, so nothing here needs to survive a restart. It is not optional though: it
-    # is what keeps every gunicorn worker agreeing about which feature flags are on.
+    # is what keeps every gunicorn worker agreeing about which feature flags are on. maxmemory
+    # is a ceiling, not a saving — this is already the smallest process in the stack — but on a
+    # memory-limited box it should evict cache entries under pressure, not grow unbounded.
+    command: ["redis-server", "--save", "", "--appendonly", "no", "--maxmemory", "32mb", "--maxmemory-policy", "allkeys-lru"]
 
 volumes:
   pgdata:

deploy/caddy/Caddyfile

@@ -10,6 +10,15 @@
 
 	encode zstd gzip
 
+	# Club logos, served straight off the shared volume — no gunicorn worker involved. Only
+	# matters while storage is local disk; once AWS_STORAGE_BUCKET_NAME is set, club.logo.url
+	# points at the bucket directly and this block simply never matches. A missing file 404s
+	# here exactly as django.views.static.serve would, so there is no need to fall through.
+	handle_path /media/* {
+		root * /srv/media
+		file_server
+	}
+
 	# X-Forwarded-Proto is what SECURE_PROXY_SSL_HEADER reads. Without it Django believes every
 	# request is plain HTTP: request.is_secure() goes false, WebAuthn disagrees with the browser
 	# about the origin, and the SSL redirect becomes a loop.

rosterchief/urls.py

@@ -7,11 +7,13 @@ second factors. ``RequireMFAMiddleware`` then blocks any staff user who has not
 enrolled.
 """
 
+import re
+
 from django.conf import settings
-from django.conf.urls.static import static
 from django.contrib import admin
-from django.urls import include, path
+from django.urls import include, path, re_path
 from django.views.generic import RedirectView
+from django.views.static import serve
 
 from api.urls import api
 from club.views import root
@@ -45,4 +47,11 @@ if not settings.AWS_STORAGE_BUCKET_NAME:
     # /media/* itself — so without this route every uploaded club logo 404s in production too.
     # Once AWS_STORAGE_BUCKET_NAME is set, club.logo.url points straight at the bucket and this
     # route is simply never hit.
-    urlpatterns += static(settings.MEDIA_URL, document_root=settings.MEDIA_ROOT)
+    #
+    # django.conf.urls.static.static() looks like the right helper, but it hard-codes its own
+    # `if not settings.DEBUG: return []` — it is documented as dev-only and silently no-ops in
+    # production no matter what guards the call site. Build the pattern directly against the
+    # view it wraps instead, which has no such gate.
+    urlpatterns += [
+        re_path(rf"^{re.escape(settings.MEDIA_URL.lstrip('/'))}(?P<path>.*)$", serve, {"document_root": settings.MEDIA_ROOT}),
+    ]