Skip to content

Commit bf48c5a

Browse files
wxiaoguangwxiaoguang
committed
fix res index
1 parent 677af6a commit bf48c5a

File tree

6 files changed

+89
-86
lines changed

6 files changed

+89
-86
lines changed

‎models/db/index.go

Lines changed: 67 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -8,45 +8,15 @@ import (
88
"context"
99
"errors"
1010
"fmt"
11-
12-
"code.gitea.io/gitea/modules/setting"
1311
)
1412

1513
// ResourceIndex represents a resource index which could be used as issue/release and others
16-
// We can create different tables i.e. issue_index, release_index and etc.
14+
// We can create different tables i.e. issue_index, release_index, etc.
1715
type ResourceIndex struct {
1816
GroupID int64 `xorm:"pk"`
1917
MaxIndex int64 `xorm:"index"`
2018
}
2119

22-
// UpsertResourceIndex the function will not return until it acquires the lock or receives an error.
23-
func UpsertResourceIndex(ctx context.Context, tableName string, groupID int64) (err error) {
24-
// An atomic UPSERT operation (INSERT/UPDATE) is the only operation
25-
// that ensures that the key is actually locked.
26-
switch {
27-
case setting.Database.UseSQLite3 || setting.Database.UsePostgreSQL:
28-
_, err = Exec(ctx, fmt.Sprintf("INSERT INTO %s (group_id, max_index) "+
29-
"VALUES (?,1) ON CONFLICT (group_id) DO UPDATE SET max_index = %s.max_index+1",
30-
tableName, tableName), groupID)
31-
case setting.Database.UseMySQL:
32-
_, err = Exec(ctx, fmt.Sprintf("INSERT INTO %s (group_id, max_index) "+
33-
"VALUES (?,1) ON DUPLICATE KEY UPDATE max_index = max_index+1", tableName),
34-
groupID)
35-
case setting.Database.UseMSSQL:
36-
// https://weblogs.sqlteam.com/dang/2009/01/31/upsert-race-condition-with-merge/
37-
_, err = Exec(ctx, fmt.Sprintf("MERGE %s WITH (HOLDLOCK) as target "+
38-
"USING (SELECT ? AS group_id) AS src "+
39-
"ON src.group_id = target.group_id "+
40-
"WHEN MATCHED THEN UPDATE SET target.max_index = target.max_index+1 "+
41-
"WHEN NOT MATCHED THEN INSERT (group_id, max_index) "+
42-
"VALUES (src.group_id, 1);", tableName),
43-
groupID)
44-
default:
45-
return fmt.Errorf("database type not supported")
46-
}
47-
return err
48-
}
49-
5020
var (
5121
// ErrResouceOutdated represents an error when request resource outdated
5222
ErrResouceOutdated = errors.New("resource outdated")
@@ -59,53 +29,89 @@ const (
5929
MaxDupIndexAttempts = 3
6030
)
6131

62-
// GetNextResourceIndex retried 3 times to generate a resource index
63-
func GetNextResourceIndex(tableName string, groupID int64) (int64, error) {
64-
for i := 0; i < MaxDupIndexAttempts; i++ {
65-
idx, err := getNextResourceIndex(tableName, groupID)
66-
if err == ErrResouceOutdated {
67-
continue
32+
// SyncMaxResourceIndex sync the max index with the resource
33+
func SyncMaxResourceIndex(ctx context.Context, tableName string, groupID, maxIndex int64) (err error) {
34+
e := GetEngine(ctx)
35+
36+
// try to update the max_index and acquire the write-lock for the record
37+
res, err := e.Exec(fmt.Sprintf("UPDATE %s SET max_index=? WHERE group_id=? AND max_index<?", tableName), maxIndex, groupID, maxIndex)
38+
if err != nil {
39+
return err
40+
}
41+
affected, err := res.RowsAffected()
42+
if err != nil {
43+
return err
44+
}
45+
if affected == 0 {
46+
// if nothing is updated, the record might not exist, try to insert and update it
47+
_, errIns := e.Exec(fmt.Sprintf("INSERT INTO %s (group_id, max_index) VALUES (?, 0)", tableName), groupID)
48+
_, err = e.Exec(fmt.Sprintf("UPDATE %s SET max_index=? WHERE group_id=? AND max_index<?", tableName), maxIndex, groupID, maxIndex)
49+
if err != nil {
50+
return err
6851
}
52+
var savedIdx int64
53+
has, err := e.SQL(fmt.Sprintf("SELECT max_index FROM %s WHERE group_id=?", tableName), groupID).Get(&savedIdx)
6954
if err != nil {
70-
return 0, err
55+
return err
56+
}
57+
// if the record still doesn't exist, there must be some errors (insert error)
58+
if !has {
59+
if errIns == nil {
60+
return errors.New("impossible error when SyncMaxResourceIndex, insert and update both succeeded but no record is saved")
61+
}
62+
return errIns
7163
}
72-
return idx, nil
7364
}
74-
return 0, ErrGetResourceIndexFailed
65+
return nil
7566
}
7667

77-
// DeleteResouceIndex delete resource index
78-
func DeleteResouceIndex(ctx context.Context, tableName string, groupID int64) error {
79-
_, err := Exec(ctx, fmt.Sprintf("DELETE FROM %s WHERE group_id=?", tableName), groupID)
80-
return err
81-
}
68+
// GetNextResourceIndex generates a resource index, it must run in the same transaction where the resource is created
69+
func GetNextResourceIndex(ctx context.Context, tableName string, groupID int64) (int64, error) {
70+
e := GetEngine(ctx)
8271

83-
// getNextResourceIndex return the next index
84-
func getNextResourceIndex(tableName string, groupID int64) (int64, error) {
85-
ctx, commiter, err := TxContext()
72+
// try to update the max_index to next value, and acquire the write-lock for the record
73+
res, err := e.Exec(fmt.Sprintf("UPDATE %s SET max_index=max_index+1 WHERE group_id=?", tableName), groupID)
8674
if err != nil {
8775
return 0, err
8876
}
89-
defer commiter.Close()
90-
var preIdx int64
91-
if _, err := GetEngine(ctx).SQL(fmt.Sprintf("SELECT max_index FROM %s WHERE group_id = ?", tableName), groupID).Get(&preIdx); err != nil {
77+
affected, err := res.RowsAffected()
78+
if err != nil {
9279
return 0, err
9380
}
94-
95-
if err := UpsertResourceIndex(ctx, tableName, groupID); err != nil {
96-
return 0, err
81+
if affected == 0 {
82+
// this slow path is only for the first time of creating a resource index
83+
_, errIns := e.Exec(fmt.Sprintf("INSERT INTO %s (group_id, max_index) VALUES (?, 0)", tableName), groupID)
84+
res, err = e.Exec(fmt.Sprintf("UPDATE %s SET max_index=max_index+1 WHERE group_id=?", tableName), groupID)
85+
if err != nil {
86+
return 0, err
87+
}
88+
affected, err = res.RowsAffected()
89+
if err != nil {
90+
return 0, err
91+
}
92+
// if the update still can not update any records, the record must not exist and there must be some errors (insert error)
93+
if affected == 0 {
94+
if errIns == nil {
95+
return 0, errors.New("impossible error when GetNextResourceIndex, insert and update both succeeded but no record is updated")
96+
}
97+
return 0, errIns
98+
}
9799
}
98100

99-
var curIdx int64
100-
has, err := GetEngine(ctx).SQL(fmt.Sprintf("SELECT max_index FROM %s WHERE group_id = ? AND max_index=?", tableName), groupID, preIdx+1).Get(&curIdx)
101+
// now, the new index is in database (protected by the transaction and write-lock)
102+
var newIdx int64
103+
has, err := e.SQL(fmt.Sprintf("SELECT max_index FROM %s WHERE group_id=?", tableName), groupID).Get(&newIdx)
101104
if err != nil {
102105
return 0, err
103106
}
104107
if !has {
105-
return 0, ErrResouceOutdated
106-
}
107-
if err := commiter.Commit(); err != nil {
108-
return 0, err
108+
return 0, errors.New("impossible error when GetNextResourceIndex, upsert succeeded but no record can be selected")
109109
}
110-
return curIdx, nil
110+
return newIdx, nil
111+
}
112+
113+
// DeleteResourceIndex delete resource index
114+
func DeleteResourceIndex(ctx context.Context, tableName string, groupID int64) error {
115+
_, err := Exec(ctx, fmt.Sprintf("DELETE FROM %s WHERE group_id=?", tableName), groupID)
116+
return err
111117
}

‎models/issues/issue.go

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1064,19 +1064,19 @@ func NewIssueWithIndex(ctx context.Context, doer *user_model.User, opts NewIssue
10641064

10651065
// NewIssue creates new issue with labels for repository.
10661066
func NewIssue(repo *repo_model.Repository, issue *Issue, labelIDs []int64, uuids []string) (err error) {
1067-
idx, err := db.GetNextResourceIndex("issue_index", repo.ID)
1068-
if err != nil {
1069-
return fmt.Errorf("generate issue index failed: %v", err)
1070-
}
1071-
1072-
issue.Index = idx
1073-
10741067
ctx, committer, err := db.TxContext()
10751068
if err != nil {
10761069
return err
10771070
}
10781071
defer committer.Close()
10791072

1073+
idx, err := db.GetNextResourceIndex(ctx, "issue_index", repo.ID)
1074+
if err != nil {
1075+
return fmt.Errorf("generate issue index failed: %w", err)
1076+
}
1077+
1078+
issue.Index = idx
1079+
10801080
if err = NewIssueWithIndex(ctx, issue.Poster, NewIssueOptions{
10811081
Repo: repo,
10821082
Issue: issue,

‎models/issues/issue_index.go

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,16 +15,12 @@ func RecalculateIssueIndexForRepo(repoID int64) error {
1515
}
1616
defer committer.Close()
1717

18-
if err := db.UpsertResourceIndex(ctx, "issue_index", repoID); err != nil {
19-
return err
20-
}
21-
2218
var max int64
23-
if _, err := db.GetEngine(ctx).Select(" MAX(`index`)").Table("issue").Where("repo_id=?", repoID).Get(&max); err != nil {
19+
if _, err = db.GetEngine(ctx).Select(" MAX(`index`)").Table("issue").Where("repo_id=?", repoID).Get(&max); err != nil {
2420
return err
2521
}
2622

27-
if _, err := db.GetEngine(ctx).Exec("UPDATE `issue_index` SET max_index=? WHERE group_id=?", max, repoID); err != nil {
23+
if err = db.SyncMaxResourceIndex(ctx, "issue_index", repoID, max); err != nil {
2824
return err
2925
}
3026

‎models/issues/issue_xref_test.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,11 @@ func testCreateIssue(t *testing.T, repo, doer int64, title, content string, ispu
131131
r := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: repo})
132132
d := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: doer})
133133

134-
idx, err := db.GetNextResourceIndex("issue_index", r.ID)
134+
ctx, committer, err := db.TxContext()
135+
assert.NoError(t, err)
136+
defer committer.Close()
137+
138+
idx, err := db.GetNextResourceIndex(ctx, "issue_index", r.ID)
135139
assert.NoError(t, err)
136140
i := &issues_model.Issue{
137141
RepoID: r.ID,
@@ -143,9 +147,6 @@ func testCreateIssue(t *testing.T, repo, doer int64, title, content string, ispu
143147
Index: idx,
144148
}
145149

146-
ctx, committer, err := db.TxContext()
147-
assert.NoError(t, err)
148-
defer committer.Close()
149150
err = issues_model.NewIssueWithIndex(ctx, d, issues_model.NewIssueOptions{
150151
Repo: r,
151152
Issue: i,

‎models/issues/pull.go

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -490,20 +490,20 @@ func (pr *PullRequest) SetMerged(ctx context.Context) (bool, error) {
490490

491491
// NewPullRequest creates new pull request with labels for repository.
492492
func NewPullRequest(outerCtx context.Context, repo *repo_model.Repository, issue *Issue, labelIDs []int64, uuids []string, pr *PullRequest) (err error) {
493-
idx, err := db.GetNextResourceIndex("issue_index", repo.ID)
494-
if err != nil {
495-
return fmt.Errorf("generate pull request index failed: %v", err)
496-
}
497-
498-
issue.Index = idx
499-
500493
ctx, committer, err := db.TxContext()
501494
if err != nil {
502495
return err
503496
}
504497
defer committer.Close()
505498
ctx.WithContext(outerCtx)
506499

500+
idx, err := db.GetNextResourceIndex(ctx, "issue_index", repo.ID)
501+
if err != nil {
502+
return fmt.Errorf("generate pull request index failed: %v", err)
503+
}
504+
505+
issue.Index = idx
506+
507507
if err = NewIssueWithIndex(ctx, issue.Poster, NewIssueOptions{
508508
Repo: repo,
509509
Issue: issue,

‎models/repo.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ func DeleteRepository(doer *user_model.User, uid, repoID int64) error {
163163
}
164164

165165
// Delete issue index
166-
if err := db.DeleteResouceIndex(ctx, "issue_index", repoID); err != nil {
166+
if err := db.DeleteResourceIndex(ctx, "issue_index", repoID); err != nil {
167167
return err
168168
}
169169

0 commit comments

Comments
 (0)