Skip to content

Commit 7443a10

Browse files
authored
Move from max( id ) to max( index ) for latest commit statuses (#30076)
This PR replaces the use of `max( id )`, and instead using ``max( `index` )`` for determining the latest commit status. Building business logic over an `auto_increment` primary key like `id` is risky and there’re already plenty of discussions on the Internet. There‘s no guarantee for `auto_increment` values to be monotonic, especially upon failures or with a cluster. In the specific case, we met the problem of commit statuses being outdated when using TiDB as the database. As [being documented](https://docs.pingcap.com/tidb/stable/auto-increment), `auto_increment` values assigned to an `insert` statement will only be monotonic on a per server (node) basis. Closes #30074.
1 parent 0d5abe3 commit 7443a10

File tree

1 file changed

+73
-47
lines changed

1 file changed

+73
-47
lines changed

models/git/commit_status.go

Lines changed: 73 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import (
2525
"code.gitea.io/gitea/modules/translation"
2626

2727
"xorm.io/builder"
28+
"xorm.io/xorm"
2829
)
2930

3031
// CommitStatus holds a single Status of a single Commit
@@ -269,44 +270,48 @@ type CommitStatusIndex struct {
269270

270271
// GetLatestCommitStatus returns all statuses with a unique context for a given commit.
271272
func GetLatestCommitStatus(ctx context.Context, repoID int64, sha string, listOptions db.ListOptions) ([]*CommitStatus, int64, error) {
272-
ids := make([]int64, 0, 10)
273-
sess := db.GetEngine(ctx).Table(&CommitStatus{}).
274-
Where("repo_id = ?", repoID).And("sha = ?", sha).
275-
Select("max( id ) as id").
276-
GroupBy("context_hash").OrderBy("max( id ) desc")
273+
getBase := func() *xorm.Session {
274+
return db.GetEngine(ctx).Table(&CommitStatus{}).
275+
Where("repo_id = ?", repoID).And("sha = ?", sha)
276+
}
277+
indices := make([]int64, 0, 10)
278+
sess := getBase().Select("max( `index` ) as `index`").
279+
GroupBy("context_hash").OrderBy("max( `index` ) desc")
277280
if !listOptions.IsListAll() {
278281
sess = db.SetSessionPagination(sess, &listOptions)
279282
}
280-
count, err := sess.FindAndCount(&ids)
283+
count, err := sess.FindAndCount(&indices)
281284
if err != nil {
282285
return nil, count, err
283286
}
284-
statuses := make([]*CommitStatus, 0, len(ids))
285-
if len(ids) == 0 {
287+
statuses := make([]*CommitStatus, 0, len(indices))
288+
if len(indices) == 0 {
286289
return statuses, count, nil
287290
}
288-
return statuses, count, db.GetEngine(ctx).In("id", ids).Find(&statuses)
291+
return statuses, count, getBase().And(builder.In("`index`", indices)).Find(&statuses)
289292
}
290293

291294
// GetLatestCommitStatusForPairs returns all statuses with a unique context for a given list of repo-sha pairs
292295
func GetLatestCommitStatusForPairs(ctx context.Context, repoIDsToLatestCommitSHAs map[int64]string, listOptions db.ListOptions) (map[int64][]*CommitStatus, error) {
293296
type result struct {
294-
ID int64
297+
Index int64
295298
RepoID int64
296299
}
297300

298301
results := make([]result, 0, len(repoIDsToLatestCommitSHAs))
299302

300-
sess := db.GetEngine(ctx).Table(&CommitStatus{})
303+
getBase := func() *xorm.Session {
304+
return db.GetEngine(ctx).Table(&CommitStatus{})
305+
}
301306

302307
// Create a disjunction of conditions for each repoID and SHA pair
303308
conds := make([]builder.Cond, 0, len(repoIDsToLatestCommitSHAs))
304309
for repoID, sha := range repoIDsToLatestCommitSHAs {
305310
conds = append(conds, builder.Eq{"repo_id": repoID, "sha": sha})
306311
}
307-
sess = sess.Where(builder.Or(conds...)).
308-
Select("max( id ) as id, repo_id").
309-
GroupBy("context_hash, repo_id").OrderBy("max( id ) desc")
312+
sess := getBase().Where(builder.Or(conds...)).
313+
Select("max( `index` ) as `index`, repo_id").
314+
GroupBy("context_hash, repo_id").OrderBy("max( `index` ) desc")
310315

311316
if !listOptions.IsListAll() {
312317
sess = db.SetSessionPagination(sess, &listOptions)
@@ -317,15 +322,21 @@ func GetLatestCommitStatusForPairs(ctx context.Context, repoIDsToLatestCommitSHA
317322
return nil, err
318323
}
319324

320-
ids := make([]int64, 0, len(results))
321325
repoStatuses := make(map[int64][]*CommitStatus)
322-
for _, result := range results {
323-
ids = append(ids, result.ID)
324-
}
325326

326-
statuses := make([]*CommitStatus, 0, len(ids))
327-
if len(ids) > 0 {
328-
err = db.GetEngine(ctx).In("id", ids).Find(&statuses)
327+
if len(results) > 0 {
328+
statuses := make([]*CommitStatus, 0, len(results))
329+
330+
conds = make([]builder.Cond, 0, len(results))
331+
for _, result := range results {
332+
cond := builder.Eq{
333+
"`index`": result.Index,
334+
"repo_id": result.RepoID,
335+
"sha": repoIDsToLatestCommitSHAs[result.RepoID],
336+
}
337+
conds = append(conds, cond)
338+
}
339+
err = getBase().Where(builder.Or(conds...)).Find(&statuses)
329340
if err != nil {
330341
return nil, err
331342
}
@@ -342,42 +353,43 @@ func GetLatestCommitStatusForPairs(ctx context.Context, repoIDsToLatestCommitSHA
342353
// GetLatestCommitStatusForRepoCommitIDs returns all statuses with a unique context for a given list of repo-sha pairs
343354
func GetLatestCommitStatusForRepoCommitIDs(ctx context.Context, repoID int64, commitIDs []string) (map[string][]*CommitStatus, error) {
344355
type result struct {
345-
ID int64
346-
Sha string
356+
Index int64
357+
SHA string
347358
}
348359

360+
getBase := func() *xorm.Session {
361+
return db.GetEngine(ctx).Table(&CommitStatus{}).Where("repo_id = ?", repoID)
362+
}
349363
results := make([]result, 0, len(commitIDs))
350364

351-
sess := db.GetEngine(ctx).Table(&CommitStatus{})
352-
353-
// Create a disjunction of conditions for each repoID and SHA pair
354365
conds := make([]builder.Cond, 0, len(commitIDs))
355366
for _, sha := range commitIDs {
356367
conds = append(conds, builder.Eq{"sha": sha})
357368
}
358-
sess = sess.Where(builder.Eq{"repo_id": repoID}.And(builder.Or(conds...))).
359-
Select("max( id ) as id, sha").
360-
GroupBy("context_hash, sha").OrderBy("max( id ) desc")
369+
sess := getBase().And(builder.Or(conds...)).
370+
Select("max( `index` ) as `index`, sha").
371+
GroupBy("context_hash, sha").OrderBy("max( `index` ) desc")
361372

362373
err := sess.Find(&results)
363374
if err != nil {
364375
return nil, err
365376
}
366377

367-
ids := make([]int64, 0, len(results))
368378
repoStatuses := make(map[string][]*CommitStatus)
369-
for _, result := range results {
370-
ids = append(ids, result.ID)
371-
}
372379

373-
statuses := make([]*CommitStatus, 0, len(ids))
374-
if len(ids) > 0 {
375-
err = db.GetEngine(ctx).In("id", ids).Find(&statuses)
380+
if len(results) > 0 {
381+
statuses := make([]*CommitStatus, 0, len(results))
382+
383+
conds = make([]builder.Cond, 0, len(results))
384+
for _, result := range results {
385+
conds = append(conds, builder.Eq{"`index`": result.Index, "sha": result.SHA})
386+
}
387+
err = getBase().And(builder.Or(conds...)).Find(&statuses)
376388
if err != nil {
377389
return nil, err
378390
}
379391

380-
// Group the statuses by repo ID
392+
// Group the statuses by commit
381393
for _, status := range statuses {
382394
repoStatuses[status.SHA] = append(repoStatuses[status.SHA], status)
383395
}
@@ -388,22 +400,36 @@ func GetLatestCommitStatusForRepoCommitIDs(ctx context.Context, repoID int64, co
388400

389401
// FindRepoRecentCommitStatusContexts returns repository's recent commit status contexts
390402
func FindRepoRecentCommitStatusContexts(ctx context.Context, repoID int64, before time.Duration) ([]string, error) {
403+
type result struct {
404+
Index int64
405+
SHA string
406+
}
407+
getBase := func() *xorm.Session {
408+
return db.GetEngine(ctx).Table(&CommitStatus{}).Where("repo_id = ?", repoID)
409+
}
410+
391411
start := timeutil.TimeStampNow().AddDuration(-before)
392-
ids := make([]int64, 0, 10)
393-
if err := db.GetEngine(ctx).Table("commit_status").
394-
Where("repo_id = ?", repoID).
395-
And("updated_unix >= ?", start).
396-
Select("max( id ) as id").
397-
GroupBy("context_hash").OrderBy("max( id ) desc").
398-
Find(&ids); err != nil {
412+
results := make([]result, 0, 10)
413+
414+
sess := getBase().And("updated_unix >= ?", start).
415+
Select("max( `index` ) as `index`, sha").
416+
GroupBy("context_hash, sha").OrderBy("max( `index` ) desc")
417+
418+
err := sess.Find(&results)
419+
if err != nil {
399420
return nil, err
400421
}
401422

402-
contexts := make([]string, 0, len(ids))
403-
if len(ids) == 0 {
423+
contexts := make([]string, 0, len(results))
424+
if len(results) == 0 {
404425
return contexts, nil
405426
}
406-
return contexts, db.GetEngine(ctx).Select("context").Table("commit_status").In("id", ids).Find(&contexts)
427+
428+
conds := make([]builder.Cond, 0, len(results))
429+
for _, result := range results {
430+
conds = append(conds, builder.Eq{"`index`": result.Index, "sha": result.SHA})
431+
}
432+
return contexts, getBase().And(builder.Or(conds...)).Select("context").Find(&contexts)
407433
}
408434

409435
// NewCommitStatusOptions holds options for creating a CommitStatus

0 commit comments

Comments
 (0)