From fc34481d054a9324ea4654dc721e54e2f608ac17 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 12 Apr 2024 09:41:50 +0800 Subject: [PATCH] Add commit status summary table to reduce query from commit status table (#30223) This PR adds a new table named commit status summary to reduce queries from the commit status table. After this change, commit status summary table will be used for the final result, commit status table will be for details. --------- Co-authored-by: Jason Song --- models/git/commit_status.go | 21 ++--- models/git/commit_status_summary.go | 84 +++++++++++++++++++ models/migrations/migrations.go | 3 + models/migrations/v1_23/v295.go | 18 ++++ services/actions/commit_status.go | 20 ++--- .../repository/commitstatus/commitstatus.go | 48 +++++++++-- tests/integration/pull_status_test.go | 7 ++ 7 files changed, 170 insertions(+), 31 deletions(-) create mode 100644 models/git/commit_status_summary.go create mode 100644 models/migrations/v1_23/v295.go diff --git a/models/git/commit_status.go b/models/git/commit_status.go index bb75dcca26..c3cda7b73d 100644 --- a/models/git/commit_status.go +++ b/models/git/commit_status.go @@ -292,30 +292,27 @@ func GetLatestCommitStatus(ctx context.Context, repoID int64, sha string, listOp } // 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) { +func GetLatestCommitStatusForPairs(ctx context.Context, repoSHAs []RepoSHA) (map[int64][]*CommitStatus, error) { type result struct { Index int64 RepoID int64 + SHA string } - results := make([]result, 0, len(repoIDsToLatestCommitSHAs)) + results := make([]result, 0, len(repoSHAs)) 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}) + conds := make([]builder.Cond, 0, len(repoSHAs)) + for _, repoSHA := range repoSHAs { + conds = append(conds, builder.Eq{"repo_id": repoSHA.RepoID, "sha": repoSHA.SHA}) } 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) - } + Select("max( `index` ) as `index`, repo_id, sha"). + GroupBy("context_hash, repo_id, sha").OrderBy("max( `index` ) desc") err := sess.Find(&results) if err != nil { @@ -332,7 +329,7 @@ func GetLatestCommitStatusForPairs(ctx context.Context, repoIDsToLatestCommitSHA cond := builder.Eq{ "`index`": result.Index, "repo_id": result.RepoID, - "sha": repoIDsToLatestCommitSHAs[result.RepoID], + "sha": result.SHA, } conds = append(conds, cond) } diff --git a/models/git/commit_status_summary.go b/models/git/commit_status_summary.go new file mode 100644 index 0000000000..01674e943d --- /dev/null +++ b/models/git/commit_status_summary.go @@ -0,0 +1,84 @@ +// Copyright 2024 Gitea. All rights reserved. +// SPDX-License-Identifier: MIT + +package git + +import ( + "context" + + "code.gitea.io/gitea/models/db" + "code.gitea.io/gitea/modules/setting" + api "code.gitea.io/gitea/modules/structs" + + "xorm.io/builder" +) + +// CommitStatusSummary holds the latest commit Status of a single Commit +type CommitStatusSummary struct { + ID int64 `xorm:"pk autoincr"` + RepoID int64 `xorm:"INDEX UNIQUE(repo_id_sha)"` + SHA string `xorm:"VARCHAR(64) NOT NULL INDEX UNIQUE(repo_id_sha)"` + State api.CommitStatusState `xorm:"VARCHAR(7) NOT NULL"` +} + +func init() { + db.RegisterModel(new(CommitStatusSummary)) +} + +type RepoSHA struct { + RepoID int64 + SHA string +} + +func GetLatestCommitStatusForRepoAndSHAs(ctx context.Context, repoSHAs []RepoSHA) ([]*CommitStatus, error) { + cond := builder.NewCond() + for _, rs := range repoSHAs { + cond = cond.Or(builder.Eq{"repo_id": rs.RepoID, "sha": rs.SHA}) + } + + var summaries []CommitStatusSummary + if err := db.GetEngine(ctx).Where(cond).Find(&summaries); err != nil { + return nil, err + } + + commitStatuses := make([]*CommitStatus, 0, len(repoSHAs)) + for _, summary := range summaries { + commitStatuses = append(commitStatuses, &CommitStatus{ + RepoID: summary.RepoID, + SHA: summary.SHA, + State: summary.State, + }) + } + return commitStatuses, nil +} + +func UpdateCommitStatusSummary(ctx context.Context, repoID int64, sha string) error { + commitStatuses, _, err := GetLatestCommitStatus(ctx, repoID, sha, db.ListOptionsAll) + if err != nil { + return err + } + state := CalcCommitStatus(commitStatuses) + // mysql will return 0 when update a record which state hasn't been changed which behaviour is different from other database, + // so we need to use insert in on duplicate + if setting.Database.Type.IsMySQL() { + _, err := db.GetEngine(ctx).Exec("INSERT INTO commit_status_summary (repo_id,sha,state) VALUES (?,?,?) ON DUPLICATE KEY UPDATE state=?", + repoID, sha, state.State, state.State) + return err + } + + if cnt, err := db.GetEngine(ctx).Where("repo_id=? AND sha=?", repoID, sha). + Cols("state"). + Update(&CommitStatusSummary{ + State: state.State, + }); err != nil { + return err + } else if cnt == 0 { + _, err = db.GetEngine(ctx).Insert(&CommitStatusSummary{ + RepoID: repoID, + SHA: sha, + State: state.State, + }) + return err + } + return nil +} diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 387cd96a53..3ea8f2acbf 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -576,7 +576,10 @@ var migrations = []Migration{ // Gitea 1.22.0 ends at 294 + // v294 -> v295 NewMigration("Add unique index for project issue table", v1_23.AddUniqueIndexForProjectIssue), + // v295 -> v296 + NewMigration("Add commit status summary table", v1_23.AddCommitStatusSummary), } // GetCurrentDBVersion returns the current db version diff --git a/models/migrations/v1_23/v295.go b/models/migrations/v1_23/v295.go new file mode 100644 index 0000000000..9a2003cfc1 --- /dev/null +++ b/models/migrations/v1_23/v295.go @@ -0,0 +1,18 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package v1_23 //nolint + +import "xorm.io/xorm" + +func AddCommitStatusSummary(x *xorm.Engine) error { + type CommitStatusSummary struct { + ID int64 `xorm:"pk autoincr"` + RepoID int64 `xorm:"INDEX UNIQUE(repo_id_sha)"` + SHA string `xorm:"VARCHAR(64) NOT NULL INDEX UNIQUE(repo_id_sha)"` + State string `xorm:"VARCHAR(7) NOT NULL"` + } + // there is no migrations because if there is no data on this table, it will fall back to get data + // from commit status + return x.Sync2(new(CommitStatusSummary)) +} diff --git a/services/actions/commit_status.go b/services/actions/commit_status.go index 4236553927..eb031511f6 100644 --- a/services/actions/commit_status.go +++ b/services/actions/commit_status.go @@ -16,6 +16,7 @@ import ( "code.gitea.io/gitea/modules/log" api "code.gitea.io/gitea/modules/structs" webhook_module "code.gitea.io/gitea/modules/webhook" + commitstatus_service "code.gitea.io/gitea/services/repository/commitstatus" "github.com/nektos/act/pkg/jobparser" ) @@ -122,18 +123,13 @@ func createCommitStatus(ctx context.Context, job *actions_model.ActionRunJob) er if err != nil { return fmt.Errorf("HashTypeInterfaceFromHashString: %w", err) } - if err := git_model.NewCommitStatus(ctx, git_model.NewCommitStatusOptions{ - Repo: repo, - SHA: commitID, - Creator: creator, - CommitStatus: &git_model.CommitStatus{ - SHA: sha, - TargetURL: fmt.Sprintf("%s/jobs/%d", run.Link(), index), - Description: description, - Context: ctxname, - CreatorID: creator.ID, - State: state, - }, + if err := commitstatus_service.CreateCommitStatus(ctx, repo, creator, commitID.String(), &git_model.CommitStatus{ + SHA: sha, + TargetURL: fmt.Sprintf("%s/jobs/%d", run.Link(), index), + Description: description, + Context: ctxname, + CreatorID: creator.ID, + State: state, }); err != nil { return fmt.Errorf("NewCommitStatus: %w", err) } diff --git a/services/repository/commitstatus/commitstatus.go b/services/repository/commitstatus/commitstatus.go index 145fc7d53c..167a5330dd 100644 --- a/services/repository/commitstatus/commitstatus.go +++ b/services/repository/commitstatus/commitstatus.go @@ -7,6 +7,7 @@ import ( "context" "crypto/sha256" "fmt" + "slices" "code.gitea.io/gitea/models/db" git_model "code.gitea.io/gitea/models/git" @@ -59,13 +60,19 @@ func CreateCommitStatus(ctx context.Context, repo *repo_model.Repository, creato sha = commit.ID.String() } - if err := git_model.NewCommitStatus(ctx, git_model.NewCommitStatusOptions{ - Repo: repo, - Creator: creator, - SHA: commit.ID, - CommitStatus: status, + if err := db.WithTx(ctx, func(ctx context.Context) error { + if err := git_model.NewCommitStatus(ctx, git_model.NewCommitStatusOptions{ + Repo: repo, + Creator: creator, + SHA: commit.ID, + CommitStatus: status, + }); err != nil { + return fmt.Errorf("NewCommitStatus[repo_id: %d, user_id: %d, sha: %s]: %w", repo.ID, creator.ID, sha, err) + } + + return git_model.UpdateCommitStatusSummary(ctx, repo.ID, commit.ID.String()) }); err != nil { - return fmt.Errorf("NewCommitStatus[repo_id: %d, user_id: %d, sha: %s]: %w", repo.ID, creator.ID, sha, err) + return err } defaultBranchCommit, err := gitRepo.GetBranchCommit(repo.DefaultBranch) @@ -114,8 +121,35 @@ func FindReposLastestCommitStatuses(ctx context.Context, repos []*repo_model.Rep return nil, fmt.Errorf("FindBranchesByRepoAndBranchName: %v", err) } + var repoSHAs []git_model.RepoSHA + for id, sha := range repoIDsToLatestCommitSHAs { + repoSHAs = append(repoSHAs, git_model.RepoSHA{RepoID: id, SHA: sha}) + } + + summaryResults, err := git_model.GetLatestCommitStatusForRepoAndSHAs(ctx, repoSHAs) + if err != nil { + return nil, fmt.Errorf("GetLatestCommitStatusForRepoAndSHAs: %v", err) + } + + for _, summary := range summaryResults { + for i, repo := range repos { + if repo.ID == summary.RepoID { + results[i] = summary + _ = slices.DeleteFunc(repoSHAs, func(repoSHA git_model.RepoSHA) bool { + return repoSHA.RepoID == repo.ID + }) + if results[i].State != "" { + if err := updateCommitStatusCache(ctx, repo.ID, repo.DefaultBranch, results[i].State); err != nil { + log.Error("updateCommitStatusCache[%d:%s] failed: %v", repo.ID, repo.DefaultBranch, err) + } + } + break + } + } + } + // call the database O(1) times to get the commit statuses for all repos - repoToItsLatestCommitStatuses, err := git_model.GetLatestCommitStatusForPairs(ctx, repoIDsToLatestCommitSHAs, db.ListOptionsAll) + repoToItsLatestCommitStatuses, err := git_model.GetLatestCommitStatusForPairs(ctx, repoSHAs) if err != nil { return nil, fmt.Errorf("GetLatestCommitStatusForPairs: %v", err) } diff --git a/tests/integration/pull_status_test.go b/tests/integration/pull_status_test.go index 26c99e6445..bb7098e424 100644 --- a/tests/integration/pull_status_test.go +++ b/tests/integration/pull_status_test.go @@ -12,6 +12,9 @@ import ( "testing" auth_model "code.gitea.io/gitea/models/auth" + git_model "code.gitea.io/gitea/models/git" + repo_model "code.gitea.io/gitea/models/repo" + "code.gitea.io/gitea/models/unittest" api "code.gitea.io/gitea/modules/structs" "github.com/stretchr/testify/assert" @@ -90,6 +93,10 @@ func TestPullCreate_CommitStatus(t *testing.T) { assert.True(t, ok) assert.Contains(t, cls, statesIcons[status]) } + + repo1 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{OwnerName: "user1", Name: "repo1"}) + css := unittest.AssertExistsAndLoadBean(t, &git_model.CommitStatusSummary{RepoID: repo1.ID, SHA: commitID}) + assert.EqualValues(t, api.CommitStatusWarning, css.State) }) }