Add a new table issue_index to store the max issue index so that issue could be deleted with no duplicated index

[lunny]

Inspired by #9931 and will replace #9931. @guillep2k

This PR add an index table for issue named issue_index to store repo's last issue index number. When creating a new issue, we will increment the number and get that to use as the issue index.

For other table need index, we could also reuse ResourceIndex to use

type MyTable ResourceIndex

and added it to tables and migrations to define a new table model.

So that issue index generating will not affect release index generating or others. This is the max difference between this and #9931. On that PR, all index will be stored in the same table with a lock_type.
Another difference is this PR still use an optimistic lock to generate the index, it will retry 3 times before giving up.

After this merged if the issue with max index is deleted(not implemented), coming issue will not has duplicated index so that the references will not be a wrong link.

[codecov-commenter]

Codecov Report

Merging #15599 (5bff46e) into main (a005265) will increase coverage by 0.01%.
The diff coverage is 48.10%.

@@            Coverage Diff             @@
##             main   #15599      +/-   ##
==========================================
+ Coverage   44.24%   44.25%   +0.01%     
==========================================
  Files         694      695       +1     
  Lines       82299    82341      +42     
==========================================
+ Hits        36410    36437      +27     
- Misses      40002    40007       +5     
- Partials     5887     5897      +10     

Impacted Files	Coverage Δ
models/repo.go	53.05% <0.00%> (-0.10%)	⬇️
models/unit_tests.go	74.31% <0.00%> (ø)
models/issue.go	58.73% <22.22%> (-0.15%)	⬇️
models/index.go	49.05% <49.05%> (ø)
models/models.go	55.33% <60.00%> (+0.66%)	⬆️
models/pull.go	55.38% <77.77%> (+0.25%)	⬆️
models/log.go	36.58% <0.00%> (-4.88%)	⬇️
models/repo_list.go	77.04% <0.00%> (-0.78%)	⬇️
services/pull/pull.go	43.83% <0.00%> (+0.45%)	⬆️
... and 3 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a005265...5bff46e. Read the comment docs.

[zeripath]

Thanks, @lunny this is of course one of those deeper SQL transactional PRs which is why everyone has avoided reviewing it I suspect - certainly I have been doing so.

I see you've put the GetResourceIndex call outside of the issue insert transaction - I guess the intention here is to have the lock on the resource table for as short as possible a time? The problem of course is that failed transactions inserting to the issue table will then result in the index being incremented despite no issue being present.

I'm not sure that I buy that this is going to be a significant speed up, as ​either the db is a table locking db or a row locking db. If you're a table locking db then the issue table is going to be completely locked during the insert and you've gained nothing by placing the ResourceIndex getter outside of the transaction except lose rollback behaviour, or you're a row locking db in which case the rollback semantics are probably worth the slight degradation. Have you tested this to see if there is an actual difference?

[lunny]

Thanks, @lunny this is of course one of those deeper SQL transactional PRs which is why everyone has avoided reviewing it I suspect - certainly I have been doing so.

I see you've put the GetResourceIndex call outside of the issue insert transaction - I guess the intention here is to have the lock on the resource table for as short as possible a time? The problem of course is that failed transactions inserting to the issue table will then result in the index being incremented despite no issue being present.

I'm not sure that I buy that this is going to be a significant speed up, as ​either the db is a table locking db or a row locking db. If you're a table locking db then the issue table is going to be completely locked during the insert and you've gained nothing by placing the ResourceIndex getter outside of the transaction except lose rollback behaviour, or you're a row locking db in which case the rollback semantics are probably worth the slight degradation. Have you tested this to see if there is an actual difference?

The reason to make GetResourceIndex outside of the issue insert transaction is to make the process as short as possible like you said. The index of issue will not be continuous when deleting issues are allowed so I don't think it's a problem that a failed issue insertion will result in a never used index.

As I know, in parallel requests situation, INSERT on duplicated on MySQL may still probably occupy deadlock. Although I write a test with 100 goroutines to insert issues which never failed. I think the lock is a gap lock which will lock a range of records on the issue_index table when inserting a new record.

[zeripath]

I still really very much prefer #9931.

A deadlock could only occur if you're locking tables in the wrong order, i.e. you lock issue then you lock resource in one set of transactions and then in some transactions you lock resource then lock issue.

However, if you're really not keen to do that then we can do things this way.

[zeripath]

I don't see the benefits of still using an optimistic approach to generate the next index though.

[lunny]

I still really very much prefer #9931.

A deadlock could only occur if you're locking tables in the wrong order, i.e. you lock issue then you lock resource in one set of transactions and then in some transactions you lock resource then lock issue.

However, if you're really not keen to do that then we can do things this way.

#9931 stores all lock resources into one table which is unnecessary. This PR at least create different tables for different resource but use the same method to manage them.

Not only table lock, both record lock and gap lock will also result in deadlock. The deadlock occurs not only because of wrong operations order but also parallel and conflicted requests.

Two locks in different tables of one transaction will result in more possibility on creating failure issues. So I think an optimistic approach should be better. That means less possibility user creating issue failure.