perf(api): improve performance on states and modules endpoints#9275
perf(api): improve performance on states and modules endpoints#9275p11o wants to merge 3 commits into
Conversation
…yset Filtering via project__project_projectmember caused a JOIN that multiplied rows, requiring a DISTINCT to deduplicate. Replace with an Exists() correlated subquery on ProjectMember, which avoids the row inflation entirely and makes DISTINCT unnecessary.
…dule queryset
The five state-group counts used Count("issue_module__issue__state__group",
distinct=True) which traverses a 3-table join path per annotation and forces
the DB to deduplicate the inflated result set. Replace with Coalesce(Subquery)
per the pattern already established in sub_issue.py (PR makeplane#8889).
Also extract the duplicated get_queryset body from ModuleListCreateAPIEndpoint,
ModuleDetailAPIEndpoint, and ModuleArchiveUnarchiveAPIEndpoint into shared
helper functions to eliminate the copy-paste.
📝 WalkthroughWalkthroughThe PR refactors two API view files. In ChangesModule queryset centralization
State endpoint membership subquery
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/api/plane/api/views/state.py (1)
172-186: 💤 Low valueConsider extracting a shared helper to reduce duplication.
The
get_querysetimplementations inStateListCreateAPIEndpoint(lines 49-61) andStateDetailAPIEndpoint(lines 173-185) are identical. Following the same pattern applied inmodule.pywhere_build_module_querysetwas extracted, a shared helper could consolidate this logic.♻️ Optional helper extraction
def _build_state_queryset(slug, project_id, user): """Build the base State queryset with membership check.""" member_check = ProjectMember.objects.filter( project_id=OuterRef("project_id"), member=user, is_active=True, deleted_at__isnull=True, ) return ( State.objects.filter(workspace__slug=slug) .filter(project_id=project_id) .filter(Exists(member_check)) .filter(is_triage=False) .filter(project__archived_at__isnull=True) .select_related("project", "workspace") )Then both endpoints can use:
def get_queryset(self): return _build_state_queryset( self.kwargs.get("slug"), self.kwargs.get("project_id"), self.request.user, )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/plane/api/views/state.py` around lines 172 - 186, Extract the duplicated queryset building logic from both StateListCreateAPIEndpoint and StateDetailAPIEndpoint get_queryset methods into a shared helper function called _build_state_queryset. This helper should accept slug, project_id, and user as parameters and contain the ProjectMember membership check and all the filtering logic that builds the State queryset. Then update both get_queryset methods in StateListCreateAPIEndpoint and StateDetailAPIEndpoint to call this new helper function instead of duplicating the entire queryset construction logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@apps/api/plane/api/views/state.py`:
- Around line 172-186: Extract the duplicated queryset building logic from both
StateListCreateAPIEndpoint and StateDetailAPIEndpoint get_queryset methods into
a shared helper function called _build_state_queryset. This helper should accept
slug, project_id, and user as parameters and contain the ProjectMember
membership check and all the filtering logic that builds the State queryset.
Then update both get_queryset methods in StateListCreateAPIEndpoint and
StateDetailAPIEndpoint to call this new helper function instead of duplicating
the entire queryset construction logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 191d05e1-ca58-45fa-a3f1-de10936682ea
📒 Files selected for processing (2)
apps/api/plane/api/views/module.pyapps/api/plane/api/views/state.py
CI requires 80% docstring coverage. Added one-line docstrings to all functions introduced or modified by the perf fixes: _build_module_queryset, and the get_queryset methods on ModuleListCreateAPIEndpoint, ModuleDetailAPIEndpoint, ModuleArchiveUnarchiveAPIEndpoint, and both State endpoint classes.
Description
These perf updates amount to a roughly 15s latency reduction as the api calls are sequential:
/api/projects/{id}/states/The
get_querysetfiltered membership via a JOIN throughproject__project_projectmember, which multiplied rows and required.distinct()to deduplicate. Replaced with anExists()correlated subquery onProjectMember— same semantics, no row inflation./api/projects/{id}/modules/Five of the six issue-count annotations used
Count("issue_module__issue__state__group", distinct=True), traversing a 3-table join path per annotation. Replaced withCoalesce(Subquery(...annotate(count=Count("id"))))— the same pattern used in #8889 (2.6s -> 0.07s improvement on sub-issues).Also extracted the three identical
get_querysetbodies fromModuleListCreateAPIEndpoint,ModuleDetailAPIEndpoint, andModuleArchiveUnarchiveAPIEndpointinto shared helpers.Type of Change
Test Scenarios
Ran the full Docker test suite (
docker compose -f docker-compose-test.yml up --build --abort-on-container-exit): 345 passed, 8 pre-existing failures inTestMagicSign*References
#8889 — prior art for the
Coalesce(Subquery)pattern in this codebaseSummary by CodeRabbit