diff options
Diffstat (limited to 'models')
| -rw-r--r-- | models/error.go | 15 | ||||
| -rw-r--r-- | models/issue.go | 40 | ||||
| -rw-r--r-- | models/pull.go | 20 |
3 files changed, 57 insertions, 18 deletions
diff --git a/models/error.go b/models/error.go index 9974287a0a..995617e83b 100644 --- a/models/error.go +++ b/models/error.go @@ -1089,6 +1089,21 @@ func (err ErrIssueLabelTemplateLoad) Error() string { return fmt.Sprintf("Failed to load label template file '%s': %v", err.TemplateFile, err.OriginalError) } +// ErrNewIssueInsert is used when the INSERT statement in newIssue fails +type ErrNewIssueInsert struct { + OriginalError error +} + +// IsErrNewIssueInsert checks if an error is a ErrNewIssueInsert. +func IsErrNewIssueInsert(err error) bool { + _, ok := err.(ErrNewIssueInsert) + return ok +} + +func (err ErrNewIssueInsert) Error() string { + return err.OriginalError.Error() +} + // __________ .__ .__ __________ __ // \______ \__ __| | | |\______ \ ____ ________ __ ____ _______/ |_ // | ___/ | \ | | | | _// __ \/ ____/ | \_/ __ \ / ___/\ __\ diff --git a/models/issue.go b/models/issue.go index da98fac7df..09d443384b 100644 --- a/models/issue.go +++ b/models/issue.go @@ -1104,21 +1104,10 @@ func newIssue(e *xorm.Session, doer *User, opts NewIssueOptions) (err error) { } // Milestone and assignee validation should happen before insert actual object. - - // There's no good way to identify a duplicate key error in database/sql; brute force some retries - dupIndexAttempts := issueMaxDupIndexAttempts - for { - _, err := e.SetExpr("`index`", "coalesce(MAX(`index`),0)+1"). - Where("repo_id=?", opts.Issue.RepoID). - Insert(opts.Issue) - if err == nil { - break - } - - dupIndexAttempts-- - if dupIndexAttempts <= 0 { - return err - } + if _, err := e.SetExpr("`index`", "coalesce(MAX(`index`),0)+1"). + Where("repo_id=?", opts.Issue.RepoID). + Insert(opts.Issue); err != nil { + return ErrNewIssueInsert{err} } inserted, err := getIssueByID(e, opts.Issue.ID) @@ -1201,6 +1190,24 @@ func newIssue(e *xorm.Session, doer *User, opts NewIssueOptions) (err error) { // NewIssue creates new issue with labels for repository. func NewIssue(repo *Repository, issue *Issue, labelIDs []int64, assigneeIDs []int64, uuids []string) (err error) { + // Retry several times in case INSERT fails due to duplicate key for (repo_id, index); see #7887 + i := 0 + for { + if err = newIssueAttempt(repo, issue, labelIDs, assigneeIDs, uuids); err == nil { + return nil + } + if !IsErrNewIssueInsert(err) { + return err + } + if i++; i == issueMaxDupIndexAttempts { + break + } + log.Error("NewIssue: error attempting to insert the new issue; will retry. Original error: %v", err) + } + return fmt.Errorf("NewIssue: too many errors attempting to insert the new issue. Last error was: %v", err) +} + +func newIssueAttempt(repo *Repository, issue *Issue, labelIDs []int64, assigneeIDs []int64, uuids []string) (err error) { sess := x.NewSession() defer sess.Close() if err = sess.Begin(); err != nil { @@ -1214,7 +1221,7 @@ func NewIssue(repo *Repository, issue *Issue, labelIDs []int64, assigneeIDs []in Attachments: uuids, AssigneeIDs: assigneeIDs, }); err != nil { - if IsErrUserDoesNotHaveAccessToRepo(err) { + if IsErrUserDoesNotHaveAccessToRepo(err) || IsErrNewIssueInsert(err) { return err } return fmt.Errorf("newIssue: %v", err) @@ -1223,7 +1230,6 @@ func NewIssue(repo *Repository, issue *Issue, labelIDs []int64, assigneeIDs []in if err = sess.Commit(); err != nil { return fmt.Errorf("Commit: %v", err) } - sess.Close() return nil } diff --git a/models/pull.go b/models/pull.go index b41fb004e1..ff1f7b773b 100644 --- a/models/pull.go +++ b/models/pull.go @@ -657,6 +657,24 @@ func (pr *PullRequest) testPatch(e Engine) (err error) { // NewPullRequest creates new pull request with labels for repository. func NewPullRequest(repo *Repository, pull *Issue, labelIDs []int64, uuids []string, pr *PullRequest, patch []byte, assigneeIDs []int64) (err error) { + // Retry several times in case INSERT fails due to duplicate key for (repo_id, index); see #7887 + i := 0 + for { + if err = newPullRequestAttempt(repo, pull, labelIDs, uuids, pr, patch, assigneeIDs); err == nil { + return nil + } + if !IsErrNewIssueInsert(err) { + return err + } + if i++; i == issueMaxDupIndexAttempts { + break + } + log.Error("NewPullRequest: error attempting to insert the new issue; will retry. Original error: %v", err) + } + return fmt.Errorf("NewPullRequest: too many errors attempting to insert the new issue. Last error was: %v", err) +} + +func newPullRequestAttempt(repo *Repository, pull *Issue, labelIDs []int64, uuids []string, pr *PullRequest, patch []byte, assigneeIDs []int64) (err error) { sess := x.NewSession() defer sess.Close() if err = sess.Begin(); err != nil { @@ -671,7 +689,7 @@ func NewPullRequest(repo *Repository, pull *Issue, labelIDs []int64, uuids []str IsPull: true, AssigneeIDs: assigneeIDs, }); err != nil { - if IsErrUserDoesNotHaveAccessToRepo(err) { + if IsErrUserDoesNotHaveAccessToRepo(err) || IsErrNewIssueInsert(err) { return err } return fmt.Errorf("newIssue: %v", err) |