Skip to content

Commit

Permalink
Clean up existing indices on Upload (aka ReportSession)
Browse files Browse the repository at this point in the history
This in particular cleans up the existing indices that exist in the production DB but not in django:

We drop these indices for the following reasons:
- `upload_index_id_type_number`: This matches the above state migration,
  and it looks like the index does not actually exist in the production DB.

The following indices exist in the production DB, but not in django state/migrations:
- `reports_upload_order_number_idx`:
  We never query by the `order_number` alone, so this index is likely unused.
- `reports_upload_report_id_f6b4ffae`: Queries on `report_id` should already been covered by the
   newly added index on `report_id`+`upload_type`.
- `reports_upload_report_id_upload_type_index_ccnew`:
  This seems to be a manually added variant of the `upload_report_type_idx` index and is thus duplicated.
- `reports_upload_report_id_upload_type_order_number_index`:
  This is the same as the above, except with an additional `order_number`.
  We do use it in queries, but I doubt the index pulls its weight, as the `order_number` changes quite
  frequently so the index is costly to maintain.
  • Loading branch information
Swatinem committed Jan 10, 2025
1 parent c3288a0 commit 610dd21
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 5 deletions.
47 changes: 47 additions & 0 deletions shared/django_apps/reports/migrations/0036_upload_indices_part2.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
# Generated by Django 4.2.16 on 2024-11-28 12:37

from django.contrib.postgres.operations import RemoveIndexConcurrently
from django.db import migrations


class Migration(migrations.Migration):
atomic = False

dependencies = [
("reports", "0035_upload_indices_part1"),
]

operations = [
# We drop these indices for the following reasons:
# - `upload_index_id_type_number`: This matches the above state migration,
# and it looks like the index does not actually exist in the production DB.
RemoveIndexConcurrently(
model_name="reportsession",
name="upload_index_id_type_number",
),
# The following indices exist in the production DB, but not in django state/migrations:
# - `reports_upload_order_number_idx`:
# We never query by the `order_number` alone, so this index is likely unused.
# - `reports_upload_report_id_f6b4ffae`: Queries on `report_id` should already been covered by the
# newly added index on `report_id`+`upload_type`.
# - `reports_upload_report_id_upload_type_index_ccnew`:
# This seems to be a manually added variant of the `upload_report_type_idx` index and is thus duplicated.
# - `reports_upload_report_id_upload_type_order_number_index`:
# This is the same as the above, except with an additional `order_number`.
# We do use it in queries, but I doubt the index pulls its weight, as the `order_number` changes quite
# frequently so the index is costly to maintain.
*(
# Interestingly, we have to run these in individual `RunSQL` statements, otherwise django would create a
# transaction around them, which is not supported for these `DROP INDEX CONCURRENTLY` statements.
migrations.RunSQL(
sql=f"""DROP INDEX CONCURRENTLY IF EXISTS "{idx}";""",
hints={"tables": ["reports_upload"]},
)
for idx in [
"reports_upload_order_number_idx",
"reports_upload_report_id_f6b4ffae",
"reports_upload_report_id_upload_type_index_ccnew",
"reports_upload_report_id_upload_type_order_number_index",
]
),
]
5 changes: 0 additions & 5 deletions shared/django_apps/reports/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -202,11 +202,6 @@ class Meta:
name="upload_report_type_idx",
fields=["report_id", "upload_type"],
),
# TODO(swatinem): remove the index below in a followup migration:
models.Index(
name="upload_index_id_type_number",
fields=["report_id", "upload_type", "order_number"],
),
]

@property
Expand Down

0 comments on commit 610dd21

Please sign in to comment.