aboutsummaryrefslogtreecommitdiffstats
path: root/models/git/commit_status.go
diff options
context:
space:
mode:
authorLunny Xiao <xiaolunwen@gmail.com>2022-12-01 00:41:49 +0800
committerGitHub <noreply@github.com>2022-12-01 00:41:49 +0800
commitb2c4870481329889a76ea2421152647d36dd8374 (patch)
tree2c38760bed01e1b2da3a36fa67917e4e60b5c252 /models/git/commit_status.go
parent67881ae99abc673954d56436730af313c93ee48f (diff)
downloadgitea-b2c4870481329889a76ea2421152647d36dd8374.tar.gz
gitea-b2c4870481329889a76ea2421152647d36dd8374.zip
Fix parallel creating commit status bug with tests (#21911)
This PR is a follow up of #21469 Co-authored-by: Lauris BH <lauris@nix.lv>
Diffstat (limited to 'models/git/commit_status.go')
-rw-r--r--models/git/commit_status.go101
1 files changed, 36 insertions, 65 deletions
diff --git a/models/git/commit_status.go b/models/git/commit_status.go
index 411fbbe536..6353bb2fa9 100644
--- a/models/git/commit_status.go
+++ b/models/git/commit_status.go
@@ -6,6 +6,7 @@ package git
import (
"context"
"crypto/sha1"
+ "errors"
"fmt"
"net/url"
"strings"
@@ -48,79 +49,49 @@ func init() {
db.RegisterModel(new(CommitStatusIndex))
}
-// upsertCommitStatusIndex the function will not return until it acquires the lock or receives an error.
-func upsertCommitStatusIndex(ctx context.Context, repoID int64, sha string) (err error) {
- // An atomic UPSERT operation (INSERT/UPDATE) is the only operation
- // that ensures that the key is actually locked.
- switch {
- case setting.Database.UseSQLite3 || setting.Database.UsePostgreSQL:
- _, err = db.Exec(ctx, "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",
- repoID, sha)
- case setting.Database.UseMySQL:
- _, err = db.Exec(ctx, "INSERT INTO `commit_status_index` (repo_id, sha, max_index) "+
- "VALUES (?,?,1) ON DUPLICATE KEY UPDATE max_index = max_index+1",
- repoID, sha)
- case setting.Database.UseMSSQL:
- // https://weblogs.sqlteam.com/dang/2009/01/31/upsert-race-condition-with-merge/
- _, err = db.Exec(ctx, "MERGE `commit_status_index` WITH (HOLDLOCK) as target "+
- "USING (SELECT ? AS repo_id, ? AS sha) AS src "+
- "ON src.repo_id = target.repo_id AND src.sha = target.sha "+
- "WHEN MATCHED THEN UPDATE SET target.max_index = target.max_index+1 "+
- "WHEN NOT MATCHED THEN INSERT (repo_id, sha, max_index) "+
- "VALUES (src.repo_id, src.sha, 1);",
- repoID, sha)
- default:
- return fmt.Errorf("database type not supported")
- }
- return err
-}
-
// GetNextCommitStatusIndex retried 3 times to generate a resource index
-func GetNextCommitStatusIndex(repoID int64, sha string) (int64, error) {
- for i := 0; i < db.MaxDupIndexAttempts; i++ {
- idx, err := getNextCommitStatusIndex(repoID, sha)
- if err == db.ErrResouceOutdated {
- continue
- }
- if err != nil {
- return 0, err
- }
- return idx, nil
- }
- return 0, db.ErrGetResourceIndexFailed
-}
+func GetNextCommitStatusIndex(ctx context.Context, repoID int64, sha string) (int64, error) {
+ e := db.GetEngine(ctx)
-// getNextCommitStatusIndex return the next index
-func getNextCommitStatusIndex(repoID int64, sha string) (int64, error) {
- ctx, commiter, err := db.TxContext(db.DefaultContext)
+ // try to update the max_index to next value, and acquire the write-lock for the record
+ res, err := e.Exec("UPDATE `commit_status_index` SET max_index=max_index+1 WHERE repo_id=? AND sha=?", repoID, sha)
if err != nil {
return 0, err
}
- defer commiter.Close()
-
- var preIdx int64
- _, err = db.GetEngine(ctx).SQL("SELECT max_index FROM `commit_status_index` WHERE repo_id = ? AND sha = ?", repoID, sha).Get(&preIdx)
+ affected, err := res.RowsAffected()
if err != nil {
return 0, err
}
-
- if err := upsertCommitStatusIndex(ctx, repoID, sha); err != nil {
- return 0, err
+ if affected == 0 {
+ // this slow path is only for the first time of creating a resource index
+ _, errIns := e.Exec("INSERT INTO `commit_status_index` (repo_id, sha, max_index) VALUES (?, ?, 0)", repoID, sha)
+ res, err = e.Exec("UPDATE `commit_status_index` SET max_index=max_index+1 WHERE repo_id=? AND sha=?", repoID, sha)
+ if err != nil {
+ return 0, err
+ }
+ affected, err = res.RowsAffected()
+ if err != nil {
+ return 0, err
+ }
+ // if the update still can not update any records, the record must not exist and there must be some errors (insert error)
+ if affected == 0 {
+ if errIns == nil {
+ return 0, errors.New("impossible error when GetNextCommitStatusIndex, insert and update both succeeded but no record is updated")
+ }
+ return 0, errIns
+ }
}
- var curIdx int64
- has, err := db.GetEngine(ctx).SQL("SELECT max_index FROM `commit_status_index` WHERE repo_id = ? AND sha = ? AND max_index=?", repoID, sha, preIdx+1).Get(&curIdx)
+ // now, the new index is in database (protected by the transaction and write-lock)
+ var newIdx int64
+ has, err := e.SQL("SELECT max_index FROM `commit_status_index` WHERE repo_id=? AND sha=?", repoID, sha).Get(&newIdx)
if err != nil {
return 0, err
}
if !has {
- return 0, db.ErrResouceOutdated
+ return 0, errors.New("impossible error when GetNextCommitStatusIndex, upsert succeeded but no record can be selected")
}
- if err := commiter.Commit(); err != nil {
- return 0, err
- }
- return curIdx, nil
+ return newIdx, nil
}
func (status *CommitStatus) loadAttributes(ctx context.Context) (err error) {
@@ -290,18 +261,18 @@ func NewCommitStatus(opts NewCommitStatusOptions) error {
return fmt.Errorf("NewCommitStatus[%s, %s]: no user specified", repoPath, opts.SHA)
}
- // Get the next Status Index
- idx, err := GetNextCommitStatusIndex(opts.Repo.ID, opts.SHA)
- if err != nil {
- return fmt.Errorf("generate commit status index failed: %w", err)
- }
-
ctx, committer, err := db.TxContext(db.DefaultContext)
if err != nil {
return fmt.Errorf("NewCommitStatus[repo_id: %d, user_id: %d, sha: %s]: %w", opts.Repo.ID, opts.Creator.ID, opts.SHA, err)
}
defer committer.Close()
+ // Get the next Status Index
+ idx, err := GetNextCommitStatusIndex(ctx, opts.Repo.ID, opts.SHA)
+ if err != nil {
+ return fmt.Errorf("generate commit status index failed: %w", err)
+ }
+
opts.CommitStatus.Description = strings.TrimSpace(opts.CommitStatus.Description)
opts.CommitStatus.Context = strings.TrimSpace(opts.CommitStatus.Context)
opts.CommitStatus.TargetURL = strings.TrimSpace(opts.CommitStatus.TargetURL)
@@ -315,7 +286,7 @@ func NewCommitStatus(opts NewCommitStatusOptions) error {
// Insert new CommitStatus
if _, err = db.GetEngine(ctx).Insert(opts.CommitStatus); err != nil {
- return fmt.Errorf("Insert CommitStatus[%s, %s]: %w", repoPath, opts.SHA, err)
+ return fmt.Errorf("insert CommitStatus[%s, %s]: %w", repoPath, opts.SHA, err)
}
return committer.Commit()