Bypass MariaDB performance bug of the "IN" sub-query, fix incorrect IssueIndex (#26279) (#26368)

Backport #26279 by @wxiaoguang

Close #26277
Fix #26285

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>

models/activities/action.go

@@ -685,18 +685,34 @@ func NotifyWatchersActions(acts []*Action) error {
 }
 
 // DeleteIssueActions delete all actions related with issueID
-func DeleteIssueActions(ctx context.Context, repoID, issueID int64) error {
+func DeleteIssueActions(ctx context.Context, repoID, issueID, issueIndex 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
+	e := db.GetEngine(ctx)
+
+	// MariaDB has a performance bug: https://jira.mariadb.org/browse/MDEV-16289
+	// so here it uses "DELETE ... WHERE IN" with pre-queried IDs.
+	var lastCommentID int64
+	commentIDs := make([]int64, 0, db.DefaultMaxInSize)
+	for {
+		commentIDs = commentIDs[:0]
+		err := e.Select("`id`").Table(&issues_model.Comment{}).
+			Where(builder.Eq{"issue_id": issueID}).And("`id` > ?", lastCommentID).
+			OrderBy("`id`").Limit(db.DefaultMaxInSize).
+			Find(&commentIDs)
+		if err != nil {
+			return err
+		} else if len(commentIDs) == 0 {
+			break
+		} else if _, err = db.GetEngine(ctx).In("comment_id", commentIDs).Delete(&Action{}); err != nil {
+			return err
+		} else {
+			lastCommentID = commentIDs[len(commentIDs)-1]
+		}
 	}
 
-	_, err := db.GetEngine(ctx).Table("action").Where("repo_id = ?", repoID).
+	_, err := e.Where("repo_id = ?", repoID).
 		In("op_type", ActionCreateIssue, ActionCreatePullRequest).
-		Where("content LIKE ?", strconv.FormatInt(issueID, 10)+"|%").
+		Where("content LIKE ?", strconv.FormatInt(issueIndex, 10)+"|%"). // "IssueIndex|content..."
 		Delete(&Action{})
 	return err
 }

models/activities/action_test.go

@@ -4,6 +4,7 @@
 package activities_test
 
 import (
+	"fmt"
 	"path"
 	"testing"
 
@@ -284,3 +285,36 @@ func TestConsistencyUpdateAction(t *testing.T) {
 	assert.NoError(t, db.GetEngine(db.DefaultContext).Where("id = ?", id).Find(&actions))
 	unittest.CheckConsistencyFor(t, &activities_model.Action{})
 }
+
+func TestDeleteIssueActions(t *testing.T) {
+	assert.NoError(t, unittest.PrepareTestDatabase())
+
+	// load an issue
+	issue := unittest.AssertExistsAndLoadBean(t, &issue_model.Issue{ID: 4})
+	assert.NotEqualValues(t, issue.ID, issue.Index) // it needs to use different ID/Index to test the DeleteIssueActions to delete some actions by IssueIndex
+
+	// insert a comment
+	err := db.Insert(db.DefaultContext, &issue_model.Comment{Type: issue_model.CommentTypeComment, IssueID: issue.ID})
+	assert.NoError(t, err)
+	comment := unittest.AssertExistsAndLoadBean(t, &issue_model.Comment{Type: issue_model.CommentTypeComment, IssueID: issue.ID})
+
+	// truncate action table and insert some actions
+	err = db.TruncateBeans(db.DefaultContext, &activities_model.Action{})
+	assert.NoError(t, err)
+	err = db.Insert(db.DefaultContext, &activities_model.Action{
+		OpType:    activities_model.ActionCommentIssue,
+		CommentID: comment.ID,
+	})
+	assert.NoError(t, err)
+	err = db.Insert(db.DefaultContext, &activities_model.Action{
+		OpType:  activities_model.ActionCreateIssue,
+		RepoID:  issue.RepoID,
+		Content: fmt.Sprintf("%d|content...", issue.Index),
+	})
+	assert.NoError(t, err)
+
+	// assert that the actions exist, then delete them
+	unittest.AssertCount(t, &activities_model.Action{}, 2)
+	assert.NoError(t, activities_model.DeleteIssueActions(db.DefaultContext, issue.RepoID, issue.ID, issue.Index))
+	unittest.AssertCount(t, &activities_model.Action{}, 0)
+}

services/issue/issue.go

@@ -242,7 +242,7 @@ func deleteIssue(ctx context.Context, issue *issues_model.Issue) error {
 			issue.MilestoneID, err)
 	}
 
-	if err := activities_model.DeleteIssueActions(ctx, issue.RepoID, issue.ID); err != nil {
+	if err := activities_model.DeleteIssueActions(ctx, issue.RepoID, issue.ID, issue.Index); err != nil {
 		return err
 	}