WIP: Remove CommitStatus.Index field.

[bobemoe]

The problem is if two commit status updates come in simultaneously, they get the same Index value, and fail to insert into db, resulting in missing data. (described in and fixes #14059)

As far as I can tell this field is only used to know the sequence for commit statuses of one commit. This can be calculated from the ID field, with no need for the additional Index field.

This PR removes the Index field and refactors the ordering to use the ID field.

I see no other use of CommitStatus.Index, apart from the API exposes ID which used to map to Index and I've changed this to map to ID.

TODO

• Remove Index field
• Sort by ID instead of Index
• Consider API impact
• Fix tests
• Migration
• Testing

[bobemoe]

I don't understand the mysql8 test failure? Did something timeout?

...

=== TestSignupEmail (integrations/signup_test.go:37)
=== TestPushDeployKeyOnEmptyRepo (integrations/ssh_key_test.go:47)
SIGABRT: abort
PC=0x474061 m=0 sigcode=0

goroutine 0 [idle]:
runtime.futex(0x42a5b08, 0x80, 0x0, 0x0, 0x200000000, 0x42a5140, 0x7ffcd8231260, 0x442573, 0x7ffcd8231240, 0x40df9f, ...)
	/usr/local/go/src/runtime/sys_linux_amd64.s:587 +0x21
runtime.futexsleep(0x42a5b08, 0x3ed400000000, 0xffffffffffffffff)
	/usr/local/go/src/runtime/os_linux.go:45 +0x46
runtime.notesleep(0x42a5b08)
	/usr/local/go/src/runtime/lock_futex.go:159 +0x9f
runtime.stopm()
	/usr/local/go/src/runtime/proc.go:1924 +0xc5
runtime.exitsyscall0(0xc006280c00)
	/usr/local/go/src/runtime/proc.go:3415 +0x11d
runtime.mcall(0x0)
	/usr/local/go/src/runtime/asm_amd64.s:318 +0x5b

...

fs     0x0
gs     0x0
timeout: the monitored command dumped core

[lunny]

It seems it's timeout. unrelated with your PR.

[codecov-io]

Codecov Report

Merging #14160 (96023c2) into master (3175d08) will decrease coverage by 0.01%.
The diff coverage is 40.00%.

@@            Coverage Diff             @@
##           master   #14160      +/-   ##
==========================================
- Coverage   42.01%   41.99%   -0.02%     
==========================================
  Files         733      733              
  Lines       78704    78691      -13     
==========================================
- Hits        33070    33050      -20     
- Misses      40202    40208       +6     
- Partials     5432     5433       +1     

Impacted Files	Coverage Δ
models/commit_status.go	62.50% <25.00%> (-0.59%)	⬇️
modules/convert/status.go	50.00% <100.00%> (ø)
modules/indexer/stats/queue.go	52.94% <0.00%> (-23.53%)	⬇️
modules/indexer/stats/db.go	40.00% <0.00%> (-16.00%)	⬇️
modules/indexer/issues/indexer.go	50.76% <0.00%> (-9.24%)	⬇️
modules/queue/unique_queue_channel.go	58.06% <0.00%> (-6.46%)	⬇️
services/pull/pull.go	42.35% <0.00%> (-0.51%)	⬇️
models/error.go	38.98% <0.00%> (ø)
models/issue_comment.go	52.87% <0.00%> (+0.15%)	⬆️
services/mirror/mirror.go	16.85% <0.00%> (+1.10%)	⬆️
... and 5 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 3175d08...35d9a74. Read the comment docs.

[lafriks]

There is no point in adding Id to unique index as all records will be unique anyway, PK already guarantees that. It needs to be rechecked why unique index was set. (Maybe context needs to be added to that uq?)

[bobemoe]

I figured the original usage was to aid the ORDER BY which cant use the index on id alone if there is a WHERE, in which case the index needs to be multipart on repo_id,sha,id as the select will look like select `id`,`index`,`sha`,`state` from `commit_status` where `repo_id`=1 and `sha`='f0e325d9a81450c3f5a81249d19149a74d5e85c9' order by `index`; and I just tried to preserve that. But I wasn't sure how to define the order of columns in the index which is important. currently they in wrong order.

Although that said, the small amount or records for a specific SHA probably wont be that expensive to order without an index.

If there is no other usage for that index, maybe it could be removed.

---

From mysql docs:

SELECT * FROM t1
  WHERE key_part1 = constant
  ORDER BY key_part2;

In this query, key_part1 is constant, so all rows accessed through the index are in key_part2 order, and an index on (key_part1, key_part2) avoids sorting if the WHERE clause is selective enough to make an index range scan cheaper than a table scan. https://dev.mysql.com/doc/refman/5.7/en/order-by-optimization.html#order-by-index-use

[kaenganxt]

Firstly, thanks for your work!
Will this be further worked on?
The bug is a major problem for my setup as my CI tool starts one check when another has finished and half of the time, the "finished" state of the first build is not shown in Gitea and thus blocks merging.

[lunny]

I will continue the work based on #15599