From cef84d7abfdd1d098ed752a29c8e02ee60ed8680 Mon Sep 17 00:00:00 2001 From: oliverpool Date: Wed, 22 May 2024 11:46:51 +0200 Subject: [PATCH 1/2] test: check hook_task deletion move test to integration to ensure Sqlite + MySQL testing (cherry picked from commit bb165fadf6cc12822394ee7bc4e0a47c4383ba83) --- models/fixtures/hook_task.yml | 6 ++++++ .../integration/repo_delete_test.go | 20 ++++++++++++++++++- 2 files changed, 25 insertions(+), 1 deletion(-) rename services/repository/delete_test.go => tests/integration/repo_delete_test.go (73%) diff --git a/models/fixtures/hook_task.yml b/models/fixtures/hook_task.yml index d573406b36..fc0e03bca1 100644 --- a/models/fixtures/hook_task.yml +++ b/models/fixtures/hook_task.yml @@ -35,3 +35,9 @@ "X-Head": "42" } } + +- + id: 4 + hook_id: 3 + uuid: uuid4 + is_delivered: false diff --git a/services/repository/delete_test.go b/tests/integration/repo_delete_test.go similarity index 73% rename from services/repository/delete_test.go rename to tests/integration/repo_delete_test.go index 869b8af11d..10e99db444 100644 --- a/services/repository/delete_test.go +++ b/tests/integration/repo_delete_test.go @@ -1,7 +1,7 @@ // Copyright 2017 The Gitea Authors. All rights reserved. // SPDX-License-Identifier: MIT -package repository_test +package integration import ( "testing" @@ -11,6 +11,7 @@ import ( repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unittest" user_model "code.gitea.io/gitea/models/user" + webhook_model "code.gitea.io/gitea/models/webhook" repo_service "code.gitea.io/gitea/services/repository" "github.com/stretchr/testify/assert" @@ -51,5 +52,22 @@ func TestDeleteOwnerRepositoriesDirectly(t *testing.T) { user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 2}) + deletedHookID := unittest.AssertExistsAndLoadBean(t, &webhook_model.Webhook{RepoID: 1}).ID + unittest.AssertExistsAndLoadBean(t, &webhook_model.HookTask{ + HookID: deletedHookID, + }) + + preservedHookID := unittest.AssertExistsAndLoadBean(t, &webhook_model.Webhook{RepoID: 3}).ID + unittest.AssertExistsAndLoadBean(t, &webhook_model.HookTask{ + HookID: preservedHookID, + }) + assert.NoError(t, repo_service.DeleteOwnerRepositoriesDirectly(db.DefaultContext, user)) + + unittest.AssertNotExistsBean(t, &webhook_model.HookTask{ + HookID: deletedHookID, + }) + unittest.AssertExistsAndLoadBean(t, &webhook_model.HookTask{ + HookID: preservedHookID, + }) } From 8f99d82cf234f2b1ab8bc190996721167c7ab743 Mon Sep 17 00:00:00 2001 From: oliverpool Date: Wed, 22 May 2024 12:16:05 +0200 Subject: [PATCH 2/2] mysql: use inner join for hook_task deletion Attempt to fix #3678 (cherry picked from commit 4ffda656e8fe489676e28541e5a01311e36698a9) --- release-notes/8.0.0/perf/3865.md | 1 + services/repository/delete.go | 23 ++++++++++++++++++++--- 2 files changed, 21 insertions(+), 3 deletions(-) create mode 100644 release-notes/8.0.0/perf/3865.md diff --git a/release-notes/8.0.0/perf/3865.md b/release-notes/8.0.0/perf/3865.md new file mode 100644 index 0000000000..88860c7154 --- /dev/null +++ b/release-notes/8.0.0/perf/3865.md @@ -0,0 +1 @@ +Attempt to speed up user deletion when using mariadb 10 (the subquery took advantage of the available index starting with mariadb 11). diff --git a/services/repository/delete.go b/services/repository/delete.go index d5a8849033..92318db0c3 100644 --- a/services/repository/delete.go +++ b/services/repository/delete.go @@ -26,6 +26,7 @@ import ( actions_module "code.gitea.io/gitea/modules/actions" "code.gitea.io/gitea/modules/lfs" "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/storage" "xorm.io/builder" @@ -125,9 +126,25 @@ func DeleteRepositoryDirectly(ctx context.Context, doer *user_model.User, repoID return err } - if _, err := db.GetEngine(ctx).In("hook_id", builder.Select("id").From("webhook").Where(builder.Eq{"webhook.repo_id": repo.ID})). - Delete(&webhook.HookTask{}); err != nil { - return err + if setting.Database.Type.IsMySQL() { + // mariadb:10 does not use the hook_task KEY when using IN. + // https://codeberg.org/forgejo/forgejo/issues/3678 + // + // Version 11 does support it, but is not available in debian yet. + // Version 11.4 LTS is not available yet (stable should be released mid 2024 https://mariadb.org/mariadb/all-releases/) + + // Sqlite does not support the DELETE *** FROM *** syntax + // https://stackoverflow.com/q/24511153/3207406 + + // in the meantime, use a dedicated query for mysql... + if _, err := db.Exec(ctx, "DELETE `hook_task` FROM `hook_task` INNER JOIN `webhook` ON `webhook`.id = `hook_task`.hook_id WHERE `webhook`.repo_id = ?", repo.ID); err != nil { + return err + } + } else { + if _, err := db.GetEngine(ctx).In("hook_id", builder.Select("id").From("webhook").Where(builder.Eq{"webhook.repo_id": repo.ID})). + Delete(&webhook.HookTask{}); err != nil { + return err + } } if err := db.DeleteBeans(ctx,