Transaction-aware retry create issue to cope with duplicate keys (#8307)
* Revert #7898 * Transaction-aware retry create issue to cope with duplicate keys * Restore INSERT ... WHERE usage * Rearrange code for clarity * Fix error return in newIssue() * Fix error message
This commit is contained in:
		
							parent
							
								
									c9f819eae0
								
							
						
					
					
						commit
						cd13f273d1
					
				
					 3 changed files with 57 additions and 18 deletions
				
			
		|  | @ -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() | ||||
| } | ||||
| 
 | ||||
| // __________      .__  .__ __________                                     __
 | ||||
| // \______   \__ __|  | |  |\______   \ ____  ________ __   ____   _______/  |_
 | ||||
| //  |     ___/  |  \  | |  | |       _// __ \/ ____/  |  \_/ __ \ /  ___/\   __\
 | ||||
|  |  | |||
|  | @ -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 | ||||
| } | ||||
|  |  | |||
|  | @ -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) | ||||
|  |  | |||
		Loading…
	
		Reference in a new issue