Compare commits

..

2 Commits

Author SHA1 Message Date
taylanbakircioglu 23257b02cf perf(api): opt-in uvicorn workers + heartbeat query consolidation (v1.8.6, Issue #35)
A user running the API on a 2-core/4GB host reported slow-feeling API
responses (Issue #35 follow-up). Review of the hot paths found no
pathological defect; the dominant factors are the single uvicorn worker
(one core serves all requests) and the constant agent-poll baseline
(4 requests per agent every 30s). Two zero-risk improvements:

- backend/Dockerfile: CMD now honors UVICORN_WORKERS, falling back to
  WEB_CONCURRENCY and then 1. Flagless uvicorn natively honors
  WEB_CONCURRENCY, so the fallback keeps any deployment that relied on it
  byte-for-byte compatible; with the final default of 1 worker uvicorn runs
  in-process exactly as before. >1 enables the multiprocess supervisor so
  multi-core hosts can use all cores. Background tasks are already
  multi-replica safe (FOR UPDATE SKIP LOCKED / advisory locks), as exercised
  by the k8s HPA deployment (2-10 replicas). `exec` keeps uvicorn as PID 1
  (clean SIGTERM, verified ~1s docker stop with 2 workers).
  docker-compose.yml passes UVICORN_WORKERS through as empty-when-unset so a
  user-set WEB_CONCURRENCY is never overridden; .env.template documents it.

- routers/agent.py heartbeat (by-name endpoint): the agent's
  status/version/upgrade_status were read with three separate single-column
  SELECTs against the same row; now one SELECT. Identical values and None
  semantics (single consistent snapshot instead of three reads); saves two
  round-trips per heartbeat per agent every 30s. The legacy by-id heartbeat
  endpoint is untouched; the heartbeat API contract is unchanged for agents
  of every version.

- README: new "Performance Tuning" section (worker/replica scaling, and how
  to use the X-Response-Time header plus "Slow request detected" logs to
  pinpoint slow endpoints).

Verification: full backend suite in docker green (1063 passed, 151 skipped;
also re-run by the runtime image build); worker-count expansion matrix
(unset->1, UVICORN_WORKERS=2->2, WEB_CONCURRENCY=3->3, both->UVICORN_WORKERS,
empty->fallback) all correct; default run confirmed single-process with
uvicorn as PID 1 and healthy API; UVICORN_WORKERS=2 confirmed parent + 2
workers, healthy API, clean shutdown; live heartbeats verified for
register + existing-agent paths AND degraded agents (no stats socket /
haproxy stopped / garbage stats CSV / unknown backend in server_statuses):
all return 200, agent row updates correctly, zero backend errors.
No schema, API, or agent changes.
2026-07-06 02:27:39 +03:00
taylanbakircioglu c8d144ca9d fix(acme): remove stray paren breaking ACME completion task (v1.8.5, Issue #35)
The background order-completion task (complete_pending_acme_orders, 60s
cycle) failed on EVERY cycle since v1.8.0 with:

    [ACME-COMPLETE] Error in completion task: syntax error at or near ")"

Root cause: the bounded DNS-01 retry OR-arm added to the atomic order-claim
query in v1.8.0 (13b65d9) carried one extra closing parenthesis, making the
whole SELECT invalid PostgreSQL. The claim is the task's first statement, so
the generic except swallowed it each minute and NO background ACME work ever
ran on v1.8.0-v1.8.4:

- orders were never claimed for finalize -> download -> save (http-01 too);
- advance_dns01_order never ran, so the DNS-01 TXT record was never
  published - DNS-01 with an automated provider (e.g. Cloudflare) could
  never validate (exactly the report in Issue #35);
- wizard-staged orders never left wizard_staged (same try block);
- retry_invalid_dns01 / reconcile_dns01_cleanup never executed;
- hourly-created renewal orders could never complete in the background.

Fix: drop the stray ')' (one line). Query semantics are unchanged.

Why the suite missed it: the unit tests mock asyncpg, so raw SQL never
reaches a real parser. Added a regression test that AST-scans the ACME
modules' SQL string literals (comments/quoted literals stripped) and fails
on unbalanced parentheses - it is red on the pre-fix tree and would have
caught the v1.8.0 regression at commit time. Scanned all six ACME modules:
this query was the only unbalanced SQL.

Verification: full backend suite in docker green (1062 passed, 151
skipped); the fixed query EXPLAINs cleanly on postgres:15; live localtest
run shows zero completion-task errors and a seeded pending order is claimed
("[ACME-COMPLETE] Claimed 1 order(s)"). Backend-only, no schema/API/agent
changes; fully backward compatible.

Reported-by: @tkkost (GitHub Issue #35)
2026-07-03 02:07:43 +03:00
9 changed files with 156 additions and 10 deletions
+9
View File
@@ -59,6 +59,15 @@ AGENT_HEARTBEAT_TIMEOUT_SECONDS=15
# Config sync interval in seconds
AGENT_CONFIG_SYNC_INTERVAL_SECONDS=30
# ============================================================================
# BACKEND PERFORMANCE
# ============================================================================
# Number of uvicorn worker processes for the backend API (default: 1).
# On multi-core hosts, setting this to the core count (e.g. 2) lets the API
# use all cores. Safe to increase: background tasks are multi-replica safe
# (the k8s deployment already runs 2+ replicas via HPA).
UVICORN_WORKERS=1
# ============================================================================
# CORS CONFIGURATION
# ============================================================================
+15
View File
@@ -1738,6 +1738,19 @@ Add new HAProxy clusters through the web interface or directly via API:
}
```
### Performance Tuning *(v1.8.6)*
The backend API defaults to a **single uvicorn worker process**, which uses one CPU core. Agents poll the API every 30 seconds (heartbeat, config, pending-requests, upgrade checks), so larger fleets add a constant baseline load. Two ways to scale:
- **Docker Compose — worker processes**: set `UVICORN_WORKERS` in your `.env` (default `1`). On a multi-core host, matching the core count (e.g. `UVICORN_WORKERS=2` on a 2-core machine) lets the API use all cores:
```bash
echo "UVICORN_WORKERS=2" >> .env && docker-compose up -d backend
```
- **Kubernetes/OpenShift — replicas**: the shipped manifests already include an HPA for the backend (2→10 replicas, `k8s/manifests/13-hpa.yaml`); raise `minReplicas`/`maxReplicas` as needed.
Both are safe: all background tasks (ACME completion, renewals, agent monitoring) are multi-replica safe by design (atomic claims via `FOR UPDATE SKIP LOCKED`, PostgreSQL advisory locks).
**Diagnosing slow requests**: every API response carries an `X-Response-Time` header, and the backend logs `Slow request detected` (WARNING) for any request taking longer than 1 second — check those log lines to pinpoint slow endpoints before tuning anything else.
## API Reference
@@ -2415,6 +2428,8 @@ Developed with ❤️ for the HAProxy community
## Release Notes
- **v1.8.6** (2026-07-06) — **Performance: opt-in API workers + heartbeat micro-optimization** (Issue #35 follow-up): the backend container can now run multiple uvicorn worker processes via the new `UVICORN_WORKERS` environment variable (default **1** — behavior unchanged unless you opt in), letting the API use all cores on multi-core hosts; background tasks were already multi-replica safe, as exercised by the Kubernetes HPA deployment. The agent heartbeat handler now reads the agent's `status`/`version`/`upgrade_status` in one query instead of three (one round-trip per heartbeat, per agent, every 30s). Added a *Performance Tuning* section to the README (worker/replica scaling and how to use the `X-Response-Time` header and `Slow request detected` logs to pinpoint slow endpoints). Zero-risk release: no schema, API, or agent changes; defaults preserve existing behavior exactly.
- **v1.8.5** (2026-07-03) — **ACME completion-task SQL fix** (Issue #35 follow-up): the background order-completion task (`complete_pending_acme_orders`, runs every 60s) died on **every cycle** with `syntax error at or near ")"` — an extra closing parenthesis introduced in v1.8.0's bounded DNS-01 retry claim query. Because that query is the task's first database call, **no background ACME work ran at all from v1.8.0 through v1.8.4**: orders were never claimed for finalize/download, the DNS-01 TXT record was never published (so DNS-01 with an automated provider such as Cloudflare could never validate), Site Wizard staged orders never left `wizard_staged`, and DNS-01 retry/TXT-cleanup never executed. The stray parenthesis is removed and a regression test now scans all ACME modules' SQL for unbalanced parentheses (the unit suite mocks the database, which is why a raw-SQL syntax error could slip through). One-line backend query fix; no schema, API, or agent changes — fully backward compatible.
- **v1.8.4** (2026-06-27) — **Agent installer self-kill fix** (Issue #31): the Linux/macOS agent installer could abort during "pre-installation cleanup" (terminal showed `Killing processes matching: haproxy-agent` then `Killed`) when the install script's own filename contained "haproxy-agent". The cleanup killed processes by matching the bare string "haproxy-agent" against full command lines, which also matched the running installer (and a `sudo`/PAM ancestor the self-exclusion did not cover), so the installer terminated itself. Cleanup now targets only the installed agent (the `$INSTALL_DIR/haproxy-agent` binary and the agent service), never the bare string, and the UI now names the downloaded scripts `install-agent-<platform>.sh` / `uninstall-agent-<platform>.sh`. Installer-only change; the running agent and its privilege model (it runs as root for HAProxy reload, config writes, keepalived, and self-upgrade) are unchanged.
- **v1.8.3** (2026-06-25) — **Agent heartbeat JSON fix** (Issue #31): a self-hosted agent could fail every heartbeat with `HTTP 400 Invalid JSON: Expecting property name enclosed in double quotes` when the system-info block it collects came back empty on an unusual host, leaving a stray comma in the hand-built heartbeat JSON. The agent script now substitutes a valid placeholder when that block is empty so it can no longer emit a stray comma, and the backend heartbeat endpoint now parses valid payloads as-is and, only when a body fails to parse, tolerates that specific malformed pattern (a leading or doubled comma) so an already-deployed agent recovers on its next heartbeat after this build is deployed. Backend + agent-script only; healthy agents of every version are byte-for-byte unaffected.
- **v1.8.2** (2026-06-25) — **ACME nonce fix** (Issue #35 follow-up): the ACME client now scopes the anti-replay nonce **per certificate authority** so a nonce issued by one CA is never sent to another. This fixes ZeroSSL/Google account registration failing with `malformed: The Replay Nonce could not be base64url-decoded` (the client previously shared one nonce across CAs and only auto-retried on `badNonce`). Account registration now always uses a fresh nonce from the target CA, and the retry covers this case too. Backend-only; HTTP-01 and Let's Encrypt are unaffected.
+10 -2
View File
@@ -37,5 +37,13 @@ USER appuser
# Expose port
EXPOSE 8000
# Run the application in production mode (without --reload)
CMD ["uvicorn", "main:app", "--host", "0.0.0.0", "--port", "8000"]
# Run the application in production mode (without --reload).
# UVICORN_WORKERS (default 1) opts into multiple worker processes on multi-core
# hosts; with 1 worker uvicorn runs in-process, identical to the flagless CMD
# this replaces. Falls back to WEB_CONCURRENCY when UVICORN_WORKERS is unset
# because flagless uvicorn honored WEB_CONCURRENCY (uvicorn config.py) — this
# keeps any deployment that relied on it byte-for-byte compatible. Background
# tasks are multi-replica safe (FOR UPDATE SKIP LOCKED / advisory locks), as
# already exercised by the k8s HPA deployment. `exec` keeps uvicorn as PID 1
# so signal handling is unchanged.
CMD ["sh", "-c", "exec uvicorn main:app --host 0.0.0.0 --port 8000 --workers ${UVICORN_WORKERS:-${WEB_CONCURRENCY:-1}}"]
-1
View File
@@ -318,7 +318,6 @@ async def complete_pending_acme_orders():
OR dns01_last_attempt_at < NOW() - (
(CASE COALESCE(dns01_attempts, 0) WHEN 0 THEN 15 WHEN 1 THEN 30 ELSE 60 END)
|| ' minutes')::INTERVAL
)
)
)
)
+7 -3
View File
@@ -1695,9 +1695,13 @@ async def agent_heartbeat_by_name(
""", x_api_key)
# Check if agent was in upgrading status and version has changed
current_agent_status = await conn.fetchval("SELECT status FROM agents WHERE id = $1", agent_id)
current_agent_version = await conn.fetchval("SELECT version FROM agents WHERE id = $1", agent_id)
current_upgrade_status = await conn.fetchval("SELECT upgrade_status FROM agents WHERE id = $1", agent_id)
# (v1.8.6: one round-trip instead of three; row is None exactly when the
# per-column fetchvals would each have returned None)
current_agent_row = await conn.fetchrow(
"SELECT status, version, upgrade_status FROM agents WHERE id = $1", agent_id)
current_agent_status = current_agent_row['status'] if current_agent_row else None
current_agent_version = current_agent_row['version'] if current_agent_row else None
current_upgrade_status = current_agent_row['upgrade_status'] if current_agent_row else None
# Determine new status - preserve upgrading status unless version actually changed
new_status = current_agent_status or 'online'
+108
View File
@@ -110,3 +110,111 @@ def test_nonce_scoped_per_directory():
assert got == "NONCE_A" # returns THIS CA's nonce
assert svc._nonce_by_dir.get("https://a.example/dir") is None # consumed (single-use)
assert svc._nonce_by_dir.get("https://b.example/dir") == "NONCE_B" # the other CA is untouched
def _sql_paren_depth(sql: str):
"""Parenthesis depth of a SQL string, counting only OUTSIDE '...' literals (with ''
escapes), `--` line comments and /* */ block comments. Single-pass state machine so a
`--` inside a literal or a `'` inside a comment cannot corrupt the count. Dollar-quoted
strings are out of scope (not used in this codebase). Returns (final_depth, min_depth).
"""
depth = 0
min_depth = 0
state = "normal"
i, n = 0, len(sql)
while i < n:
ch = sql[i]
nxt = sql[i + 1] if i + 1 < n else ""
if state == "normal":
if ch == "'":
state = "string"
elif ch == "-" and nxt == "-":
state = "line_comment"
i += 1
elif ch == "/" and nxt == "*":
state = "block_comment"
i += 1
elif ch == "(":
depth += 1
elif ch == ")":
depth -= 1
min_depth = min(min_depth, depth)
elif state == "string":
if ch == "'":
if nxt == "'":
i += 1 # escaped '' stays inside the literal
else:
state = "normal"
elif state == "line_comment":
if ch == "\n":
state = "normal"
else: # block_comment
if ch == "*" and nxt == "/":
state = "normal"
i += 1
i += 1
return depth, min_depth
def test_acme_sql_parentheses_balanced():
"""Issue #35 v1.8.5: the completion task's order-claim query shipped (v1.8.0-v1.8.4) with an
extra closing parenthesis, so EVERY 60s cycle died with `syntax error at or near ")"` and no
background ACME work (claim/finalize/download, DNS-01 publish, wizard-staged promotion,
retry, TXT cleanup) ever ran. The suite never caught it because the DB layer is mocked and
raw SQL never reaches a real parser. This guard scans the ACME modules' SQL string literals
for unbalanced parentheses.
Guard scope is deliberately conservative to avoid false positives on production changes:
keyword matching is case-sensitive (SQL is uppercase in this codebase; prose in docstrings
is not) and f-string fragments are excluded (they split at `{`, so a fragment may be
legitimately unbalanced).
"""
import ast
import re
backend_dir = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
modules = [
"main.py",
os.path.join("services", "dns01_orchestrator.py"),
os.path.join("services", "acme_service.py"),
os.path.join("services", "letsencrypt_service.py"),
os.path.join("routers", "letsencrypt.py"),
os.path.join("routers", "acme_diagnostics.py"),
]
problems = []
for rel in modules:
with open(os.path.join(backend_dir, rel), encoding="utf-8") as fh:
tree = ast.parse(fh.read())
fstring_parts = {
id(const)
for joined in ast.walk(tree) if isinstance(joined, ast.JoinedStr)
for const in ast.walk(joined) if isinstance(const, ast.Constant)
}
for node in ast.walk(tree):
if not (isinstance(node, ast.Constant) and isinstance(node.value, str)):
continue
if id(node) in fstring_parts:
continue
sql = node.value
if not re.search(r"\b(SELECT|INSERT|UPDATE|DELETE)\b", sql):
continue
if not re.search(r"\b(FROM|INTO|SET|WHERE)\b", sql):
continue
depth, min_depth = _sql_paren_depth(sql)
if depth != 0 or min_depth < 0:
problems.append(f"{rel}:{node.lineno} (paren depth {depth:+d}, min {min_depth})")
assert not problems, f"Unbalanced parentheses in SQL literal(s): {problems}"
def test_sql_paren_depth_scanner():
# The guard's scanner itself: parens in literals/comments must not count; '' escapes and
# block comments handled; an extra ')' is reported via min_depth even if a later '(' would
# re-balance the total.
assert _sql_paren_depth("SELECT (1)") == (0, 0)
assert _sql_paren_depth("SELECT (1))") == (-1, -1) # the v1.8.0 bug shape
assert _sql_paren_depth("SELECT ')' , '((' FROM t") == (0, 0) # literals ignored
assert _sql_paren_depth("SELECT 'it''s ))' FROM t") == (0, 0) # '' escape stays inside
assert _sql_paren_depth("SELECT 1 -- comment ) (\nFROM t") == (0, 0) # line comment ignored
assert _sql_paren_depth("SELECT 1 /* ) */ FROM t") == (0, 0) # block comment ignored
assert _sql_paren_depth("SELECT 'a--b' AND (x=1\n)") == (0, 0) # -- inside literal is data
assert _sql_paren_depth("WHERE x) AND (y") == (0, -1) # net 0 but went negative
+3
View File
@@ -51,6 +51,9 @@ services:
- LOG_LEVEL=INFO
- PUBLIC_URL=http://localhost:8080
- MANAGEMENT_BASE_URL=http://localhost:8080
# Empty when unset on the host: the image CMD then falls back to
# WEB_CONCURRENCY (uvicorn's native env) and finally to 1.
- UVICORN_WORKERS=${UVICORN_WORKERS:-}
volumes:
- haproxy_configs:/etc/haproxy
expose:
+1 -1
View File
@@ -1,6 +1,6 @@
{
"name": "haproxy-openmanager-frontend",
"version": "1.8.4",
"version": "1.8.6",
"description": "HAProxy Load Balancer Management UI",
"license": "AGPL-3.0-or-later",
"dependencies": {
+3 -3
View File
@@ -1,5 +1,5 @@
{
"version": "1.8.4",
"releaseName": "Agent installer self-kill fix",
"releaseDate": "2026-06-27"
"version": "1.8.6",
"releaseName": "Opt-in API workers + heartbeat micro-optimization",
"releaseDate": "2026-07-06"
}