perf: optimize the person endpoint (#11635)

* perf: cap per-client concurrency on /person/ and /api/v1/

Keyed on the Cloudflare client address, with an empty key for every other
path so only these two prefixes are limited.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* perf: query the two authorship tables separately in Person.rfcs()

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* perf: avoid table scans and per-alias queries in lookup_persons

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* perf: assemble the profile page's data in the view

Gathers the RFC publication dates, reference counts and replaced-draft set for
every listed person in one query each, rather than a query per table cell, and
evaluates each per-person list once instead of on every template reference.

The expired Internet-Drafts heading now counts the drafts it lists; it counted
the replaced ones the list omits. Roles with the same name sort by group
acronym instead of by whatever order the query returned.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* perf: cache each rendered profile section

A repeat view of a profile, including the revalidation behind a conditional
request, now costs neither the queries nor the render. Sections are keyed on
person and date rather than position on the page, so the per-section element
ids move from a loop counter to the person's id.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* test: guard the profile page's query count and section cache

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* revert: perf: cap per-client concurrency on /person/ and /api/v1/

This reverts commit fed51f0.

* refactor: make the profile section cache lifetime a setting

PERSON_PROFILE_CACHE_SECONDS, overridable from the environment in the k8s
deployment.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix: stop dating the profile page's empty-section messages

The dates claimed a precision the page does not have: sections are cached
independently, so the data behind two of them can differ by a cache lifetime
while both printed the same date. Without them nothing in a section depends on
when it was rendered, so the cache key no longer needs the date either.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

Author: rjsparks

ietf/person/models.py

@@ -278,9 +278,21 @@ def rfcs(self):
         # When RfcAuthors are populated, this may over-return if an author is dropped
         # from the author list between the final draft and the published RFC. Should
         # ignore DocumentAuthors when an RfcAuthor exists for a draft.
-        rfcs = list(Document.objects.filter(type="rfc").filter(models.Q(documentauthor__person=self)|models.Q(rfcauthor__person=self)).distinct())
-        rfcs.sort(key=lambda d: d.name )
-        return rfcs
+        #
+        # The two authorship tables are queried separately and combined here. As a
+        # single ORed queryset, neither person_id index is usable and the join has to
+        # be materialized in full before being deduplicated.
+        ids = set(
+            Document.objects.filter(
+                type="rfc", documentauthor__person=self
+            ).values_list("pk", flat=True)
+        )
+        ids.update(
+            Document.objects.filter(
+                type="rfc", rfcauthor__person=self
+            ).values_list("pk", flat=True)
+        )
+        return sorted(Document.objects.filter(pk__in=ids), key=lambda d: d.name)
 
     def active_drafts(self):
         from ietf.doc.models import Document

ietf/person/tests.py

@@ -27,6 +27,7 @@
 import debug                            # pyflakes:ignore
 
 from ietf.community.models import CommunityList
+from ietf.doc.factories import WgDraftFactory, WgRfcFactory
 from ietf.group.factories import RoleFactory
 from ietf.group.models import Group
 from ietf.message.models import Message
@@ -125,6 +126,61 @@ def test_person_profile(self):
         r = self.client.get(photo_url)
         self.assertEqual(r.status_code, 200)
 
+    def test_person_profile_query_count(self):
+        """The page's cost must not scale with how much a person has written"""
+
+        def profile_queries(rfcs, active, expired):
+            person = PersonFactory()
+            RoleFactory(person=person, name_id="chair")
+            WgRfcFactory.create_batch(rfcs, authors=[person])
+            WgDraftFactory.create_batch(active, authors=[person])
+            WgDraftFactory.create_batch(
+                expired, authors=[person], states=[("draft", "expired")]
+            )
+            url = urlreverse(
+                "ietf.person.views.profile",
+                kwargs={"email_or_name": person.plain_name()},
+            )
+            with CaptureQueriesContext(connection) as context:
+                r = self.client.get(url)
+            self.assertEqual(r.status_code, 200)
+            return len(context.captured_queries)
+
+        few = profile_queries(1, 1, 1)
+        many = profile_queries(6, 4, 5)
+        self.assertEqual(
+            many,
+            few,
+            f"{many} queries for 15 documents vs {few} for 3 - a query per row crept in",
+        )
+
+    @override_settings(
+        CACHES={
+            "default": {"BACKEND": "django.core.cache.backends.dummy.DummyCache"},
+            "slowpages": {
+                "BACKEND": "django.core.cache.backends.locmem.LocMemCache",
+                "LOCATION": "test-person-profile",
+            },
+        }
+    )
+    def test_person_profile_sections_cached(self):
+        person = PersonFactory()
+        WgRfcFactory(authors=[person])
+        WgDraftFactory(authors=[person])
+        url = urlreverse(
+            "ietf.person.views.profile", kwargs={"email_or_name": person.plain_name()}
+        )
+
+        first = self.client.get(url)
+        self.assertEqual(first.status_code, 200)
+        with CaptureQueriesContext(connection) as context:
+            second = self.client.get(url)
+        self.assertEqual(second.status_code, 200)
+        # The cached section is HTML, not text to be escaped again.
+        self.assertEqual(first.content, second.content)
+        self.assertContains(second, person.name)
+        self.assertLess(len(context.captured_queries), 5)
+
     def test_person_profile_without_email(self):
         person = PersonFactory(name="foobar@example.com")
         # delete Email record

ietf/person/utils.py

@@ -337,11 +337,14 @@ def get_dots(person):
         return dots
 
 def lookup_persons(email_or_name):
-    aliases = Alias.objects.filter(name__iexact=email_or_name)
+    aliases = Alias.objects.filter(name__iexact=email_or_name).select_related("person")
     persons = set(a.person for a in aliases)
 
     if '@' in email_or_name:
-        emails = Email.objects.filter(address__iexact=email_or_name)
+        # Email.address is a citext column, so an exact match is already
+        # case-insensitive and can use the index on it. Asking for iexact wraps the
+        # column in UPPER() and costs a scan of the table.
+        emails = Email.objects.filter(address=email_or_name).select_related("person")
         persons.update(e.person for e in emails)
 
     persons = [p for p in persons if p and p.id]

ietf/person/views.py

@@ -7,14 +7,15 @@
 
 from django.conf import settings
 from django.contrib import messages
-from django.db.models import Q
+from django.core.cache import caches
+from django.db.models import Count, Q
 from django.http import HttpResponse, Http404
 from django.shortcuts import render, redirect
 from django.template.loader import render_to_string
-from django.utils import timezone
 
 import debug                            # pyflakes:ignore
 
+from ietf.doc.models import DocEvent, RelatedDocument
 from ietf.ietfauth.utils import role_required
 from ietf.person.models import Email, Person
 from ietf.person.fields import select2_id_name_json
@@ -26,6 +27,9 @@
     merge_persons,
 )
 from ietf.utils.mail import send_mail_text
