From a9fdf1646beeb11ce2c208d846be294846fdbd01 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 1 Dec 2022 18:34:59 +0800 Subject: [PATCH 1/2] Fix generate index failure possibility on postgres --- models/db/index.go | 20 ++++++++++++++++++++ models/git/commit_status.go | 18 ++++++++++++++++++ tests/integration/repo_commits_test.go | 4 ++-- 3 files changed, 40 insertions(+), 2 deletions(-) diff --git a/models/db/index.go b/models/db/index.go index ca664c9cf125a..191edc88e1db1 100644 --- a/models/db/index.go +++ b/models/db/index.go @@ -7,6 +7,9 @@ import ( "context" "errors" "fmt" + "strconv" + + "code.gitea.io/gitea/modules/setting" ) // ResourceIndex represents a resource index which could be used as issue/release and others @@ -55,8 +58,25 @@ func SyncMaxResourceIndex(ctx context.Context, tableName string, groupID, maxInd return nil } +func postGresGetNextResourceIndex(ctx context.Context, tableName string, groupID int64) (int64, error) { + res, err := GetEngine(ctx).Query(fmt.Sprintf("INSERT INTO %s (group_id, max_index) "+ + "VALUES (?,1) ON CONFLICT (group_id) DO UPDATE SET max_index = %s.max_index+1 RETURNING max_index", + tableName, tableName), groupID) + if err != nil { + return 0, err + } + if len(res) == 0 { + return 0, ErrGetResourceIndexFailed + } + return strconv.ParseInt(string(res[0]["max_index"]), 10, 64) +} + // GetNextResourceIndex generates a resource index, it must run in the same transaction where the resource is created func GetNextResourceIndex(ctx context.Context, tableName string, groupID int64) (int64, error) { + if setting.Database.UsePostgreSQL { + return postGresGetNextResourceIndex(ctx, tableName, groupID) + } + e := GetEngine(ctx) // try to update the max_index to next value, and acquire the write-lock for the record diff --git a/models/git/commit_status.go b/models/git/commit_status.go index 6353bb2fa9f97..732b53563f4b2 100644 --- a/models/git/commit_status.go +++ b/models/git/commit_status.go @@ -9,6 +9,7 @@ import ( "errors" "fmt" "net/url" + "strconv" "strings" "time" @@ -49,8 +50,25 @@ func init() { db.RegisterModel(new(CommitStatusIndex)) } +func postGresGetCommitStatusIndex(ctx context.Context, repoID int64, sha string) (int64, error) { + res, err := db.GetEngine(ctx).Query("INSERT INTO `commit_status_index` (repo_id, sha, max_index) "+ + "VALUES (?,?,1) ON CONFLICT (repo_id, sha) DO UPDATE SET max_index = `commit_status_index`.max_index+1 RETURNING max_index", + repoID, sha) + if err != nil { + return 0, err + } + if len(res) == 0 { + return 0, db.ErrGetResourceIndexFailed + } + return strconv.ParseInt(string(res[0]["max_index"]), 10, 64) +} + // GetNextCommitStatusIndex retried 3 times to generate a resource index func GetNextCommitStatusIndex(ctx context.Context, repoID int64, sha string) (int64, error) { + if setting.Database.UsePostgreSQL { + return postGresGetCommitStatusIndex(ctx, repoID, sha) + } + e := db.GetEngine(ctx) // try to update the max_index to next value, and acquire the write-lock for the record diff --git a/tests/integration/repo_commits_test.go b/tests/integration/repo_commits_test.go index 3840c508dcebc..0d794cec757b8 100644 --- a/tests/integration/repo_commits_test.go +++ b/tests/integration/repo_commits_test.go @@ -135,8 +135,8 @@ func TestRepoCommitsStatusParallel(t *testing.T) { var wg sync.WaitGroup for i := 0; i < 10; i++ { wg.Add(1) - go func(t *testing.T, i int) { - t.Run(fmt.Sprintf("ParallelCreateStatus_%d", i), func(t *testing.T) { + go func(parentT *testing.T, i int) { + parentT.Run(fmt.Sprintf("ParallelCreateStatus_%d", i), func(t *testing.T) { runBody := doAPICreateCommitStatus(NewAPITestContext(t, "user2", "repo1"), path.Base(commitURL), api.CommitStatusState("pending")) runBody(t) wg.Done() From cff6753333a8477898971b96f1bf0b8abeaf73dd Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 2 Dec 2022 07:58:16 +0800 Subject: [PATCH 2/2] Apply suggestions from code review Co-authored-by: silverwind --- models/db/index.go | 4 ++-- models/git/commit_status.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/models/db/index.go b/models/db/index.go index 191edc88e1db1..f840a62c8913f 100644 --- a/models/db/index.go +++ b/models/db/index.go @@ -58,7 +58,7 @@ func SyncMaxResourceIndex(ctx context.Context, tableName string, groupID, maxInd return nil } -func postGresGetNextResourceIndex(ctx context.Context, tableName string, groupID int64) (int64, error) { +func postgresGetNextResourceIndex(ctx context.Context, tableName string, groupID int64) (int64, error) { res, err := GetEngine(ctx).Query(fmt.Sprintf("INSERT INTO %s (group_id, max_index) "+ "VALUES (?,1) ON CONFLICT (group_id) DO UPDATE SET max_index = %s.max_index+1 RETURNING max_index", tableName, tableName), groupID) @@ -74,7 +74,7 @@ func postGresGetNextResourceIndex(ctx context.Context, tableName string, groupID // GetNextResourceIndex generates a resource index, it must run in the same transaction where the resource is created func GetNextResourceIndex(ctx context.Context, tableName string, groupID int64) (int64, error) { if setting.Database.UsePostgreSQL { - return postGresGetNextResourceIndex(ctx, tableName, groupID) + return postgresGetNextResourceIndex(ctx, tableName, groupID) } e := GetEngine(ctx) diff --git a/models/git/commit_status.go b/models/git/commit_status.go index 732b53563f4b2..928f1771fe939 100644 --- a/models/git/commit_status.go +++ b/models/git/commit_status.go @@ -50,7 +50,7 @@ func init() { db.RegisterModel(new(CommitStatusIndex)) } -func postGresGetCommitStatusIndex(ctx context.Context, repoID int64, sha string) (int64, error) { +func postgresGetCommitStatusIndex(ctx context.Context, repoID int64, sha string) (int64, error) { res, err := db.GetEngine(ctx).Query("INSERT INTO `commit_status_index` (repo_id, sha, max_index) "+ "VALUES (?,?,1) ON CONFLICT (repo_id, sha) DO UPDATE SET max_index = `commit_status_index`.max_index+1 RETURNING max_index", repoID, sha) @@ -66,7 +66,7 @@ func postGresGetCommitStatusIndex(ctx context.Context, repoID int64, sha string) // GetNextCommitStatusIndex retried 3 times to generate a resource index func GetNextCommitStatusIndex(ctx context.Context, repoID int64, sha string) (int64, error) { if setting.Database.UsePostgreSQL { - return postGresGetCommitStatusIndex(ctx, repoID, sha) + return postgresGetCommitStatusIndex(ctx, repoID, sha) } e := db.GetEngine(ctx)