diff --git a/models/db/index.go b/models/db/index.go index 673c382b27fcf..1a6359c6ea453 100644 --- a/models/db/index.go +++ b/models/db/index.go @@ -47,6 +47,36 @@ func UpsertResourceIndex(ctx context.Context, tableName string, groupID int64) ( return err } +// UpsertDecrResourceIndex the function will not return until it acquires the lock or receives an error. +func UpsertDecrResourceIndex(ctx context.Context, tableName string, groupID, prevIdx int64) (err error) { + // An atomic UPSERT operation (INSERT/UPDATE) is the only operation + // that ensures that the key is actually locked. + // It will try to decrease the max_index, however if max_index is already higher, + // then don't try to decrease the index value. + switch { + case setting.Database.UseSQLite3 || setting.Database.UsePostgreSQL: + _, err = Exec(ctx, fmt.Sprintf("INSERT INTO %s (group_id, max_index) "+ + "VALUES (?,1) ON CONFLICT (group_id) DO UPDATE SET max_index = CASE WHEN %s.max_index = %d THEN %s.max_index-1 ELSE %s.max_index END", + tableName, tableName, prevIdx, tableName, tableName), groupID) + case setting.Database.UseMySQL: + _, err = Exec(ctx, fmt.Sprintf("INSERT INTO %s (group_id, max_index) "+ + "VALUES (?,1) ON DUPLICATE KEY UPDATE max_index = CASE WHEN max_index = %d THEN max_index-1 ELSE max_index END", tableName, prevIdx), + groupID) + case setting.Database.UseMSSQL: + // https://weblogs.sqlteam.com/dang/2009/01/31/upsert-race-condition-with-merge/ + _, err = Exec(ctx, fmt.Sprintf("MERGE %s WITH (HOLDLOCK) as target "+ + "USING (SELECT ? AS group_id) AS src "+ + "ON src.group_id = target.group_id "+ + "WHEN MATCHED THEN UPDATE SET target.max_index = CASE WHEN target.max_index = %d THEN target.max_index-1 ELSE target.max_index END"+ + "WHEN NOT MATCHED THEN INSERT (group_id, max_index) "+ + "VALUES (src.group_id, 1);", tableName, prevIdx), + groupID) + default: + return fmt.Errorf("database type not supported") + } + return err +} + var ( // ErrResouceOutdated represents an error when request resource outdated ErrResouceOutdated = errors.New("resource outdated") diff --git a/models/issues/issue.go b/models/issues/issue.go index 49bc229c6bd03..c8c46de255f02 100644 --- a/models/issues/issue.go +++ b/models/issues/issue.go @@ -1077,12 +1077,27 @@ func NewIssue(repo *repo_model.Repository, issue *Issue, labelIDs []int64, uuids } defer committer.Close() + // onFail will try to gracefully revert the update in the resource index. + onFail := func() error { + // Close commiter. + committer.Close() + // Try to revert the increase in resource index. + if err := db.UpsertDecrResourceIndex(db.DefaultContext, "issue_index", repo.ID, idx); err != nil { + return fmt.Errorf("UpsertDecrResourceIndex: %v", err) + } + return nil + } + if err = NewIssueWithIndex(ctx, issue.Poster, NewIssueOptions{ Repo: repo, Issue: issue, LabelIDs: labelIDs, Attachments: uuids, }); err != nil { + if err := onFail(); err != nil { + return fmt.Errorf("couldn't gracefully handle error: %v", err) + } + if repo_model.IsErrUserDoesNotHaveAccessToRepo(err) || IsErrNewIssueInsert(err) { return err } @@ -1090,6 +1105,10 @@ func NewIssue(repo *repo_model.Repository, issue *Issue, labelIDs []int64, uuids } if err = committer.Commit(); err != nil { + if err := onFail(); err != nil { + return fmt.Errorf("couldn't gracefully handle error: %v", err) + } + return fmt.Errorf("Commit: %v", err) } diff --git a/models/issues/pull.go b/models/issues/pull.go index f96b03445e91f..392577b6a4bff 100644 --- a/models/issues/pull.go +++ b/models/issues/pull.go @@ -504,6 +504,17 @@ func NewPullRequest(outerCtx context.Context, repo *repo_model.Repository, issue defer committer.Close() ctx.WithContext(outerCtx) + // onFail will try to gracefully revert the update in the resource index. + onFail := func() error { + // Close commiter. + committer.Close() + // Try to revert the increase in resource index. + if err := db.UpsertDecrResourceIndex(outerCtx, "issue_index", repo.ID, idx); err != nil { + return fmt.Errorf("UpsertDecrResourceIndex: %v", err) + } + return nil + } + if err = NewIssueWithIndex(ctx, issue.Poster, NewIssueOptions{ Repo: repo, Issue: issue, @@ -511,6 +522,9 @@ func NewPullRequest(outerCtx context.Context, repo *repo_model.Repository, issue Attachments: uuids, IsPull: true, }); err != nil { + if err := onFail(); err != nil { + return fmt.Errorf("couldn't gracefully handle error: %v", err) + } if repo_model.IsErrUserDoesNotHaveAccessToRepo(err) || IsErrNewIssueInsert(err) { return err } @@ -521,10 +535,18 @@ func NewPullRequest(outerCtx context.Context, repo *repo_model.Repository, issue pr.BaseRepo = repo pr.IssueID = issue.ID if err = db.Insert(ctx, pr); err != nil { + if err := onFail(); err != nil { + return fmt.Errorf("couldn't gracefully handle error: %v", err) + } + return fmt.Errorf("insert pull repo: %v", err) } if err = committer.Commit(); err != nil { + if err := onFail(); err != nil { + return fmt.Errorf("couldn't gracefully handle error: %v", err) + } + return fmt.Errorf("Commit: %v", err) }