diff --git a/Dockerfile b/Dockerfile index 7fd22a02..7d7c4242 100644 --- a/Dockerfile +++ b/Dockerfile @@ -109,8 +109,13 @@ RUN apt-get update \ && rm -rf /var/lib/apt/lists/* COPY requirements.txt ./ +# gunicorn and gevent are container-only: requirements.txt is a universal, +# hash-checked lockfile that also serves the Windows/IIS install, where waitress +# is the server and neither of these is used. gevent is the worker class (see +# the CMD below) because the endpoint long-poll holds a connection per agent, +# which a thread pool cannot absorb at fleet scale. RUN pip install --no-cache-dir -r requirements.txt \ - && pip install --no-cache-dir gunicorn + && pip install --no-cache-dir gunicorn gevent COPY shopdb/ ./shopdb/ COPY plugins/ ./plugins/ @@ -134,21 +139,27 @@ USER shopdb EXPOSE 5001 +# Probe the public health route, not the login endpoint. Probing POST +# /api/auth/login logged a 400 every 30s and ran the credential path from a +# health check. The route answers 200 even when the database is degraded, so the +# body's status field is what decides health. Compose files that terminate TLS +# override this with an https probe; plain http is correct for the pull and +# airgap stacks, which inherit this unchanged. HEALTHCHECK --interval=30s --timeout=5s --start-period=10s --retries=3 \ - CMD curl --silent http://localhost:5001/api/auth/login -X POST \ - -H "Content-Type: application/json" -d '{}' \ - | grep -q "VALIDATION_ERROR" || exit 1 + CMD curl --silent http://localhost:5001/api/dashboard/health \ + | grep -q '"status":"ok"' || exit 1 -# Threaded workers, matching docker-compose.yml - see the reasoning there. A +# Async workers, matching docker-compose.yml - see the reasoning there. A # compose file overrides this CMD, but an image run without one (a plain # `docker run`, a site's own orchestrator) would otherwise still get the # blocking sync default and the same starvation. CMD ["gunicorn", \ "--bind", "0.0.0.0:5001", \ - "--worker-class", "gthread", \ + "--worker-class", "gevent", \ "--workers", "4", \ - "--threads", "8", \ - "--timeout", "30", \ + "--worker-connections", "2000", \ + "--timeout", "60", \ "--access-logfile", "-", \ + "--access-logformat", "%(h)s %(l)s %(u)s %(t)s \"%(r)s\" %(s)s %(b)s %(D)sus \"%(f)s\" \"%(a)s\"", \ "--error-logfile", "-", \ "wsgi:app"] diff --git a/docker-compose.yml b/docker-compose.yml index 8af86016..c80a3588 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -79,28 +79,39 @@ services: - gunicorn - --bind - 0.0.0.0:5001 - # Threaded workers, not the blocking sync default. An agent that fails - # the TLS handshake retries in a loop, and with 4 sync workers a handful - # of those pinned every worker: real UI requests queued 20+ seconds while - # the CPU sat idle. A stuck connection now holds one THREAD, and 32 - # (4 x 8) have to be exhausted before a person waits. + # Async workers, not threads. The endpoint agent long-polls + # /api/endpoints/commands/next and re-polls immediately on a 204, so every + # agent holds a connection essentially continuously and fleet size IS the + # concurrency. Under gthread that meant 4 x 8 = 32 slots total, and a + # single worker's 8 threads could fill with sleeping agents while the CPU + # sat idle - measured: 8 of 9 threads in timed sleep, UI p95 24s. # - # Found in production at the other site (their commit db75ca7). Their - # server, our agent - so the same client behaviour will reach any site - # running the endpoint agent, and the fix belongs in the product rather - # than on one branch. + # A held connection under gevent is a greenlet, not an OS thread, so idle + # is nearly free. Measured on this stack (devnotes/gevent-sizing-simulation.md): + # 700 simulated agents plus UI, integration and kiosk traffic served with + # zero errors at UI median 7.5ms, peak RSS 479MB. CPU, not memory or DB + # connections, is the binding resource. - --worker-class - - gthread + - gevent - --workers - "4" - - --threads - - "8" - # 30, not 60: a hung connection frees its thread in half the time, which - # is what decides whether a retry storm is absorbed or queues. + # Per-worker greenlet ceiling. 4 x 2000 is far above any realistic fleet; + # it exists so a connection spike queues rather than being refused. + - --worker-connections + - "2000" + # 60, not 30: the timeout must exceed _CLAIM_HOLD_SECONDS in + # plugins/endpoints/api/routes.py or gunicorn kills a healthy long-poll + # mid-hold. Cheap here because a stuck connection costs a greenlet. - --timeout - - "30" + - "60" - --access-logfile - "-" + # %(D)s is request duration in microseconds. Without it a slow request is + # indistinguishable from a fast one in the log, and latency has to be + # reconstructed from outside the container. Keep in sync with the + # Dockerfile CMD. + - --access-logformat + - '%(h)s %(l)s %(u)s %(t)s "%(r)s" %(s)s %(b)s %(D)sus "%(f)s" "%(a)s"' - --error-logfile - "-" - --certfile @@ -112,8 +123,11 @@ services: # Match the TLS listener: probe over https and accept the self-signed cert # (-k). The Dockerfile default probes http and would stay red once gunicorn # only speaks TLS. - test: ["CMD", "curl", "--silent", "--insecure", "https://localhost:5001/api/auth/login", - "-X", "POST", "-H", "Content-Type: application/json", "-d", "{}"] + # /api/dashboard/health is the public probe route and answers 200 even when + # the database is down, so the body's status field is what decides health. + # Probing POST /api/auth/login instead logged a 400 every 30s and exercised + # the credential path from a health check. + test: ["CMD-SHELL", "curl --silent --insecure https://localhost:5001/api/dashboard/health | grep -q '\"status\":\"ok\"'"] interval: 30s timeout: 5s start_period: 10s diff --git a/plugins/endpoints/api/routes.py b/plugins/endpoints/api/routes.py index d2e49458..17bac052 100644 --- a/plugins/endpoints/api/routes.py +++ b/plugins/endpoints/api/routes.py @@ -1828,6 +1828,25 @@ def revoke_agentkey(): # How long the agent claim poll is held open server-side before returning empty. # Kept below the agent HttpClient timeout and typical proxy idle timeouts so a # quiet fleet re-polls cleanly instead of tripping a read timeout. +# +# 25s is the value this endpoint is designed around; it was temporarily dropped +# to 5s under the blocking gthread worker pool, where a held connection pinned +# one of a worker's 8 OS threads. Measured then: 14 agents at hold=25 left a +# single worker with 8 of 9 threads parked in timed sleep and UI requests hashed +# to that worker waited 12-24s. Under gevent a held connection is a greenlet and +# the per-worker ceiling is --worker-connections (2000), not 8 threads, so an +# idle hold no longer costs anything a UI request needs. +# +# Restored to 25 because hold length sets the fleet's REQUEST RATE, not its +# concurrency: the agent re-polls immediately on a 204, so hold=5 is 5x the +# requests of hold=25, and TLS handshake CPU was the measured burst bottleneck +# (devnotes/gevent-sizing-simulation.md). 25 also restores the margin against +# the agent's MinIdlePollDuration=5s floor, which CommandWorker.IsIdlePollClose +# uses to tell a normal hold-window close apart from a transient failure; at +# hold=5 that margin was ~20ms. +# +# Constraint: gunicorn --timeout must stay above this or a healthy long-poll is +# killed mid-hold. docker-compose.yml sets 60. _CLAIM_HOLD_SECONDS = 25 _CLAIM_POLL_INTERVAL = 1.0