diff --git a/integrations/locked_resource_test.go b/integrations/locked_resource_test.go new file mode 100644 index 0000000000000..c82c2cf14abd3 --- /dev/null +++ b/integrations/locked_resource_test.go @@ -0,0 +1,137 @@ +// Copyright 2020 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package integrations + +import ( + "fmt" + "strings" + "testing" + "time" + + "code.gitea.io/gitea/models" + "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/setting" + + "github.com/stretchr/testify/assert" +) + +const ( + // The tests will fail if the waiter function takes less than + // blockerDelay minus tolerance to complete. + // Note: these values might require tuning in order to avoid + // false negatives. + waiterDelay = 100 * time.Millisecond + blockerDelay = 200 * time.Millisecond + tolerance = 50 * time.Millisecond // Should be <= (blockerDelay-waiterDelay)/2 +) + +type waitResult struct { + Waited time.Duration + Err error +} + +func TestLockedResource(t *testing.T) { + defer prepareTestEnv(t)() + + // Skip SQLite3 test if we're using _txlock=immediate because we won't be able + // to create multiple update sessions with that setting in place which invalidates + // the test (although the locking mechanism is still useful). + connstr, err := setting.DBConnStr() + assert.NoError(t, err) + if strings.Contains(strings.ToLower(connstr), "_txlock=immediate") { + log.Info("TestLockedResource: test skipped for SQLite because _txlock=immediate is set.") + return + } + + // We need to check whether two goroutines block each other + // Sadly, there's no way to ensure the second goroutine is + // waiting other than using a time delay. The longer the delay, + // the more certain we are the second goroutine is waiting. + + // This check **must** fail as we're not blocking anything + assert.Error(t, blockTest("no block", func(ctx models.DBContext) error { + return nil + })) + + models.AssertNotExistsBean(t, &models.LockedResource{LockType: "test-1", LockKey: 1}) + + // Test with creation (i.e. new lock type) + assert.NoError(t, blockTest("block-new", func(ctx models.DBContext) error { + _, err := models.GetLockedResourceCtx(ctx, "block-test-1", 1) + return err + })) + + // Test without creation (i.e. lock type already exists) + assert.NoError(t, blockTest("block-existing", func(ctx models.DBContext) error { + _, err := models.GetLockedResourceCtx(ctx, "block-test-1", 1) + return err + })) + + // Test with temporary record + assert.NoError(t, blockTest("block-temp", func(ctx models.DBContext) error { + return models.TemporarilyLockResourceKeyCtx(ctx, "temp-1", 1) + })) +} + +func blockTest(name string, f func(ctx models.DBContext) error) error { + cb := make(chan waitResult) + cw := make(chan waitResult) + ref := time.Now() + + go func() { + cb <- blockTestFunc(name, true, ref, f) + }() + go func() { + cw <- blockTestFunc(name, false, ref, f) + }() + + resb := <-cb + resw := <-cw + if resb.Err != nil { + return resb.Err + } + if resw.Err != nil { + return resw.Err + } + + if resw.Waited < blockerDelay-tolerance { + return fmt.Errorf("Waiter not blocked on %s; wait: %d ms, expected > %d ms", + name, resw.Waited.Milliseconds(), (blockerDelay - tolerance).Milliseconds()) + } + + return nil +} + +func blockTestFunc(name string, blocker bool, ref time.Time, f func(ctx models.DBContext) error) (wr waitResult) { + if blocker { + name = fmt.Sprintf("blocker [%s]", name) + } else { + name = fmt.Sprintf("waiter [%s]", name) + } + err := models.WithTx(func(ctx models.DBContext) error { + log.Trace("Entering %s @%d", name, time.Since(ref).Milliseconds()) + if !blocker { + log.Trace("Waiting on %s @%d", name, time.Since(ref).Milliseconds()) + time.Sleep(waiterDelay) + log.Trace("Wait finished on %s @%d", name, time.Since(ref).Milliseconds()) + } + if err := f(ctx); err != nil { + return err + } + if blocker { + log.Trace("Waiting on %s @%d", name, time.Since(ref).Milliseconds()) + time.Sleep(blockerDelay) + log.Trace("Wait finished on %s @%d", name, time.Since(ref).Milliseconds()) + } else { + wr.Waited = time.Since(ref) + } + log.Trace("Finishing %s @%d", name, time.Since(ref).Milliseconds()) + return nil + }) + if err != nil { + wr.Err = fmt.Errorf("error in %s: %v", name, err) + } + return +} diff --git a/models/error.go b/models/error.go index 7370bd1571e56..44e04de68e104 100644 --- a/models/error.go +++ b/models/error.go @@ -1189,21 +1189,6 @@ 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() -} - // ErrIssueWasClosed is used when close a closed issue type ErrIssueWasClosed struct { ID int64 diff --git a/models/issue.go b/models/issue.go index 263655c08944c..8df6b54903e03 100644 --- a/models/issue.go +++ b/models/issue.go @@ -75,9 +75,12 @@ var ( const issueTasksRegexpStr = `(^\s*[-*]\s\[[\sxX]\]\s.)|(\n\s*[-*]\s\[[\sxX]\]\s.)` const issueTasksDoneRegexpStr = `(^\s*[-*]\s\[[xX]\]\s.)|(\n\s*[-*]\s\[[xX]\]\s.)` -const issueMaxDupIndexAttempts = 3 const maxIssueIDs = 950 +// IssueLockedEnumerator is the name of the locked_resource used to +// numerate issues in a repository. +const IssueLockedEnumerator = "repository-index" + func init() { issueTasksPat = regexp.MustCompile(issueTasksRegexpStr) issueTasksDonePat = regexp.MustCompile(issueTasksDoneRegexpStr) @@ -848,19 +851,23 @@ func newIssue(e *xorm.Session, doer *User, opts NewIssueOptions) (err error) { } // Milestone validation should happen before insert actual object. - 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) + // Obtain the next issue number for this repository, which will be locked + // and reserved for the remaining of the transaction. Should the transaction + // be rolled back, the previous value will be restored. + idxresource, err := GetLockedResource(e, IssueLockedEnumerator, opts.Issue.RepoID) if err != nil { - return err + return fmt.Errorf("GetLockedResource(%s)", IssueLockedEnumerator) + } + idxresource.Counter++ + if err := idxresource.UpdateValue(e); err != nil { + return fmt.Errorf("locked.UpdateValue(%s)", IssueLockedEnumerator) } + opts.Issue.Index = idxresource.Counter - // Patch Index with the value calculated by the database - opts.Issue.Index = inserted.Index + if _, err = e.Insert(opts.Issue); err != nil { + return err + } if opts.Issue.MilestoneID > 0 { if _, err = e.Exec("UPDATE `milestone` SET num_issues=num_issues+1 WHERE id=?", opts.Issue.MilestoneID); err != nil { @@ -938,24 +945,6 @@ 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, 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, 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, uuids []string) (err error) { sess := x.NewSession() defer sess.Close() if err = sess.Begin(); err != nil { @@ -968,7 +957,7 @@ func newIssueAttempt(repo *Repository, issue *Issue, labelIDs []int64, uuids []s LabelIDs: labelIDs, Attachments: uuids, }); err != nil { - if IsErrUserDoesNotHaveAccessToRepo(err) || IsErrNewIssueInsert(err) { + if IsErrUserDoesNotHaveAccessToRepo(err) { return err } return fmt.Errorf("newIssue: %v", err) diff --git a/models/issue_test.go b/models/issue_test.go index e1995fc5d2edb..899fc2990487e 100644 --- a/models/issue_test.go +++ b/models/issue_test.go @@ -338,7 +338,7 @@ func TestGetRepoIDsForIssuesOptions(t *testing.T) { } } -func testInsertIssue(t *testing.T, title, content string) { +func testInsertIssue(t *testing.T, title, content string, idx int64) int64 { repo := AssertExistsAndLoadBean(t, &Repository{ID: 1}).(*Repository) user := AssertExistsAndLoadBean(t, &User{ID: 2}).(*User) @@ -357,18 +357,22 @@ func testInsertIssue(t *testing.T, title, content string) { assert.True(t, has) assert.EqualValues(t, issue.Title, newIssue.Title) assert.EqualValues(t, issue.Content, newIssue.Content) - // there are 5 issues and max index is 5 on repository 1, so this one should 6 - assert.EqualValues(t, 6, newIssue.Index) + assert.EqualValues(t, idx, newIssue.Index) - _, err = x.ID(issue.ID).Delete(new(Issue)) - assert.NoError(t, err) + return issue.ID } func TestIssue_InsertIssue(t *testing.T) { assert.NoError(t, PrepareTestDatabase()) - testInsertIssue(t, "my issue1", "special issue's comments?") - testInsertIssue(t, `my issue2, this is my son's love \n \r \ `, "special issue's '' comments?") + // there are 5 issues and max index is 5 on repository 1, so this one should be 6 + created := testInsertIssue(t, "my issue1", "special issue's comments?", 6) + + _, err := x.ID(created).Delete(new(Issue)) + assert.NoError(t, err) + + // deleting an issue should not let a new issue reuse its index number; this one should be 7 + _ = testInsertIssue(t, `my issue2, this is my son's love \n \r \ `, "special issue's '' comments?", 7) } func TestIssue_ResolveMentions(t *testing.T) { diff --git a/models/issue_xref_test.go b/models/issue_xref_test.go index 936d1124be475..160cb8d93af1a 100644 --- a/models/issue_xref_test.go +++ b/models/issue_xref_test.go @@ -130,9 +130,12 @@ func testCreateIssue(t *testing.T, repo, doer int64, title, content string, ispu sess := x.NewSession() defer sess.Close() assert.NoError(t, sess.Begin()) - _, err := sess.SetExpr("`index`", "coalesce(MAX(`index`),0)+1").Where("repo_id=?", repo).Insert(i) + idxresource, err := GetLockedResource(sess, IssueLockedEnumerator, repo) assert.NoError(t, err) - i, err = getIssueByID(sess, i.ID) + idxresource.Counter++ + assert.NoError(t, idxresource.UpdateValue(sess)) + i.Index = idxresource.Counter + _, err = sess.Insert(i) assert.NoError(t, err) assert.NoError(t, i.addCrossReferences(sess, d, false)) assert.NoError(t, sess.Commit()) diff --git a/models/locked_resource.go b/models/locked_resource.go new file mode 100644 index 0000000000000..8b60bc0efc24c --- /dev/null +++ b/models/locked_resource.go @@ -0,0 +1,116 @@ +// Copyright 2020 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package models + +import ( + "fmt" + + "code.gitea.io/gitea/modules/setting" +) + +// LockedResource represents the locking key for a pessimistic +// lock that can hold a counter +type LockedResource struct { + LockType string `xorm:"pk VARCHAR(30)"` + LockKey int64 `xorm:"pk"` + Counter int64 `xorm:"NOT NULL DEFAULT 0"` +} + +// GetLockedResource gets or creates a pessimistic lock on the given type and key +func GetLockedResource(e Engine, lockType string, lockKey int64) (*LockedResource, error) { + resource := &LockedResource{LockType: lockType, LockKey: lockKey} + + if err := upsertLockedResource(e, resource); err != nil { + return nil, fmt.Errorf("upsertLockedResource: %v", err) + } + + // Read back the record we've created or locked to get the current Counter value + if has, err := e.Table(resource).NoCache().NoAutoCondition().AllCols(). + Where("lock_type = ? AND lock_key = ?", lockType, lockKey).Get(resource); err != nil { + return nil, fmt.Errorf("get locked resource %s:%d: %v", lockType, lockKey, err) + } else if !has { + return nil, fmt.Errorf("unexpected upsert fail %s:%d", lockType, lockKey) + } + + return resource, nil +} + +// UpdateValue updates the value of the counter of a locked resource +func (r *LockedResource) UpdateValue(e Engine) error { + // Bypass ORM to support lock_type == "" and lock_key == 0 + _, err := e.Exec("UPDATE locked_resource SET counter = ? WHERE lock_type = ? AND lock_key = ?", + r.Counter, r.LockType, r.LockKey) + return err +} + +// Delete deletes the locked resource from the database, +// but the key remains locked until the end of the transaction +func (r *LockedResource) Delete(e Engine) error { + // Bypass ORM to support lock_type == "" and lock_key == 0 + _, err := e.Exec("DELETE FROM locked_resource WHERE lock_type = ? AND lock_key = ?", r.LockType, r.LockKey) + return err +} + +// DeleteLockedResourceKey deletes a locked resource by key +func DeleteLockedResourceKey(e Engine, lockType string, lockKey int64) error { + // Bypass ORM to support lock_type == "" and lock_key == 0 + _, err := e.Exec("DELETE FROM locked_resource WHERE lock_type = ? AND lock_key = ?", lockType, lockKey) + return err +} + +// TemporarilyLockResourceKey locks the given key but does not leave a permanent record +func TemporarilyLockResourceKey(e Engine, lockType string, lockKey int64) error { + // Temporary locked resources should not exist in the table. + // This allows us to use a simple INSERT to lock the key. + _, err := e.Exec("INSERT INTO locked_resource (lock_type, lock_key) VALUES (?, ?)", lockType, lockKey) + if err == nil { + _, err = e.Exec("DELETE FROM locked_resource WHERE lock_type = ? AND lock_key = ?", lockType, lockKey) + } + return err +} + +// GetLockedResourceCtx gets or creates a pessimistic lock on the given type and key +func GetLockedResourceCtx(ctx DBContext, lockType string, lockKey int64) (*LockedResource, error) { + return GetLockedResource(ctx.e, lockType, lockKey) +} + +// DeleteLockedResourceKeyCtx deletes a locked resource by key +func DeleteLockedResourceKeyCtx(ctx DBContext, lockType string, lockKey int64) error { + return DeleteLockedResourceKey(ctx.e, lockType, lockKey) +} + +// TemporarilyLockResourceKeyCtx locks the given key but does not leave a permanent record +func TemporarilyLockResourceKeyCtx(ctx DBContext, lockType string, lockKey int64) error { + return TemporarilyLockResourceKey(ctx.e, lockType, lockKey) +} + +// upsertLockedResource will create or lock the given key in the database. +// the function will not return until it acquires the lock or receives an error. +func upsertLockedResource(e Engine, resource *LockedResource) (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 = e.Exec("INSERT INTO locked_resource (lock_type, lock_key) "+ + "VALUES (?,?) ON CONFLICT(lock_type, lock_key) DO UPDATE SET lock_key = ?", + resource.LockType, resource.LockKey, resource.LockKey) + case setting.Database.UseMySQL: + _, err = e.Exec("INSERT INTO locked_resource (lock_type, lock_key) "+ + "VALUES (?,?) ON DUPLICATE KEY UPDATE lock_key = lock_key", + resource.LockType, resource.LockKey) + case setting.Database.UseMSSQL: + // https://weblogs.sqlteam.com/dang/2009/01/31/upsert-race-condition-with-merge/ + _, err = e.Exec("MERGE locked_resource WITH (HOLDLOCK) as target "+ + "USING (SELECT ? AS lock_type, ? AS lock_key) AS src "+ + "ON src.lock_type = target.lock_type AND src.lock_key = target.lock_key "+ + "WHEN MATCHED THEN UPDATE SET target.lock_key = target.lock_key "+ + "WHEN NOT MATCHED THEN INSERT (lock_type, lock_key) "+ + "VALUES (src.lock_type, src.lock_key);", + resource.LockType, resource.LockKey) + default: + return fmt.Errorf("database type not supported") + } + return +} diff --git a/models/locked_resource_test.go b/models/locked_resource_test.go new file mode 100644 index 0000000000000..48959ae3f4de1 --- /dev/null +++ b/models/locked_resource_test.go @@ -0,0 +1,101 @@ +// Copyright 2020 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package models + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "xorm.io/xorm" +) + +func TestLockedResource(t *testing.T) { + assert.NoError(t, PrepareTestDatabase()) + + withTransaction := func(t *testing.T, f func(t *testing.T, sess *xorm.Session) bool) { + sess := x.NewSession() + defer sess.Close() + err := sess.Begin() + if !assert.NoError(t, err) { + return + } + if success := f(t, sess); !success { + return + } + err = sess.Commit() + assert.NoError(t, err) + } + + // Get lock, increment counter value + withTransaction(t, func(t *testing.T, sess *xorm.Session) bool { + resource, err := GetLockedResource(sess, "test-1", 1) + if !assert.NoError(t, err) || !assert.NotEmpty(t, resource) || !assert.Equal(t, int64(0), resource.Counter) { + return false + } + resource.Counter++ + err = resource.UpdateValue(sess) + return assert.NoError(t, err) + }) + + // Get lock, check counter value + withTransaction(t, func(t *testing.T, sess *xorm.Session) bool { + resource, err := GetLockedResource(sess, "test-1", 1) + return assert.NoError(t, err) && assert.NotEmpty(t, resource) && assert.Equal(t, int64(1), resource.Counter) + }) + + // Make sure LockKey == 0 is supported and we're not + // affecting other records + withTransaction(t, func(t *testing.T, sess *xorm.Session) bool { + resource, err := GetLockedResource(sess, "test-1", 0) + if !assert.NoError(t, err) || !assert.NotEmpty(t, resource) || !assert.Equal(t, int64(0), resource.Counter) { + return false + } + resource.Counter = 77 + return assert.NoError(t, resource.UpdateValue(sess)) + }) + resource, err := GetLockedResource(x, "test-1", 0) + assert.NoError(t, err) + assert.NotEmpty(t, resource) + assert.Equal(t, int64(77), resource.Counter) + + assert.NoError(t, DeleteLockedResourceKey(x, "test-1", 0)) + AssertExistsAndLoadBean(t, &LockedResource{LockType: "test-1", LockKey: 1}) + + // Attempt temp lock on an existing key, expect error + withTransaction(t, func(t *testing.T, sess *xorm.Session) bool { + err := TemporarilyLockResourceKey(sess, "test-1", 1) + // Must give error + return assert.Error(t, err) + }) + + // Delete lock + withTransaction(t, func(t *testing.T, sess *xorm.Session) bool { + resource, err := GetLockedResource(sess, "test-1", 1) + if !assert.NoError(t, err) || !assert.NotEmpty(t, resource) { + return false + } + return assert.NoError(t, resource.Delete(sess)) + }) + AssertNotExistsBean(t, &LockedResource{LockType: "test-1", LockKey: 1}) + + // Get lock, then delete by key + withTransaction(t, func(t *testing.T, sess *xorm.Session) bool { + resource, err := GetLockedResource(sess, "test-1", 2) + return assert.NoError(t, err) && assert.NotEmpty(t, resource) + }) + AssertExistsAndLoadBean(t, &LockedResource{LockType: "test-1", LockKey: 2}) + withTransaction(t, func(t *testing.T, sess *xorm.Session) bool { + return assert.NoError(t, DeleteLockedResourceKey(sess, "test-1", 2)) + }) + AssertNotExistsBean(t, &LockedResource{LockType: "test-1", LockKey: 2}) + + // Attempt temp lock on an valid key, expect success + withTransaction(t, func(t *testing.T, sess *xorm.Session) bool { + return assert.NoError(t, TemporarilyLockResourceKey(sess, "test-1", 1)) + }) + + // Note: testing the validity of the locking mechanism (i.e. whether it actually locks) + // is performed at the integration tests to ensure that all the supported databases are checked. +} diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 6868aad7b190a..126ecf549232a 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -210,6 +210,8 @@ var migrations = []Migration{ NewMigration("Add Branch Protection Block Outdated Branch", addBlockOnOutdatedBranch), // v138 -> v139 NewMigration("Add ResolveDoerID to Comment table", addResolveDoerIDCommentColumn), + // v139 -> v140 + NewMigration("Add locked_resource table", addLockedResourceTable), } // GetCurrentDBVersion returns the current db version diff --git a/models/migrations/v139.go b/models/migrations/v139.go new file mode 100644 index 0000000000000..70ecb5f05fbe8 --- /dev/null +++ b/models/migrations/v139.go @@ -0,0 +1,47 @@ +// Copyright 2020 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package migrations + +import ( + "code.gitea.io/gitea/models" + + "xorm.io/xorm" +) + +func addLockedResourceTable(x *xorm.Engine) error { + + type LockedResource struct { + LockType string `xorm:"pk VARCHAR(30)"` + LockKey int64 `xorm:"pk"` + Counter int64 `xorm:"NOT NULL DEFAULT 0"` + } + + sess := x.NewSession() + defer sess.Close() + + if err := sess.Begin(); err != nil { + return err + } + + if err := sess.Sync2(new(LockedResource)); err != nil { + return err + } + + // Remove data we're goint to rebuild + if _, err := sess.Delete(&LockedResource{LockType: models.IssueLockedEnumerator}); err != nil { + return err + } + + // Create current data for all repositories with issues and PRs + if _, err := sess.Exec("INSERT INTO locked_resource (lock_type, lock_key, counter) "+ + "SELECT ?, max_data.repo_id, max_data.max_index "+ + "FROM ( SELECT issue.repo_id AS repo_id, max(issue.`index`) AS max_index "+ + "FROM issue GROUP BY issue.repo_id) AS max_data", + models.IssueLockedEnumerator); err != nil { + return err + } + + return sess.Commit() +} diff --git a/models/models.go b/models/models.go index c818c651007b4..98eb43ffdfcac 100644 --- a/models/models.go +++ b/models/models.go @@ -125,6 +125,7 @@ func init() { new(Task), new(LanguageStat), new(EmailHash), + new(LockedResource), ) gonicNames := []string{"SSL", "UID"} diff --git a/models/pull.go b/models/pull.go index 9f1f485266a59..ad6d625eafbef 100644 --- a/models/pull.go +++ b/models/pull.go @@ -408,24 +408,6 @@ func (pr *PullRequest) SetMerged() (bool, error) { // NewPullRequest creates new pull request with labels for repository. func NewPullRequest(repo *Repository, pull *Issue, labelIDs []int64, uuids []string, pr *PullRequest) (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); 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) (err error) { sess := x.NewSession() defer sess.Close() if err = sess.Begin(); err != nil { @@ -439,7 +421,7 @@ func newPullRequestAttempt(repo *Repository, pull *Issue, labelIDs []int64, uuid Attachments: uuids, IsPull: true, }); err != nil { - if IsErrUserDoesNotHaveAccessToRepo(err) || IsErrNewIssueInsert(err) { + if IsErrUserDoesNotHaveAccessToRepo(err) { return err } return fmt.Errorf("newIssue: %v", err) diff --git a/models/test_fixtures.go b/models/test_fixtures.go index 6c160742bd4f3..b8ddafcaa7d1d 100644 --- a/models/test_fixtures.go +++ b/models/test_fixtures.go @@ -68,5 +68,15 @@ func LoadFixtures() error { } } } + // Finally, we must rebuild the last issue index used for each repositories + if _, err := x.Delete(&LockedResource{LockType: IssueLockedEnumerator}); err != nil { + return err + } + _, err = x.Exec("INSERT INTO locked_resource (lock_type, lock_key, counter) "+ + "SELECT ?, max_data.repo_id, max_data.max_index "+ + "FROM ( SELECT issue.repo_id AS repo_id, max(issue.`index`) AS max_index "+ + "FROM issue GROUP BY issue.repo_id) AS max_data", + IssueLockedEnumerator) + return err }