Skip to content

Commit b1dae9f

Browse files
authored
Move from max( id ) to max( index ) for latest commit statuses (#30076) (#30155)
Backport #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 7bffb92 commit b1dae9f

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
@@ -281,44 +282,48 @@ type CommitStatusIndex struct {
281282

282283
// GetLatestCommitStatus returns all statuses with a unique context for a given commit.
283284
func GetLatestCommitStatus(ctx context.Context, repoID int64, sha string, listOptions db.ListOptions) ([]*CommitStatus, int64, error) {
284-
ids := make([]int64, 0, 10)
285-
sess := db.GetEngine(ctx).Table(&CommitStatus{}).
286-
Where("repo_id = ?", repoID).And("sha = ?", sha).
287-
Select("max( id ) as id").
288-
GroupBy("context_hash").OrderBy("max( id ) desc")
285+
getBase := func() *xorm.Session {
286+
return db.GetEngine(ctx).Table(&CommitStatus{}).
287+
Where("repo_id = ?", repoID).And("sha = ?", sha)
288+
}
289+
indices := make([]int64, 0, 10)
290+
sess := getBase().Select("max( `index` ) as `index`").
291+
GroupBy("context_hash").OrderBy("max( `index` ) desc")
289292
if !listOptions.IsListAll() {
290293
sess = db.SetSessionPagination(sess, &listOptions)
291294
}
292-
count, err := sess.FindAndCount(&ids)
295+
count, err := sess.FindAndCount(&indices)
293296
if err != nil {
294297
return nil, count, err
295298
}
296-
statuses := make([]*CommitStatus, 0, len(ids))
297-
if len(ids) == 0 {
299+
statuses := make([]*CommitStatus, 0, len(indices))
300+
if len(indices) == 0 {
298301
return statuses, count, nil
299302
}
300-
return statuses, count, db.GetEngine(ctx).In("id", ids).Find(&statuses)
303+
return statuses, count, getBase().And(builder.In("`index`", indices)).Find(&statuses)
301304
}
302305

303306
// GetLatestCommitStatusForPairs returns all statuses with a unique context for a given list of repo-sha pairs
304307
func GetLatestCommitStatusForPairs(ctx context.Context, repoIDsToLatestCommitSHAs map[int64]string, listOptions db.ListOptions) (map[int64][]*CommitStatus, error) {
305308
type result struct {
306-
ID int64
309+
Index int64
307310
RepoID int64
308311
}
309312

310313
results := make([]result, 0, len(repoIDsToLatestCommitSHAs))
311314

312-
sess := db.GetEngine(ctx).Table(&CommitStatus{})
315+
getBase := func() *xorm.Session {
316+
return db.GetEngine(ctx).Table(&CommitStatus{})
317+
}
313318

314319
// Create a disjunction of conditions for each repoID and SHA pair
315320
conds := make([]builder.Cond, 0, len(repoIDsToLatestCommitSHAs))
316321
for repoID, sha := range repoIDsToLatestCommitSHAs {
317322
conds = append(conds, builder.Eq{"repo_id": repoID, "sha": sha})
318323
}
319-
sess = sess.Where(builder.Or(conds...)).
320-
Select("max( id ) as id, repo_id").
321-
GroupBy("context_hash, repo_id").OrderBy("max( id ) desc")
324+
sess := getBase().Where(builder.Or(conds...)).
325+
Select("max( `index` ) as `index`, repo_id").
326+
GroupBy("context_hash, repo_id").OrderBy("max( `index` ) desc")
322327

323328
sess = db.SetSessionPagination(sess, &listOptions)
324329

@@ -327,15 +332,21 @@ func GetLatestCommitStatusForPairs(ctx context.Context, repoIDsToLatestCommitSHA
327332
return nil, err
328333
}
329334

330-
ids := make([]int64, 0, len(results))
331335
repoStatuses := make(map[int64][]*CommitStatus)
332-
for _, result := range results {
333-
ids = append(ids, result.ID)
334-
}
335336

336-
statuses := make([]*CommitStatus, 0, len(ids))
337-
if len(ids) > 0 {
338-
err = db.GetEngine(ctx).In("id", ids).Find(&statuses)
337+
if len(results) > 0 {
338+
statuses := make([]*CommitStatus, 0, len(results))
339+
340+
conds = make([]builder.Cond, 0, len(results))
341+
for _, result := range results {
342+
cond := builder.Eq{
343+
"`index`": result.Index,
344+
"repo_id": result.RepoID,
345+
"sha": repoIDsToLatestCommitSHAs[result.RepoID],
346+
}
347+
conds = append(conds, cond)
348+
}
349+
err = getBase().Where(builder.Or(conds...)).Find(&statuses)
339350
if err != nil {
340351
return nil, err
341352
}
@@ -352,42 +363,43 @@ func GetLatestCommitStatusForPairs(ctx context.Context, repoIDsToLatestCommitSHA
352363
// GetLatestCommitStatusForRepoCommitIDs returns all statuses with a unique context for a given list of repo-sha pairs
353364
func GetLatestCommitStatusForRepoCommitIDs(ctx context.Context, repoID int64, commitIDs []string) (map[string][]*CommitStatus, error) {
354365
type result struct {
355-
ID int64
356-
Sha string
366+
Index int64
367+
SHA string
357368
}
358369

370+
getBase := func() *xorm.Session {
371+
return db.GetEngine(ctx).Table(&CommitStatus{}).Where("repo_id = ?", repoID)
372+
}
359373
results := make([]result, 0, len(commitIDs))
360374

361-
sess := db.GetEngine(ctx).Table(&CommitStatus{})
362-
363-
// Create a disjunction of conditions for each repoID and SHA pair
364375
conds := make([]builder.Cond, 0, len(commitIDs))
365376
for _, sha := range commitIDs {
366377
conds = append(conds, builder.Eq{"sha": sha})
367378
}
368-
sess = sess.Where(builder.Eq{"repo_id": repoID}.And(builder.Or(conds...))).
369-
Select("max( id ) as id, sha").
370-
GroupBy("context_hash, sha").OrderBy("max( id ) desc")
379+
sess := getBase().And(builder.Or(conds...)).
380+
Select("max( `index` ) as `index`, sha").
381+
GroupBy("context_hash, sha").OrderBy("max( `index` ) desc")
371382

372383
err := sess.Find(&results)
373384
if err != nil {
374385
return nil, err
375386
}
376387

377-
ids := make([]int64, 0, len(results))
378388
repoStatuses := make(map[string][]*CommitStatus)
379-
for _, result := range results {
380-
ids = append(ids, result.ID)
381-
}
382389

383-
statuses := make([]*CommitStatus, 0, len(ids))
384-
if len(ids) > 0 {
385-
err = db.GetEngine(ctx).In("id", ids).Find(&statuses)
390+
if len(results) > 0 {
391+
statuses := make([]*CommitStatus, 0, len(results))
392+
393+
conds = make([]builder.Cond, 0, len(results))
394+
for _, result := range results {
395+
conds = append(conds, builder.Eq{"`index`": result.Index, "sha": result.SHA})
396+
}
397+
err = getBase().And(builder.Or(conds...)).Find(&statuses)
386398
if err != nil {
387399
return nil, err
388400
}
389401

390-
// Group the statuses by repo ID
402+
// Group the statuses by commit
391403
for _, status := range statuses {
392404
repoStatuses[status.SHA] = append(repoStatuses[status.SHA], status)
393405
}
@@ -398,22 +410,36 @@ func GetLatestCommitStatusForRepoCommitIDs(ctx context.Context, repoID int64, co
398410

399411
// FindRepoRecentCommitStatusContexts returns repository's recent commit status contexts
400412
func FindRepoRecentCommitStatusContexts(ctx context.Context, repoID int64, before time.Duration) ([]string, error) {
413+
type result struct {
414+
Index int64
415+
SHA string
416+
}
417+
getBase := func() *xorm.Session {
418+
return db.GetEngine(ctx).Table(&CommitStatus{}).Where("repo_id = ?", repoID)
419+
}
420+
401421
start := timeutil.TimeStampNow().AddDuration(-before)
402-
ids := make([]int64, 0, 10)
403-
if err := db.GetEngine(ctx).Table("commit_status").
404-
Where("repo_id = ?", repoID).
405-
And("updated_unix >= ?", start).
406-
Select("max( id ) as id").
407-
GroupBy("context_hash").OrderBy("max( id ) desc").
408-
Find(&ids); err != nil {
422+
results := make([]result, 0, 10)
423+
424+
sess := getBase().And("updated_unix >= ?", start).
425+
Select("max( `index` ) as `index`, sha").
426+
GroupBy("context_hash, sha").OrderBy("max( `index` ) desc")
427+
428+
err := sess.Find(&results)
429+
if err != nil {
409430
return nil, err
410431
}
411432

412-
contexts := make([]string, 0, len(ids))
413-
if len(ids) == 0 {
433+
contexts := make([]string, 0, len(results))
434+
if len(results) == 0 {
414435
return contexts, nil
415436
}
416-
return contexts, db.GetEngine(ctx).Select("context").Table("commit_status").In("id", ids).Find(&contexts)
437+
438+
conds := make([]builder.Cond, 0, len(results))
439+
for _, result := range results {
440+
conds = append(conds, builder.Eq{"`index`": result.Index, "sha": result.SHA})
441+
}
442+
return contexts, getBase().And(builder.Or(conds...)).Select("context").Find(&contexts)
417443
}
418444

419445
// NewCommitStatusOptions holds options for creating a CommitStatus

0 commit comments

Comments
 (0)