justin
1562febdcf
Opening any person page on a large tree took 4-5s on an idle server. Root cause: list_persons looped over every person calling privacy.person_visibility (which issues TWO get_membership_role queries per call) AND _attach_primary_name (one name query per person). On the reporter's 2,324-person tree that's ~7,000 serialized DB round-trips per page load — the person page fetches the full person list to build its name-lookup map. Fix: - Resolve the viewer's membership role ONCE. Members see the whole tree (full), so skip the per-person privacy engine entirely. - Add _attach_primary_names: one batched names query (person_id IN (...), ordered the same as the single-person query so it picks the same name) instead of one per person. - Apply the same batching to the non-member path, search_persons, the deleted- persons list, and public_view_service.list_public_persons. Member-path list_persons goes from ~3·N queries to ~3 total. Other tree-wide list endpoints (events/relationships/media/citations) were already flat selects. Adds a regression test that asserts list_persons issues a constant number of queries (not proportional to person count). Suite: 103 passing. Signed-off-by: Justin Paul <justin@jpaul.me>
40 lines
1.5 KiB
Python
40 lines
1.5 KiB
Python
"""Regression guard: list_persons must batch — a constant number of queries,
|
|
not one (or three) per person. A 2k-person tree took ~4s before this was fixed."""
|
|
|
|
import sqlalchemy as sa
|
|
|
|
from tests.conftest import auth, register
|
|
|
|
|
|
async def test_list_persons_does_not_n_plus_one(client, engine):
|
|
owner = auth(await register(client, "perf-owner@ex.com"))
|
|
tid = (await client.post("/api/v1/trees", json={"name": "Perf"}, headers=owner)).json()["id"]
|
|
n = 25
|
|
for i in range(n):
|
|
await client.post(
|
|
f"/api/v1/trees/{tid}/persons",
|
|
json={"given": f"P{i}", "surname": "X"},
|
|
headers=owner,
|
|
)
|
|
|
|
selects = 0
|
|
|
|
def _count(conn, cursor, statement, params, context, executemany):
|
|
nonlocal selects
|
|
if statement.lstrip().upper().startswith("SELECT"):
|
|
selects += 1
|
|
|
|
sa.event.listen(engine.sync_engine, "before_cursor_execute", _count)
|
|
try:
|
|
resp = await client.get(f"/api/v1/trees/{tid}/persons", headers=owner)
|
|
finally:
|
|
sa.event.remove(engine.sync_engine, "before_cursor_execute", _count)
|
|
|
|
assert resp.status_code == 200
|
|
body = resp.json()
|
|
assert len(body) == n
|
|
assert all(p["primary_name"] for p in body) # names still resolve correctly
|
|
# Batched: a small constant (auth, role, persons, one names query, …) — NOT
|
|
# proportional to n. The old per-person path was ~3·n SELECTs.
|
|
assert 0 < selects < n, f"expected a constant query count, got {selects} for {n} people"
|