Merge pull request '[v7.0/forgejo] mysql: faster user deletion (hook_task query) for mariadb 10' (#3888) from bp-v7.0/forgejo-4ffda65-bb165fa into v7.0/forgejo

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/3888
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>

models/fixtures/hook_task.yml

@@ -35,3 +35,9 @@
         "X-Head": "42"
       }
     }
+
+-
+  id: 4
+  hook_id: 3
+  uuid: uuid4
+  is_delivered: false

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).

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})).
+	if setting.Database.Type.IsMySQL() {
-		Delete(&webhook.HookTask{}); err != nil {
+		// mariadb:10 does not use the hook_task KEY when using IN.
-		return err
+		// 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,

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,
+	})
 }