mirror of
https://github.com/bugsink/bugsink.git
synced 2026-02-22 06:18:41 -06:00
Project-deletion: slight optimization
Removes the following 2 redundant queries from the deletion process:
```
SELECT "tags_tagkey"."id" FROM "tags_tagkey" WHERE "tags_tagkey"."project_id" IN (1) ORDER BY "tags_tagkey"."project_id" ASC, "tags_tagkey"."id" ASC LIMIT 498
UPDATE "projects_project" SET "stored_event_count" = ("projects_project"."stored_event_count" - 1) WHERE "projects_project"."id" = 1
```
This commit is contained in:
@@ -231,7 +231,7 @@ def prune_orphans(model, d_ids_to_check):
|
||||
# vacuuming once in a while" is a much better fit for that.
|
||||
|
||||
|
||||
def do_pre_delete(project_id, model, pks_to_delete):
|
||||
def do_pre_delete(project_id, model, pks_to_delete, is_for_project):
|
||||
"More model-specific cleanup, if needed; only for Event model at the moment."
|
||||
|
||||
if model.__name__ != "Event":
|
||||
@@ -246,11 +246,15 @@ def do_pre_delete(project_id, model, pks_to_delete):
|
||||
.values_list("id", "storage_backend")
|
||||
)
|
||||
|
||||
if is_for_project:
|
||||
# no need to update the stored_event_count for the project, because the project is being deleted
|
||||
return
|
||||
|
||||
# note: don't bother to do the same thing for Issue.stored_event_count, since we're in the process of deleting Issue
|
||||
Project.objects.filter(id=project_id).update(stored_event_count=F('stored_event_count') - len(pks_to_delete))
|
||||
|
||||
|
||||
def delete_deps_with_budget(project_id, referring_model, fk_name, referred_ids, budget, dep_graph):
|
||||
def delete_deps_with_budget(project_id, referring_model, fk_name, referred_ids, budget, dep_graph, is_for_project):
|
||||
r"""
|
||||
Deletes all objects of type referring_model that refer to any of the referred_ids via fk_name.
|
||||
Returns the number of deleted objects.
|
||||
@@ -291,6 +295,7 @@ def delete_deps_with_budget(project_id, referring_model, fk_name, referred_ids,
|
||||
[d["pk"] for d in relevant_ids],
|
||||
budget - num_deleted,
|
||||
dep_graph,
|
||||
is_for_project,
|
||||
)
|
||||
|
||||
if num_deleted >= budget:
|
||||
@@ -300,12 +305,16 @@ def delete_deps_with_budget(project_id, referring_model, fk_name, referred_ids,
|
||||
# left. We can now delete the referring objects themselves (limited by budget).
|
||||
relevant_ids_after_rec = relevant_ids[:budget - num_deleted]
|
||||
|
||||
do_pre_delete(project_id, referring_model, [d['pk'] for d in relevant_ids_after_rec])
|
||||
do_pre_delete(project_id, referring_model, [d['pk'] for d in relevant_ids_after_rec], is_for_project)
|
||||
|
||||
my_num_deleted, del_d = referring_model.objects.filter(pk__in=[d['pk'] for d in relevant_ids_after_rec]).delete()
|
||||
num_deleted += my_num_deleted
|
||||
assert set(del_d.keys()) == {referring_model._meta.label} # assert no-cascading (we do that ourselves)
|
||||
|
||||
if is_for_project:
|
||||
# short-circuit: project-deletion implies "no orphans" because the project kill everything with it.
|
||||
return num_deleted
|
||||
|
||||
# Note that prune_orphans doesn't respect the budget. Reason: it's not easy to do, b/c the order is reversed (we
|
||||
# would need to predict somehow at the previous step how much budget to leave unused) and we don't care _that much_
|
||||
# about a precise budget "at the edges of our algo", as long as we don't have a "single huge blocking thing".
|
||||
|
||||
@@ -64,6 +64,7 @@ def delete_issue_deps(project_id, issue_id):
|
||||
[issue_id],
|
||||
budget - num_deleted,
|
||||
dep_graph,
|
||||
is_for_project=False,
|
||||
)
|
||||
|
||||
num_deleted += this_num_deleted
|
||||
|
||||
@@ -123,6 +123,7 @@ def delete_project_deps(project_id):
|
||||
[project_id],
|
||||
budget - num_deleted,
|
||||
dep_graph,
|
||||
is_for_project=True,
|
||||
)
|
||||
|
||||
num_deleted += this_num_deleted
|
||||
|
||||
@@ -66,7 +66,7 @@ class ProjectDeletionTestCase(TransactionTestCase):
|
||||
# correct for bugsink/transaction.py's select_for_update for non-sqlite databases
|
||||
correct_for_select_for_update = 1 if 'sqlite' not in settings.DATABASES['default']['ENGINE'] else 0
|
||||
|
||||
with self.assertNumQueries(29 + correct_for_select_for_update):
|
||||
with self.assertNumQueries(27 + correct_for_select_for_update):
|
||||
self.project.delete_deferred()
|
||||
|
||||
# tests run w/ TASK_ALWAYS_EAGER, so in the below we can just check the database directly
|
||||
|
||||
Reference in New Issue
Block a user