From 7443a10fc3d722d3326a0cb7b15b208f907c72d7 Mon Sep 17 00:00:00 2001 From: YR Chen Date: Thu, 28 Mar 2024 16:01:15 +0800 Subject: [PATCH] Move from `max( id )` to `max( index )` for latest commit statuses (#30076) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- models/git/commit_status.go | 120 ++++++++++++++++++++++-------------- 1 file changed, 73 insertions(+), 47 deletions(-) diff --git a/models/git/commit_status.go b/models/git/commit_status.go index 2d1d1bcb06d..bb75dcca263 100644 --- a/models/git/commit_status.go +++ b/models/git/commit_status.go @@ -25,6 +25,7 @@ import ( "code.gitea.io/gitea/modules/translation" "xorm.io/builder" + "xorm.io/xorm" ) // CommitStatus holds a single Status of a single Commit @@ -269,44 +270,48 @@ type CommitStatusIndex struct { // GetLatestCommitStatus returns all statuses with a unique context for a given commit. func GetLatestCommitStatus(ctx context.Context, repoID int64, sha string, listOptions db.ListOptions) ([]*CommitStatus, int64, error) { - ids := make([]int64, 0, 10) - sess := db.GetEngine(ctx).Table(&CommitStatus{}). - Where("repo_id = ?", repoID).And("sha = ?", sha). - Select("max( id ) as id"). - GroupBy("context_hash").OrderBy("max( id ) desc") + getBase := func() *xorm.Session { + return db.GetEngine(ctx).Table(&CommitStatus{}). + Where("repo_id = ?", repoID).And("sha = ?", sha) + } + indices := make([]int64, 0, 10) + sess := getBase().Select("max( `index` ) as `index`"). + GroupBy("context_hash").OrderBy("max( `index` ) desc") if !listOptions.IsListAll() { sess = db.SetSessionPagination(sess, &listOptions) } - count, err := sess.FindAndCount(&ids) + count, err := sess.FindAndCount(&indices) if err != nil { return nil, count, err } - statuses := make([]*CommitStatus, 0, len(ids)) - if len(ids) == 0 { + statuses := make([]*CommitStatus, 0, len(indices)) + if len(indices) == 0 { return statuses, count, nil } - return statuses, count, db.GetEngine(ctx).In("id", ids).Find(&statuses) + return statuses, count, getBase().And(builder.In("`index`", indices)).Find(&statuses) } // GetLatestCommitStatusForPairs returns all statuses with a unique context for a given list of repo-sha pairs func GetLatestCommitStatusForPairs(ctx context.Context, repoIDsToLatestCommitSHAs map[int64]string, listOptions db.ListOptions) (map[int64][]*CommitStatus, error) { type result struct { - ID int64 + Index int64 RepoID int64 } results := make([]result, 0, len(repoIDsToLatestCommitSHAs)) - sess := db.GetEngine(ctx).Table(&CommitStatus{}) + getBase := func() *xorm.Session { + return db.GetEngine(ctx).Table(&CommitStatus{}) + } // Create a disjunction of conditions for each repoID and SHA pair conds := make([]builder.Cond, 0, len(repoIDsToLatestCommitSHAs)) for repoID, sha := range repoIDsToLatestCommitSHAs { conds = append(conds, builder.Eq{"repo_id": repoID, "sha": sha}) } - sess = sess.Where(builder.Or(conds...)). - Select("max( id ) as id, repo_id"). - GroupBy("context_hash, repo_id").OrderBy("max( id ) desc") + sess := getBase().Where(builder.Or(conds...)). + Select("max( `index` ) as `index`, repo_id"). + GroupBy("context_hash, repo_id").OrderBy("max( `index` ) desc") if !listOptions.IsListAll() { sess = db.SetSessionPagination(sess, &listOptions) @@ -317,15 +322,21 @@ func GetLatestCommitStatusForPairs(ctx context.Context, repoIDsToLatestCommitSHA return nil, err } - ids := make([]int64, 0, len(results)) repoStatuses := make(map[int64][]*CommitStatus) - for _, result := range results { - ids = append(ids, result.ID) - } - statuses := make([]*CommitStatus, 0, len(ids)) - if len(ids) > 0 { - err = db.GetEngine(ctx).In("id", ids).Find(&statuses) + if len(results) > 0 { + statuses := make([]*CommitStatus, 0, len(results)) + + conds = make([]builder.Cond, 0, len(results)) + for _, result := range results { + cond := builder.Eq{ + "`index`": result.Index, + "repo_id": result.RepoID, + "sha": repoIDsToLatestCommitSHAs[result.RepoID], + } + conds = append(conds, cond) + } + err = getBase().Where(builder.Or(conds...)).Find(&statuses) if err != nil { return nil, err } @@ -342,42 +353,43 @@ func GetLatestCommitStatusForPairs(ctx context.Context, repoIDsToLatestCommitSHA // GetLatestCommitStatusForRepoCommitIDs returns all statuses with a unique context for a given list of repo-sha pairs func GetLatestCommitStatusForRepoCommitIDs(ctx context.Context, repoID int64, commitIDs []string) (map[string][]*CommitStatus, error) { type result struct { - ID int64 - Sha string + Index int64 + SHA string } + getBase := func() *xorm.Session { + return db.GetEngine(ctx).Table(&CommitStatus{}).Where("repo_id = ?", repoID) + } results := make([]result, 0, len(commitIDs)) - sess := db.GetEngine(ctx).Table(&CommitStatus{}) - - // Create a disjunction of conditions for each repoID and SHA pair conds := make([]builder.Cond, 0, len(commitIDs)) for _, sha := range commitIDs { conds = append(conds, builder.Eq{"sha": sha}) } - sess = sess.Where(builder.Eq{"repo_id": repoID}.And(builder.Or(conds...))). - Select("max( id ) as id, sha"). - GroupBy("context_hash, sha").OrderBy("max( id ) desc") + sess := getBase().And(builder.Or(conds...)). + Select("max( `index` ) as `index`, sha"). + GroupBy("context_hash, sha").OrderBy("max( `index` ) desc") err := sess.Find(&results) if err != nil { return nil, err } - ids := make([]int64, 0, len(results)) repoStatuses := make(map[string][]*CommitStatus) - for _, result := range results { - ids = append(ids, result.ID) - } - statuses := make([]*CommitStatus, 0, len(ids)) - if len(ids) > 0 { - err = db.GetEngine(ctx).In("id", ids).Find(&statuses) + if len(results) > 0 { + statuses := make([]*CommitStatus, 0, len(results)) + + conds = make([]builder.Cond, 0, len(results)) + for _, result := range results { + conds = append(conds, builder.Eq{"`index`": result.Index, "sha": result.SHA}) + } + err = getBase().And(builder.Or(conds...)).Find(&statuses) if err != nil { return nil, err } - // Group the statuses by repo ID + // Group the statuses by commit for _, status := range statuses { repoStatuses[status.SHA] = append(repoStatuses[status.SHA], status) } @@ -388,22 +400,36 @@ func GetLatestCommitStatusForRepoCommitIDs(ctx context.Context, repoID int64, co // FindRepoRecentCommitStatusContexts returns repository's recent commit status contexts func FindRepoRecentCommitStatusContexts(ctx context.Context, repoID int64, before time.Duration) ([]string, error) { + type result struct { + Index int64 + SHA string + } + getBase := func() *xorm.Session { + return db.GetEngine(ctx).Table(&CommitStatus{}).Where("repo_id = ?", repoID) + } + start := timeutil.TimeStampNow().AddDuration(-before) - ids := make([]int64, 0, 10) - if err := db.GetEngine(ctx).Table("commit_status"). - Where("repo_id = ?", repoID). - And("updated_unix >= ?", start). - Select("max( id ) as id"). - GroupBy("context_hash").OrderBy("max( id ) desc"). - Find(&ids); err != nil { + results := make([]result, 0, 10) + + sess := getBase().And("updated_unix >= ?", start). + Select("max( `index` ) as `index`, sha"). + GroupBy("context_hash, sha").OrderBy("max( `index` ) desc") + + err := sess.Find(&results) + if err != nil { return nil, err } - contexts := make([]string, 0, len(ids)) - if len(ids) == 0 { + contexts := make([]string, 0, len(results)) + if len(results) == 0 { return contexts, nil } - return contexts, db.GetEngine(ctx).Select("context").Table("commit_status").In("id", ids).Find(&contexts) + + conds := make([]builder.Cond, 0, len(results)) + for _, result := range results { + conds = append(conds, builder.Eq{"`index`": result.Index, "sha": result.SHA}) + } + return contexts, getBase().And(builder.Or(conds...)).Select("context").Find(&contexts) } // NewCommitStatusOptions holds options for creating a CommitStatus