Retrying the insert on issue fails, and masks the actual error

[guillep2k]

• Gitea version (or commit ref): c6fb7fe

Description

The changes introduced in #7898 simply don't work, because if xorm.Session.Insert() returns any errors, the session becomes invalidated, so subsequent attempts fail with:

newIssue: pq: current transaction is aborted, commands ignored until end of transaction block

gitea/models/issue.go

Lines 1108 to 1122 in c6fb7fe

	// 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
	}
	}

I'm sorry, after so many attempts to solve the potential duplicate index, we're back to square zero.

@lunny how would you like to handle this?

[lunny]

The retry should be done after the transaction end. It should be out of newIssue but we should return the reason of failed. So that we could know if we need retry.

[guillep2k]

The retry should be done after the transaction end. It should be out of newIssue but we should return the reason of failed. So that we could know if we need retry.

I agree, but as I've said here, detecting the correct error is very difficult.

[lunny]

If the error is insert error we could retry it, I think.

[guillep2k]

If the error is insert error we could retry it, I think.

Oh, you mean any errors, as long as they come from the insert? That's much easier. I'll work on that. I'll asume that #8304 will be merged and I'll base on it.

[lunny]

Should be fixed by #8307

[guillep2k]

Agree