perf: optimize the person endpoint - #11635
Conversation
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>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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>
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>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This reverts commit fed51f0.
jennifer-richards
left a comment
jennifer-richards
left a comment
There was a problem hiding this comment.
Seems generally fine. Minor comment, a nit, and a more substantive change suggested for the cache keying / statement about "today" - see inline.
| # 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") |
There was a problem hiding this comment.
This obligates us to use a case-insensitive collation for this field when we move to Django 5.2. I think we want to do that anyway, but we need to remember to adjust this if we take a different approach with collation.
|
|
||
| REFERENCE_RELATIONSHIPS = ("refnorm", "refinfo", "refunk", "refold") | ||
|
|
||
| PROFILE_CACHE_SECONDS = 900 |
There was a problem hiding this comment.
perhaps expose via settings?
| """ | ||
| slowpages = caches["slowpages"] | ||
| keys = { | ||
| person.pk: f"person:profile:{person.pk}:{timezone.localdate(today)}" |
There was a problem hiding this comment.
I don't think keying on the date makes sense - it's redundant with the cache timeout knob, except in not meaningful corners. In particular, it doesn't keep all the sections of the profile page in sync with each other unless you assume particular timing of role changes relative to midnight.
I guess it's here to keep the sections all agreeing on "has no roles as of " in the display. IMO either let that vary, or (even better) change it to "does not currently have any roles" and be vague about exactly what instant that refers to. Strictly speaking that's more accurate than the existing behavior.
| </tbody> | ||
| </table> | ||
| {% else %} | ||
| <p>{{ person.first_name }} has no active roles as of {{ today|date:"Y-m-d" }}.</p> |
There was a problem hiding this comment.
See comment above about perhaps changing the "as of " text + interpolation
PERSON_PROFILE_CACHE_SECONDS, overridable from the environment in the k8s deployment. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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>
2 participants
No description provided.