From 9b71369be9006af289bca63d35bc83ca1e734394 Mon Sep 17 00:00:00 2001 From: Gusted Date: Sat, 29 Jul 2023 16:59:23 +0200 Subject: [PATCH] [GITEA] Use join for the deleting issue actions query - The action tables can become very large as it's a dumpster for every action that an user does on an repository. - The following query: `DELETE FROM action WHERE comment_id IN (SELECT id FROM comment WHERE issue_id=?)` is not using indexes for `comment_id` and is instead using an full table scan by MariaDB. - Rewriting the query to use an JOIN will allow MariaDB to use the index. - More information: https://codeberg.org/Codeberg-Infrastructure/techstack-support/issues/9 - Backport https://codeberg.org/forgejo/forgejo/pulls/1154 --- models/activities/action.go | 20 +++++++++++++++----- models/activities/action_test.go | 9 +++++++++ models/fixtures/action.yml | 10 ++++++++++ models/fixtures/comment.yml | 9 +++++++++ models/fixtures/issue.yml | 2 +- tests/integration/api_nodeinfo_test.go | 2 +- 6 files changed, 45 insertions(+), 7 deletions(-) diff --git a/models/activities/action.go b/models/activities/action.go index 2e8a9c1de2..f5835598b8 100644 --- a/models/activities/action.go +++ b/models/activities/action.go @@ -687,11 +687,21 @@ func NotifyWatchersActions(acts []*Action) error { // DeleteIssueActions delete all actions related with issueID func DeleteIssueActions(ctx context.Context, repoID, issueID int64) error { // delete actions assigned to this issue - subQuery := builder.Select("`id`"). - From("`comment`"). - Where(builder.Eq{"`issue_id`": issueID}) - if _, err := db.GetEngine(ctx).In("comment_id", subQuery).Delete(&Action{}); err != nil { - return err + + // MySQL doesn't use the indexes on comment_id when using a subquery. + // It does uses the indexes when using an JOIN, however SQLite doesn't + // allow JOINs in DELETE statements and XORM doesn't allow them as well. + // So, an specific raw SQL query for MySQL so the query makes use of indexes. + if setting.Database.Type.IsMySQL() { + if _, err := db.GetEngine(ctx).Exec("DELETE action FROM action JOIN comment ON action.comment_id = comment.id WHERE comment.issue_id = ?", issueID); err != nil { + return err + } + } else { + subQuery := builder.Select("`id`").From("`comment`"). + Where(builder.Eq{"`issue_id`": issueID}) + if _, err := db.GetEngine(ctx).In("comment_id", subQuery).Delete(&Action{}); err != nil { + return err + } } _, err := db.GetEngine(ctx).Table("action").Where("repo_id = ?", repoID). diff --git a/models/activities/action_test.go b/models/activities/action_test.go index 7044bcc004..61602a44cb 100644 --- a/models/activities/action_test.go +++ b/models/activities/action_test.go @@ -242,6 +242,15 @@ func TestGetFeedsCorrupted(t *testing.T) { assert.Equal(t, int64(0), count) } +func TestDeleteIssueActions(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + comment := unittest.AssertExistsAndLoadBean(t, &issue_model.Comment{ID: 8}) + unittest.AssertExistsAndLoadBean(t, &activities_model.Action{ID: 10, CommentID: comment.ID}) + assert.NoError(t, activities_model.DeleteIssueActions(db.DefaultContext, 32, 17)) + unittest.AssertNotExistsBean(t, &activities_model.Action{ID: 10, CommentID: comment.ID}) +} + func TestConsistencyUpdateAction(t *testing.T) { if !setting.Database.Type.IsSQLite3() { t.Skip("Test is only for SQLite database.") diff --git a/models/fixtures/action.yml b/models/fixtures/action.yml index af9ce93ba5..57654eb5b2 100644 --- a/models/fixtures/action.yml +++ b/models/fixtures/action.yml @@ -73,3 +73,13 @@ is_private: false created_unix: 1680454039 content: '4|' # issueId 5 + +- id: 10 + user_id: 15 + op_type: 10 # issue comment + act_user_id: 15 + repo_id: 32 # public + comment_id: 8 + is_private: false + created_unix: 1680454039 + content: '2|meh...' # issueId 5 diff --git a/models/fixtures/comment.yml b/models/fixtures/comment.yml index bd64680c8c..f85b1b7af8 100644 --- a/models/fixtures/comment.yml +++ b/models/fixtures/comment.yml @@ -66,3 +66,12 @@ tree_path: "README.md" created_unix: 946684812 invalidated: true + +- + id: 8 + type: 0 # comment + poster_id: 15 + issue_id: 17 # in repo_id 32 + content: "meh..." + created_unix: 946684812 + updated_unix: 946684812 diff --git a/models/fixtures/issue.yml b/models/fixtures/issue.yml index 174345ff5a..bc7463a11e 100644 --- a/models/fixtures/issue.yml +++ b/models/fixtures/issue.yml @@ -283,7 +283,7 @@ priority: 0 is_closed: false is_pull: false - num_comments: 0 + num_comments: 1 created_unix: 1602935696 updated_unix: 1602935696 is_locked: false diff --git a/tests/integration/api_nodeinfo_test.go b/tests/integration/api_nodeinfo_test.go index f931d4eef3..e9ca4e5813 100644 --- a/tests/integration/api_nodeinfo_test.go +++ b/tests/integration/api_nodeinfo_test.go @@ -35,6 +35,6 @@ func TestNodeinfo(t *testing.T) { assert.Equal(t, "forgejo", nodeinfo.Software.Name) assert.Equal(t, 25, nodeinfo.Usage.Users.Total) assert.Equal(t, 18, nodeinfo.Usage.LocalPosts) - assert.Equal(t, 2, nodeinfo.Usage.LocalComments) + assert.Equal(t, 3, nodeinfo.Usage.LocalComments) }) }