Skip to content

Commit 7df09e3

Browse files
lunnywxiaoguang
andauthored
Move issue pin to an standalone table for querying performance (#33452)
Noticed a SQL in gitea.com has a bigger load. It seems both `is_pull` and `pin_order` are not indexed columns in the database. ```SQL SELECT `id`, `repo_id`, `index`, `poster_id`, `original_author`, `original_author_id`, `name`, `content`, `content_version`, `milestone_id`, `priority`, `is_closed`, `is_pull`, `num_comments`, `ref`, `pin_order`, `deadline_unix`, `created_unix`, `updated_unix`, `closed_unix`, `is_locked`, `time_estimate` FROM `issue` WHERE (repo_id =?) AND (is_pull = 0) AND (pin_order > 0) ORDER BY pin_order ``` I came across a comment #24406 (comment) from @delvh , which presents a more reasonable approach. Based on this, this PR will migrate all issue and pull request pin data from the `issue` table to the `issue_pin` table. This change benefits larger Gitea instances by improving scalability and performance. --------- Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
1 parent f5a81f9 commit 7df09e3

File tree

13 files changed

+396
-218
lines changed

13 files changed

+396
-218
lines changed

models/fixtures/issue_pin.yml

+6
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
-
2+
id: 1
3+
repo_id: 2
4+
issue_id: 4
5+
is_pull: false
6+
pin_order: 1

models/issues/issue.go

+30-185
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ type Issue struct {
9797
// TODO: RemoveIssueRef: see "repo/issue/branch_selector_field.tmpl"
9898
Ref string
9999

100-
PinOrder int `xorm:"DEFAULT 0"`
100+
PinOrder int `xorm:"-"` // 0 means not loaded, -1 means loaded but not pinned
101101

102102
DeadlineUnix timeutil.TimeStamp `xorm:"INDEX"`
103103

@@ -291,6 +291,23 @@ func (issue *Issue) LoadMilestone(ctx context.Context) (err error) {
291291
return nil
292292
}
293293

294+
func (issue *Issue) LoadPinOrder(ctx context.Context) error {
295+
if issue.PinOrder != 0 {
296+
return nil
297+
}
298+
issuePin, err := GetIssuePin(ctx, issue)
299+
if err != nil && !db.IsErrNotExist(err) {
300+
return err
301+
}
302+
303+
if issuePin != nil {
304+
issue.PinOrder = issuePin.PinOrder
305+
} else {
306+
issue.PinOrder = -1
307+
}
308+
return nil
309+
}
310+
294311
// LoadAttributes loads the attribute of this issue.
295312
func (issue *Issue) LoadAttributes(ctx context.Context) (err error) {
296313
if err = issue.LoadRepo(ctx); err != nil {
@@ -330,6 +347,10 @@ func (issue *Issue) LoadAttributes(ctx context.Context) (err error) {
330347
return err
331348
}
332349

350+
if err = issue.LoadPinOrder(ctx); err != nil {
351+
return err
352+
}
353+
333354
if err = issue.Comments.LoadAttributes(ctx); err != nil {
334355
return err
335356
}
@@ -342,6 +363,14 @@ func (issue *Issue) LoadAttributes(ctx context.Context) (err error) {
342363
return issue.loadReactions(ctx)
343364
}
344365

366+
// IsPinned returns if a Issue is pinned
367+
func (issue *Issue) IsPinned() bool {
368+
if issue.PinOrder == 0 {
369+
setting.PanicInDevOrTesting("issue's pinorder has not been loaded")
370+
}
371+
return issue.PinOrder > 0
372+
}
373+
345374
func (issue *Issue) ResetAttributesLoaded() {
346375
issue.isLabelsLoaded = false
347376
issue.isMilestoneLoaded = false
@@ -720,190 +749,6 @@ func (issue *Issue) HasOriginalAuthor() bool {
720749
return issue.OriginalAuthor != "" && issue.OriginalAuthorID != 0
721750
}
722751

723-
var ErrIssueMaxPinReached = util.NewInvalidArgumentErrorf("the max number of pinned issues has been readched")
724-
725-
// IsPinned returns if a Issue is pinned
726-
func (issue *Issue) IsPinned() bool {
727-
return issue.PinOrder != 0
728-
}
729-
730-
// Pin pins a Issue
731-
func (issue *Issue) Pin(ctx context.Context, user *user_model.User) error {
732-
// If the Issue is already pinned, we don't need to pin it twice
733-
if issue.IsPinned() {
734-
return nil
735-
}
736-
737-
var maxPin int
738-
_, err := db.GetEngine(ctx).SQL("SELECT MAX(pin_order) FROM issue WHERE repo_id = ? AND is_pull = ?", issue.RepoID, issue.IsPull).Get(&maxPin)
739-
if err != nil {
740-
return err
741-
}
742-
743-
// Check if the maximum allowed Pins reached
744-
if maxPin >= setting.Repository.Issue.MaxPinned {
745-
return ErrIssueMaxPinReached
746-
}
747-
748-
_, err = db.GetEngine(ctx).Table("issue").
749-
Where("id = ?", issue.ID).
750-
Update(map[string]any{
751-
"pin_order": maxPin + 1,
752-
})
753-
if err != nil {
754-
return err
755-
}
756-
757-
// Add the pin event to the history
758-
opts := &CreateCommentOptions{
759-
Type: CommentTypePin,
760-
Doer: user,
761-
Repo: issue.Repo,
762-
Issue: issue,
763-
}
764-
if _, err = CreateComment(ctx, opts); err != nil {
765-
return err
766-
}
767-
768-
return nil
769-
}
770-
771-
// UnpinIssue unpins a Issue
772-
func (issue *Issue) Unpin(ctx context.Context, user *user_model.User) error {
773-
// If the Issue is not pinned, we don't need to unpin it
774-
if !issue.IsPinned() {
775-
return nil
776-
}
777-
778-
// This sets the Pin for all Issues that come after the unpined Issue to the correct value
779-
_, err := db.GetEngine(ctx).Exec("UPDATE issue SET pin_order = pin_order - 1 WHERE repo_id = ? AND is_pull = ? AND pin_order > ?", issue.RepoID, issue.IsPull, issue.PinOrder)
780-
if err != nil {
781-
return err
782-
}
783-
784-
_, err = db.GetEngine(ctx).Table("issue").
785-
Where("id = ?", issue.ID).
786-
Update(map[string]any{
787-
"pin_order": 0,
788-
})
789-
if err != nil {
790-
return err
791-
}
792-
793-
// Add the unpin event to the history
794-
opts := &CreateCommentOptions{
795-
Type: CommentTypeUnpin,
796-
Doer: user,
797-
Repo: issue.Repo,
798-
Issue: issue,
799-
}
800-
if _, err = CreateComment(ctx, opts); err != nil {
801-
return err
802-
}
803-
804-
return nil
805-
}
806-
807-
// PinOrUnpin pins or unpins a Issue
808-
func (issue *Issue) PinOrUnpin(ctx context.Context, user *user_model.User) error {
809-
if !issue.IsPinned() {
810-
return issue.Pin(ctx, user)
811-
}
812-
813-
return issue.Unpin(ctx, user)
814-
}
815-
816-
// MovePin moves a Pinned Issue to a new Position
817-
func (issue *Issue) MovePin(ctx context.Context, newPosition int) error {
818-
// If the Issue is not pinned, we can't move them
819-
if !issue.IsPinned() {
820-
return nil
821-
}
822-
823-
if newPosition < 1 {
824-
return fmt.Errorf("The Position can't be lower than 1")
825-
}
826-
827-
dbctx, committer, err := db.TxContext(ctx)
828-
if err != nil {
829-
return err
830-
}
831-
defer committer.Close()
832-
833-
var maxPin int
834-
_, err = db.GetEngine(dbctx).SQL("SELECT MAX(pin_order) FROM issue WHERE repo_id = ? AND is_pull = ?", issue.RepoID, issue.IsPull).Get(&maxPin)
835-
if err != nil {
836-
return err
837-
}
838-
839-
// If the new Position bigger than the current Maximum, set it to the Maximum
840-
if newPosition > maxPin+1 {
841-
newPosition = maxPin + 1
842-
}
843-
844-
// Lower the Position of all Pinned Issue that came after the current Position
845-
_, err = db.GetEngine(dbctx).Exec("UPDATE issue SET pin_order = pin_order - 1 WHERE repo_id = ? AND is_pull = ? AND pin_order > ?", issue.RepoID, issue.IsPull, issue.PinOrder)
846-
if err != nil {
847-
return err
848-
}
849-
850-
// Higher the Position of all Pinned Issues that comes after the new Position
851-
_, err = db.GetEngine(dbctx).Exec("UPDATE issue SET pin_order = pin_order + 1 WHERE repo_id = ? AND is_pull = ? AND pin_order >= ?", issue.RepoID, issue.IsPull, newPosition)
852-
if err != nil {
853-
return err
854-
}
855-
856-
_, err = db.GetEngine(dbctx).Table("issue").
857-
Where("id = ?", issue.ID).
858-
Update(map[string]any{
859-
"pin_order": newPosition,
860-
})
861-
if err != nil {
862-
return err
863-
}
864-
865-
return committer.Commit()
866-
}
867-
868-
// GetPinnedIssues returns the pinned Issues for the given Repo and type
869-
func GetPinnedIssues(ctx context.Context, repoID int64, isPull bool) (IssueList, error) {
870-
issues := make(IssueList, 0)
871-
872-
err := db.GetEngine(ctx).
873-
Table("issue").
874-
Where("repo_id = ?", repoID).
875-
And("is_pull = ?", isPull).
876-
And("pin_order > 0").
877-
OrderBy("pin_order").
878-
Find(&issues)
879-
if err != nil {
880-
return nil, err
881-
}
882-
883-
err = issues.LoadAttributes(ctx)
884-
if err != nil {
885-
return nil, err
886-
}
887-
888-
return issues, nil
889-
}
890-
891-
// IsNewPinAllowed returns if a new Issue or Pull request can be pinned
892-
func IsNewPinAllowed(ctx context.Context, repoID int64, isPull bool) (bool, error) {
893-
var maxPin int
894-
_, err := db.GetEngine(ctx).SQL("SELECT COUNT(pin_order) FROM issue WHERE repo_id = ? AND is_pull = ? AND pin_order > 0", repoID, isPull).Get(&maxPin)
895-
if err != nil {
896-
return false, err
897-
}
898-
899-
return maxPin < setting.Repository.Issue.MaxPinned, nil
900-
}
901-
902-
// IsErrIssueMaxPinReached returns if the error is, that the User can't pin more Issues
903-
func IsErrIssueMaxPinReached(err error) bool {
904-
return err == ErrIssueMaxPinReached
905-
}
906-
907752
// InsertIssues insert issues to database
908753
func InsertIssues(ctx context.Context, issues ...*Issue) error {
909754
ctx, committer, err := db.TxContext(ctx)

models/issues/issue_list.go

+33
Original file line numberDiff line numberDiff line change
@@ -506,6 +506,39 @@ func (issues IssueList) loadTotalTrackedTimes(ctx context.Context) (err error) {
506506
return nil
507507
}
508508

509+
func (issues IssueList) LoadPinOrder(ctx context.Context) error {
510+
if len(issues) == 0 {
511+
return nil
512+
}
513+
514+
issueIDs := container.FilterSlice(issues, func(issue *Issue) (int64, bool) {
515+
return issue.ID, issue.PinOrder == 0
516+
})
517+
if len(issueIDs) == 0 {
518+
return nil
519+
}
520+
issuePins, err := GetIssuePinsByIssueIDs(ctx, issueIDs)
521+
if err != nil {
522+
return err
523+
}
524+
525+
for _, issue := range issues {
526+
if issue.PinOrder != 0 {
527+
continue
528+
}
529+
for _, pin := range issuePins {
530+
if pin.IssueID == issue.ID {
531+
issue.PinOrder = pin.PinOrder
532+
break
533+
}
534+
}
535+
if issue.PinOrder == 0 {
536+
issue.PinOrder = -1
537+
}
538+
}
539+
return nil
540+
}
541+
509542
// loadAttributes loads all attributes, expect for attachments and comments
510543
func (issues IssueList) LoadAttributes(ctx context.Context) error {
511544
if _, err := issues.LoadRepositories(ctx); err != nil {

0 commit comments

Comments
 (0)