Summary
Most API responses include, for every object, the actions the current user may take on it (user_permissions). Today that list is worked out separately for each row of a response, even though on a project-scoped page the user and the project are the same for every row. As a result, list endpoints and dropdowns run several extra database queries per row, and the cost grows with the page size. Doing this work once per request would make list pages cheaper across the API and remove a trap that new serializers fall into without noticing.
What we observed
Cachalot serves repeated queries from cache, so this is hard to see when testing by hand.
Where it comes from (based on reading the code)
DefaultSerializer.to_representation (ami/base/serializers.py) calls get_permissions for every row, and almost every serializer inherits it. A new serializer pays the per-row cost unless its author knows to opt out.
- For each row,
BaseModel.get_user_object_permissions resolves the project and calls get_perms. Job.get_custom_user_permissions also builds a debug message with an f-string that includes the job, which formats the job (reading status) even when debug logging is off. When the queryset used .only(), that read is an extra query per row.
- One serializer already avoids this:
TaxaListSerializer.get_permissions (ami/main/api/serializers.py) caches the project and the user's project permissions on the serializer, which DRF reuses for every row of a list response, so they are resolved once per request. A few nested serializers instead skip permissions by returning an empty list.
Direction to discuss
- Move the per-request caching from
TaxaListSerializer into DefaultSerializer.get_permissions, keyed by project id, so every serializer resolves a user's project permissions once per request rather than once per row. Object-level checks that genuinely differ per row (for example, whether the user created an identification) would stay per row.
- Switch the debug logging in the permission code to lazy
%s formatting, so messages are not built when debug is off.
- Optionally, have the test suite fail on repeated lazy relation loads (for example with django-zeal). It would not catch the permission queries themselves, which is why the first item is the main fix.
What we still need to verify
- Re-measure the occurrence list, the captures list, and the jobs list before and after, with cachalot disabled and at two page sizes, and confirm the per-row count drops to zero or near it.
- Check that
user_permissions in responses is unchanged for each role (anonymous, non-member, member, project manager, superuser), using the existing permission tests.
- Confirm no serializer relies on a per-row difference in project permissions, for example a list spanning several projects. Those cases would need the cache keyed by project rather than a single value per request.
Related
Summary
Most API responses include, for every object, the actions the current user may take on it (
user_permissions). Today that list is worked out separately for each row of a response, even though on a project-scoped page the user and the project are the same for every row. As a result, list endpoints and dropdowns run several extra database queries per row, and the cost grows with the page size. Doing this work once per request would make list pages cheaper across the API and remove a trap that new serializers fall into without noticing.What we observed
OccurrenceListSerializer.get_permissions: a project lookup and Guardian'sget_perms(user, project).Cachalot serves repeated queries from cache, so this is hard to see when testing by hand.
Where it comes from (based on reading the code)
DefaultSerializer.to_representation(ami/base/serializers.py) callsget_permissionsfor every row, and almost every serializer inherits it. A new serializer pays the per-row cost unless its author knows to opt out.BaseModel.get_user_object_permissionsresolves the project and callsget_perms.Job.get_custom_user_permissionsalso builds a debug message with an f-string that includes the job, which formats the job (readingstatus) even when debug logging is off. When the queryset used.only(), that read is an extra query per row.TaxaListSerializer.get_permissions(ami/main/api/serializers.py) caches the project and the user's project permissions on the serializer, which DRF reuses for every row of a list response, so they are resolved once per request. A few nested serializers instead skip permissions by returning an empty list.Direction to discuss
TaxaListSerializerintoDefaultSerializer.get_permissions, keyed by project id, so every serializer resolves a user's project permissions once per request rather than once per row. Object-level checks that genuinely differ per row (for example, whether the user created an identification) would stay per row.%sformatting, so messages are not built when debug is off.What we still need to verify
user_permissionsin responses is unchanged for each role (anonymous, non-member, member, project manager, superuser), using the existing permission tests.Related