-
Notifications
You must be signed in to change notification settings - Fork 3.4k
[MOB-1199] dev: update index constraints for notifications to optimize query performance #7870
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
8999120
339d584
3e27e55
8e2616f
f3b9ce7
d1829d4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| # Generated by Django 4.2.22 on 2025-09-29 15:36 | ||
|
|
||
| from django.db import migrations, models | ||
| from django.contrib.postgres.operations import AddIndexConcurrently | ||
|
|
||
|
|
||
| class Migration(migrations.Migration): | ||
| atomic = False | ||
|
|
||
| dependencies = [ | ||
| ('db', '0110_workspaceuserproperties_navigation_control_preference_and_more'), | ||
| ] | ||
|
|
||
| operations = [ | ||
| AddIndexConcurrently( | ||
| model_name='notification', | ||
| index=models.Index(fields=['receiver', 'workspace', 'read_at', 'created_at'], name='notif_receiver_status_idx'), | ||
| ), | ||
| AddIndexConcurrently( | ||
| model_name='notification', | ||
| index=models.Index(fields=['receiver', 'workspace', 'entity_name', 'read_at'], name='notif_receiver_entity_idx'), | ||
| ), | ||
| AddIndexConcurrently( | ||
| model_name='notification', | ||
| index=models.Index(fields=['receiver', 'workspace', 'snoozed_till', 'archived_at'], name='notif_receiver_state_idx'), | ||
| ), | ||
| AddIndexConcurrently( | ||
| model_name='notification', | ||
| index=models.Index(fields=['receiver', 'workspace', 'sender'], name='notif_receiver_sender_idx'), | ||
| ), | ||
| AddIndexConcurrently( | ||
| model_name='notification', | ||
| index=models.Index(fields=['workspace', 'entity_identifier', 'entity_name'], name='notif_entity_lookup_idx'), | ||
| ), | ||
|
Comment on lines
+14
to
+34
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chainVerify composite index field order matches query patterns. The field order in composite indexes is critical for performance due to the leftmost prefix rule. Ensure that the index field orders align with your most common query patterns:
Also verify that these new indexes don't significantly overlap with existing indexes, which could waste storage and slow down writes. Run the following script to check for existing indexes on the notification table: 🏁 Script executed: #!/bin/bash
# Description: Check for existing indexes on notification model that might overlap
# Search for existing indexes in the Notification model Meta class
ast-grep --pattern $'class Notification($$$):
$$$
class Meta:
$$$
indexes = $$$
$$$'
# Also search for any index-related constraints or db_index fields
rg -nP --type=py -C5 'class Notification|db_index\s*=\s*True' -g '**/models/*notification*.py'Length of output: 917 I need to gather more complete information about the Notification model and how it's queried. Let me retrieve the full model definition and search for query patterns. Length of output: 5145 🏁 Script executed: #!/bin/bash
# Search for query patterns on the notification model to verify field order alignment
rg -nP --type=py 'Notification\.objects\.(filter|exclude|get)' -C3 -g '!migrations' | head -100Length of output: 6211 🏁 Script executed: #!/bin/bash
# Check for any existing indexes or constraints on notification fields
rg -nP --type=py 'class.*Notification.*Meta|indexes\s*=|db_index' apps/api/plane/db/models/notification.pyLength of output: 22 🏁 Script executed: #!/bin/bash
# Look for existing index definitions in all migrations related to notification
fd '.*notification.*\.py' apps/api/plane/db/migrations/ | xargs grep -l 'AddIndex\|RemoveIndex' | head -20Length of output: 84 🏁 Script executed: #!/bin/bash
# Check the actual index creation/removal statements in older migrations
rg -nP --type=py 'AddIndex|RemoveIndex' apps/api/plane/db/migrations/ -B2 -A5 | grep -i notification | head -60Length of output: 4912 I need to verify whether these indexes are redundantly defined and check migration dependencies. Length of output: 1222 🏁 Script executed: #!/bin/bash
# Check the complete migration file 0111 to see dependencies
cat apps/api/plane/db/migrations/0111_notification_notif_receiver_status_idx_and_more.pyLength of output: 1523 🏁 Script executed: #!/bin/bash
# Search for earlier migrations that might have added these indexes
fd '010[0-9].*\.py' apps/api/plane/db/migrations/ | sort | xargs grep -l 'notif_receiver\|notif_entity'Length of output: 77 🏁 Script executed: #!/bin/bash
# Check if the model Meta.indexes were recently added or always present
git log --oneline --all -- apps/api/plane/db/models/notification.py 2>/dev/null | head -20 || echo "Git history not available"Length of output: 280 Let me check the git commits to understand the sequence of changes: Length of output: 376 🏁 Script executed: #!/bin/bash
# Check if model.Meta.indexes existed before migration 0111 was created
git show 8999120ff:apps/api/plane/db/models/notification.py 2>/dev/null | grep -A 30 "class Meta:" | head -40 || echo "Cannot retrieve file"Length of output: 1617 🏁 Script executed: #!/bin/bash
# Get more context on queries that use receiver AND workspace together
rg -nP 'Notification\.objects\.(filter|exclude)' -A2 -B1 --type=py -g '!migrations' | grep -A3 'receiver\|workspace'Length of output: 1347 Remove migration 0111 indexes that duplicate model Meta.indexes definitions. The five notification indexes added in migration 0111 are already defined in the model's Meta.indexes (apps/api/plane/db/models/notification.py, lines 39-55). Indexes defined in Meta.indexes are automatically created during migration, so redefining them in migration 0111 will cause duplicate key errors. Either:
Additionally, verify the field order 🤖 Prompt for AI Agents |
||
| AddIndexConcurrently( | ||
| model_name='fileasset', | ||
| index=models.Index(fields=['asset'], name='asset_asset_idx'), | ||
| ), | ||
|
Comment on lines
+35
to
+38
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Undocumented index on fileasset table. The
|
||
| ] | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Migration number discrepancy with PR objectives.
The PR description references migration
0108_*, but this file is numbered0111_*. This suggests either the PR description is outdated or there were additional migrations created between the initial plan and implementation.Good use of
atomic = FalsewithAddIndexConcurrentlyto prevent table locking during index creation.