[MOB-1199] dev: update index constraints for notifications to optimize query performance #7870
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds six new DB indexes (five on Notification, one on FileAsset) and a non-atomic Django migration that creates them concurrently (uses AddIndexConcurrently, atomic = False, depends on migration 0107). Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (3)
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. 🧪 Early access (Sonnet 4.5): enabledWe are currently testing the Sonnet 4.5 model, which is expected to improve code review quality. However, this model may lead to increased noise levels in the review comments. Please disable the early access features if the noise level causes any inconvenience. Note:
Comment |
|
Linked to Plane Work Item(s) This comment was auto-generated by Plane |
…otification-fields
There was a problem hiding this comment.
Pull Request Overview
This PR optimizes query performance for the notification table by adding strategic database indexes to support common query patterns.
- Added 5 new composite indexes to the notification table
- Created Django migration to implement the index changes
- Focused on receiver-based queries, entity lookups, and notification state filtering
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| apps/api/plane/db/models/notification.py | Added 5 new composite indexes to optimize notification queries |
| apps/api/plane/db/migrations/0108_notification_notif_receiver_status_idx_and_more.py | Django migration to create the new indexes |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/api/plane/db/migrations/0108_notification_notif_receiver_status_idx_and_more.py (1)
13-32: Consider concurrent index creation for production deployments.The migration uses standard
AddIndexoperations, which will lock the table during index creation. For a largenotificationstable in production, this could cause downtime or blocking.Django supports concurrent index creation via
CREATE INDEX CONCURRENTLY(PostgreSQL). Consider modifying this migration to usemigrations.AddIndexConcurrentlyif deploying to a production database with significant data volume.Example modification for concurrent index creation:
from django.contrib.postgres.operations import AddIndexConcurrently class Migration(migrations.Migration): atomic = False # Required for concurrent operations operations = [ AddIndexConcurrently( model_name='notification', index=models.Index(fields=['receiver', 'workspace', 'read_at', 'created_at'], name='notif_receiver_status_idx'), ), # ... repeat for other indexes ]Note: This requires PostgreSQL and adds complexity to rollback scenarios.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/api/plane/db/migrations/0108_notification_notif_receiver_status_idx_and_more.py(1 hunks)apps/api/plane/db/models/notification.py(1 hunks)
🔇 Additional comments (5)
apps/api/plane/db/migrations/0108_notification_notif_receiver_status_idx_and_more.py (2)
1-33: Migration matches model definitions correctly.The migration operations correctly mirror the index definitions in the model file. All five indexes have matching field orders and names.
8-10: Dependency chain verified. Migration 0107 exists and directly precedes 0108—no conflicts detected.apps/api/plane/db/models/notification.py (3)
50-52: Index usage confirmed. Multiple ORM queries filter onsnoozed_tillandarchived_at(e.g., in apps/api/plane/app/views/notification/base.py around lines 72–87), so the composite index onreceiver, workspace, snoozed_till, archived_atis appropriately utilized.
57-60: Unable to locate usage of composite key filters
I couldn’t find any code filtering notifications byworkspace,entity_identifier, andentity_nametogether. Please confirm such queries exist to justify the composite index or consider removing it.
41-44: Confirm index field order matches actual query filters.
No occurrences of queries filtering by bothreceiverandworkspacewere found—please manually verify that["receiver", "workspace", "read_at", "created_at"]aligns with your most common WHERE clauses and sort patterns.
|
You have run out of free Bugbot PR reviews for this billing cycle. This will reset on October 20. To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/api/plane/db/migrations/0108_notification_notif_receiver_status_idx_and_more.py (1)
14-35: Consider index maintenance overhead and monitoring.Adding five composite indexes will improve read performance but increases:
- Write operation overhead (INSERT/UPDATE/DELETE on notifications)
- Storage requirements
- Index maintenance during vacuuming
Recommendations:
- Monitor index usage after deployment using PostgreSQL's
pg_stat_user_indexesview to identify unused indexes- Track index bloat and schedule periodic REINDEX operations if needed
- Measure query performance before/after to quantify improvements
- Consider index-only scans by including frequently-selected columns in indexes if query patterns support it
Example monitoring query:
SELECT schemaname, tablename, indexname, idx_scan, idx_tup_read, idx_tup_fetch FROM pg_stat_user_indexes WHERE tablename = 'notification' ORDER BY idx_scan;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/api/plane/db/migrations/0108_notification_notif_receiver_status_idx_and_more.py(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (2)
apps/api/plane/db/migrations/0108_notification_notif_receiver_status_idx_and_more.py (2)
1-12: LGTM! Proper setup for concurrent index creation.The migration correctly imports
AddIndexConcurrentlyand setsatomic = False, which is required for concurrent index operations in PostgreSQL to avoid table locking during index creation.
14-35: Composite indexes align with model schema and expected query patterns.All five indexes use non‐nullable leading columns (
receiver,workspace), nullable fields (read_at,snoozed_till,archived_at) appear later, and no redundant coverage exists given existing single‐ and two‐column indexes. Migrations match theMeta.indexesinNotification. No changes needed.
Description
notificationtable to improve query performance and data access efficiency.Type of Change
References
[MOB-1199]
Note
Add multiple composite indexes on
notificationsto speed up common lookups by receiver, workspace, status, entity, state, sender, and entity lookup.db.models.notification.Notificationand migration0108_*:notif_receiver_status_idxon['receiver', 'workspace', 'read_at', 'created_at'].notif_receiver_entity_idxon['receiver', 'workspace', 'entity_name', 'read_at'].notif_receiver_state_idxon['receiver', 'workspace', 'snoozed_till', 'archived_at'].notif_receiver_sender_idxon['receiver', 'workspace', 'sender'].notif_entity_lookup_idxon['workspace', 'entity_identifier', 'entity_name'].Written by Cursor Bugbot for commit 339d584. This will update automatically on new commits. Configure here.
Summary by CodeRabbit
Performance Improvements
Chores