Skip to content

Commit 0bec048

Browse files
Production stress-test harness (Locust, seed/cleanup CLI, observability, runner) + fix concurrent cold-evaluation race (#607)
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent da46e06 commit 0bec048

30 files changed

Lines changed: 2256 additions & 26 deletions

backend/app/core/config.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,9 @@ class Settings(BaseSettings):
4545
ACCESS_TOKEN_EXPIRE_MINUTES: int = 60 * 24 * 8
4646
DOMAIN: str = "localhost"
4747
ENVIRONMENT: Environment = Environment.local
48+
# Env var so lowering it under Sentry quota pressure is a config flip,
49+
# not a code change + deploy.
50+
SENTRY_TRACES_SAMPLE_RATE: float = 1.0
4851

4952
@computed_field # type: ignore[misc]
5053
@property

backend/app/evaluation/main.py

Lines changed: 73 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,12 @@
77
"""
88

99
import logging
10+
from time import monotonic, sleep
1011
from typing import Any
1112

1213
from fastapi import BackgroundTasks
14+
from sqlalchemy import text
15+
from sqlalchemy.exc import IntegrityError
1316
from sqlalchemy.sql import func
1417
from sqlmodel import Session, select
1518

@@ -26,24 +29,18 @@
2629

2730
logger = logging.getLogger(__name__)
2831

32+
EVAL_LOCK_POLL_SECONDS = 1.0
33+
EVAL_LOCK_WAIT_SECONDS = 120.0
2934

30-
def update_or_select_document_evaluation(
31-
background_tasks: BackgroundTasks,
32-
session: Session,
33-
document: Document,
34-
) -> MetricsEnvelope:
35-
"""Return the document's metrics, recomputing on cache miss or stale row.
3635

37-
A cached `Evaluation` row is considered fresh iff its `payload_version`
38-
matches `CURRENT_PAYLOAD_VERSION` (registry hasn't changed) and its
39-
`updated_at` is at least as new as the document's. Otherwise the metrics
40-
are recomputed via `compute_metrics` and the row is upserted.
36+
def _cached_envelope(
37+
evaluation: Evaluation | None, document: Document
38+
) -> MetricsEnvelope | None:
39+
"""Return the cached envelope iff the row is fresh, else None.
4140
42-
Failures are never cached — a cache hit always returns `failed=[]`.
41+
Fresh iff `payload_version` matches `CURRENT_PAYLOAD_VERSION` (registry
42+
hasn't changed) and `updated_at` is at least as new as the document's.
4343
"""
44-
evaluation = session.exec(
45-
select(Evaluation).where(Evaluation.document_id == document.document_id)
46-
).one_or_none()
4744
if (
4845
evaluation
4946
and evaluation.payload_version == CURRENT_PAYLOAD_VERSION
@@ -56,6 +53,62 @@ def update_or_select_document_evaluation(
5653
metrics=evaluation.metrics,
5754
failed=[],
5855
)
56+
return None
57+
58+
59+
def update_or_select_document_evaluation(
60+
background_tasks: BackgroundTasks,
61+
session: Session,
62+
document: Document,
63+
) -> MetricsEnvelope:
64+
"""Return the document's metrics, recomputing on cache miss or stale row.
65+
66+
On a miss, a per-document Postgres advisory lock serializes computes
67+
across all backend tasks so a thundering herd of cache-cold requests runs
68+
one compute instead of N. Losers poll instead of blocking on the lock:
69+
a parked waiter holds its pooled connection for the whole compute, so
70+
~15 cache-cold requests on one document would exhaust a task's pool
71+
(5 + 10 overflow) and starve unrelated endpoints.
72+
73+
Failures are never cached — a cache hit always returns `failed=[]`.
74+
"""
75+
evaluation = session.exec(
76+
select(Evaluation).where(Evaluation.document_id == document.document_id)
77+
).one_or_none()
78+
if cached := _cached_envelope(evaluation, document):
79+
return cached
80+
81+
deadline = monotonic() + EVAL_LOCK_WAIT_SECONDS
82+
while not session.execute(
83+
text("SELECT pg_try_advisory_xact_lock(hashtextextended(:doc, 0))"),
84+
{"doc": str(document.document_id)},
85+
).scalar_one():
86+
# End the transaction so the pool reclaims our connection while we sleep.
87+
session.rollback()
88+
sleep(EVAL_LOCK_POLL_SECONDS)
89+
evaluation = session.exec(
90+
select(Evaluation).where(Evaluation.document_id == document.document_id)
91+
).one_or_none()
92+
if cached := _cached_envelope(evaluation, document):
93+
return cached
94+
if monotonic() >= deadline:
95+
# Winner is wedged; compute without the lock rather than fail —
96+
# the IntegrityError path below tolerates concurrent commits.
97+
logger.warning(
98+
"evaluation lock wait timed out for %s; computing without it",
99+
document.document_id,
100+
)
101+
break
102+
else:
103+
# A winner may have committed between our cache check and the
104+
# acquire; expire so the re-read isn't served from the identity map.
105+
session.expire_all()
106+
evaluation = session.exec(
107+
select(Evaluation).where(Evaluation.document_id == document.document_id)
108+
).one_or_none()
109+
if cached := _cached_envelope(evaluation, document):
110+
return cached
111+
59112
envelope = compute_metrics(background_tasks, session, document.document_id)
60113

61114
if evaluation:
@@ -70,7 +123,12 @@ def update_or_select_document_evaluation(
70123
payload_version=envelope["payload_version"],
71124
)
72125
)
73-
session.commit()
126+
try:
127+
session.commit()
128+
except IntegrityError:
129+
# A concurrent request computed and cached the same document first
130+
# (both saw a cold cache); our freshly computed envelope is still valid.
131+
session.rollback()
74132
return envelope
75133

76134

backend/app/main.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,8 @@
107107
if settings.ENVIRONMENT in ("production", "qa"):
108108
sentry_sdk.init(
109109
dsn="https://b14aae02017e3a9c425de4b22af7dd0c@o4507623009091584.ingest.us.sentry.io/4507623009746944",
110-
traces_sample_rate=1.0,
110+
traces_sample_rate=settings.SENTRY_TRACES_SAMPLE_RATE,
111+
# relative to traces_sample_rate, so one knob scales both
111112
profiles_sample_rate=1.0,
112113
environment=settings.ENVIRONMENT.value,
113114
)

backend/cli.py

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,14 @@
3535
from os import environ
3636
from app.models import DistrictrMap, Overlay
3737
from datetime import datetime, timezone
38+
from stress_test.config import settings as stress_settings
39+
from stress_test.seed import (
40+
NAME_PREFIX as STRESS_TEST_NAME_PREFIX,
41+
delete_documents as _delete_stress_documents,
42+
find_stress_documents as _find_stress_documents,
43+
manifest_document_ids as _manifest_document_ids,
44+
seed_documents as _seed_stress_documents,
45+
)
3846

3947

4048
logger = logging.getLogger(__name__)
@@ -902,5 +910,87 @@ def check_missing_graphs(session: Session, skip_alert: bool):
902910
logger.info("Alert published to SNS topic %s", topic_arn)
903911

904912

913+
@cli.command("stress-test-seed")
914+
@click.option(
915+
"--config-url",
916+
help="Stress-test config JSON (URL or local path). Default: $STRESS_CONFIG_URL "
917+
"or the CDN config (see stress_test/config.py).",
918+
)
919+
@click.option(
920+
"--base-url",
921+
help="API origin to create seed documents against (seeding goes over HTTP so "
922+
"docs are created exactly as the app would). Default: $STRESS_BASE_URL or "
923+
"http://localhost:8000.",
924+
)
925+
@click.option(
926+
"--run-id",
927+
help="Tags document names, User-Agent, and the manifest filename. "
928+
"Default: $STRESS_RUN_ID.",
929+
)
930+
@click.option(
931+
"--manifest",
932+
help="Manifest output path (local or s3://). "
933+
"Default: stress_test_manifest_<run-id>.json.",
934+
)
935+
@with_session
936+
def stress_test_seed(
937+
session: Session,
938+
config_url: str | None,
939+
base_url: str | None,
940+
run_id: str | None,
941+
manifest: str | None,
942+
):
943+
"""Create the stress-test seed documents from the config JSON and write the
944+
manifest of created document ids (consumed by the locustfile and by
945+
`stress-test-cleanup`)."""
946+
run_id = run_id or stress_settings.RUN_ID
947+
documents = _seed_stress_documents(
948+
session=session,
949+
base_url=base_url or stress_settings.BASE_URL,
950+
config_url=config_url or stress_settings.CONFIG_URL,
951+
run_id=run_id,
952+
manifest_path=manifest or f"stress_test_manifest_{run_id}.json",
953+
)
954+
logger.info(f"Seeded {len(documents)} documents")
955+
956+
957+
@cli.command("stress-test-cleanup")
958+
@click.option(
959+
"--manifest",
960+
"-m",
961+
"manifests",
962+
multiple=True,
963+
help="Manifest JSON (seed or runtime shape; local path or s3://). Repeatable.",
964+
)
965+
@click.option(
966+
"--yes",
967+
"-y",
968+
is_flag=True,
969+
help="Delete leftover [STRESS-TEST]-named documents without prompting",
970+
)
971+
@with_session
972+
def stress_test_cleanup(session: Session, manifests: tuple[str, ...], yes: bool):
973+
"""Delete stress-test documents (rows + assignment partitions): everything
974+
listed in the given manifests, then — belt and suspenders — any leftover
975+
document whose metadata name starts with [STRESS-TEST], after confirmation."""
976+
ids: list[str] = []
977+
for path in manifests:
978+
ids.extend(_manifest_document_ids(get_local_or_s3_path(file_path=path)))
979+
if ids:
980+
deleted = _delete_stress_documents(session, ids)
981+
logger.info(f"Deleted {deleted} of {len(ids)} manifest-listed documents")
982+
983+
leftovers = _find_stress_documents(session)
984+
if not leftovers:
985+
logger.info(f"No {STRESS_TEST_NAME_PREFIX}-named documents remain")
986+
return
987+
for document_id, name in leftovers:
988+
click.echo(f" {document_id} {name}")
989+
if yes or click.confirm(
990+
f"Delete these {len(leftovers)} {STRESS_TEST_NAME_PREFIX}-named document(s)?"
991+
):
992+
_delete_stress_documents(session, [document_id for document_id, _ in leftovers])
993+
994+
905995
if __name__ == "__main__":
906996
cli()

backend/stress_test/.gitignore

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
venv/
2+
artifacts/
3+
*.html
4+
*_stats.csv
5+
*_stats_history.csv
6+
*_failures.csv
7+
*_exceptions.csv
8+
stress_test_manifest_*.json
9+
stress_test_runtime_manifest_*.json
10+
.env

0 commit comments

Comments
 (0)