+from ietf.utils.timezone import RPC_TZINFO
+
+REFERENCE_RELATIONSHIPS = ("refnorm", "refinfo", "refunk", "refold")
 
 
 def ajax_select2_search(request, model_name):
@@ -77,9 +81,136 @@ def ajax_select2_search(request, model_name):
     return HttpResponse(select2_id_name_json(objs), content_type='application/json')
 
 
+def rfc_rows(persons):
+    """Build the RFC table rows for each person
+
+    Returns a dict keyed on person pk. The columns are gathered for every person at
+    once - read one at a time off the Document, each row costs a query per column.
+    """
+    rfcs = {p.pk: p.rfcs() for p in persons}
+    rfc_ids = {d.pk for docs in rfcs.values() for d in docs}
+
+    # The references of the draft an RFC was published from count as the RFC's own.
+    draft_of = dict(
+        RelatedDocument.objects.filter(
+            target_id__in=rfc_ids, relationship="became_rfc"
+        ).values_list("target_id", "source_id")
+    )
+    referenced_by = dict(
+        RelatedDocument.objects.filter(
+            target_id__in=rfc_ids | set(draft_of.values()),
+            relationship__in=REFERENCE_RELATIONSHIPS,
+            source__type__slug="rfc",
+        )
+        .values("target_id")
+        .annotate(count=Count("id"))
+        .values_list("target_id", "count")
+    )
+
+    # Matches Document.latest_event ordering, so the first row seen for a document
+    # is the one its pub_date would have reported.
+    published = {}
+    for doc_id, time in (
+        DocEvent.objects.filter(doc_id__in=rfc_ids, type="published_rfc")
+        .order_by("-time", "-id")
+        .values_list("doc_id", "time")
+    ):
+        published.setdefault(doc_id, time)
+
+    return {
+        pk: [
+            {
+                "doc": doc,
+                "pub_date": (
+                    published[doc.pk].astimezone(RPC_TZINFO).date()
+                    if doc.pk in published
+                    else None
+                ),
+                "referenced_by": referenced_by.get(doc.pk, 0)
+                + referenced_by.get(draft_of.get(doc.pk), 0),
+            }
+            for doc in docs
+        ]
+        for pk, docs in rfcs.items()
+    }
+
+
+def profile_data(persons):
+    """Build everything person/profile.html renders for each of persons"""
+    rfcs = rfc_rows(persons)
+    expired = {p.pk: list(p.expired_drafts().prefetch_related("states")) for p in persons}
+    replaced = set(
+        RelatedDocument.objects.filter(
+            target_id__in={d.pk for docs in expired.values() for d in docs},
+            relationship="replaces",
+        ).values_list("target_id", flat=True)
+    )
+
+    profiles = []
+    for person in persons:
+        # Role.Meta orders by name_id alone, which leaves ties to the query plan.
+        roles = sorted(
+            person.role_set.select_related("name", "group", "email"),
+            key=lambda r: (r.name_id, r.group.acronym),
+        )
+        profiles.append(
+            {
+                "person": person,
+                "has_roles": bool(roles),
+                "roles": [
+                    r
+                    for r in roles
+                    if r.group.state_id in ["active", "bof"]
+                    and r.group.acronym != "secretariat"
+                ],
+                "ext_resources": list(
+                    person.personextresource_set.select_related("name")
+                ),
+                "rfcs": rfcs[person.pk],
+                "active_drafts": list(
+                    person.active_drafts().prefetch_related("states")
+                ),
+                "expired_drafts": [
+                    d for d in expired[person.pk] if d.pk not in replaced
+                ],
+                "has_drafts": person.has_drafts(),
+            }
+        )
+    return profiles
+
+
+def profile_sections(persons):
+    """Render each person's part of the profile page
+
+    The rendered sections are cached, so a repeat view of a profile - including the
+    revalidation a conditional request makes - costs neither the queries nor the
+    render. Nothing in a section is tied to the moment it was rendered, so how stale
+    one can be is entirely PERSON_PROFILE_CACHE_SECONDS.
+    """
+    slowpages = caches["slowpages"]
+    keys = {person.pk: f"person:profile:{person.pk}" for person in persons}
+    sections = slowpages.get_many(list(keys.values()))
+
+    uncached = [person for person in persons if keys[person.pk] not in sections]
+    for profile in profile_data(uncached):
+        person = profile["person"]
+        section = {
+            "id": person.pk,
+            "name": str(person),
+            "has_drafts": profile["has_drafts"],
+            "html": render_to_string("person/profile_body.html", {"profile": profile}),
+        }
+        slowpages.set(keys[person.pk], section, settings.PERSON_PROFILE_CACHE_SECONDS)
+        sections[keys[person.pk]] = section
+
+    return [sections[keys[person.pk]] for person in persons]
+
+
 def profile(request, email_or_name):
     persons = lookup_persons(email_or_name)
-    return render(request, 'person/profile.html', {'persons': persons, 'today': timezone.now()})
+    return render(
+        request, "person/profile.html", {"sections": profile_sections(persons)}
+    )
 
 
 def profile_by_uuid(request, uuid):
@@ -95,7 +226,7 @@ def profile_by_uuid(request, uuid):
     return render(
         request,
         "person/profile.html",
-        {"persons": [person_uuid.person], "today": timezone.now()},
+        {"sections": profile_sections([person_uuid.person])},
     )
 
 

ietf/settings.py

@@ -886,6 +886,10 @@ def skip_unreadable_post(record):
 PDFIZER_CACHE_TIME = HTMLIZER_CACHE_TIME
 PDFIZER_URL_PREFIX = IDTRACKER_BASE_URL+"/doc/pdf"
 
+# How long a rendered person profile section is served from the slowpages cache.
+# This is how stale a profile's roles and documents can be.
+PERSON_PROFILE_CACHE_SECONDS = 60*15    # 15 minutes
+
 # Email settings
 IPR_EMAIL_FROM = 'ietf-ipr@ietf.org'
 AUDIO_IMPORT_EMAIL = ['ietf@meetecho.com']