Skip to content

Commit 49f224a

Browse files
committed
fix: improve performance of SyncBranchesToDB
1 parent 75950d0 commit 49f224a

File tree

3 files changed

+114
-50
lines changed

3 files changed

+114
-50
lines changed

models/git/branch.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,11 @@ func GetBranch(ctx context.Context, repoID int64, branchName string) (*Branch, e
158158
return &branch, nil
159159
}
160160

161+
func GetBranches(ctx context.Context, repoID int64, branchNames []string) ([]*Branch, error) {
162+
branches := make([]*Branch, 0, len(branchNames))
163+
return branches, db.GetEngine(ctx).Where("repo_id=?", repoID).In("name", branchNames).Find(&branches)
164+
}
165+
161166
func AddBranches(ctx context.Context, branches []*Branch) error {
162167
for _, branch := range branches {
163168
if _, err := db.GetEngine(ctx).Insert(branch); err != nil {

routers/private/hook_post_receive.go

Lines changed: 28 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,7 @@ func HookPostReceive(ctx *gitea_context.PrivateContext) {
9595
return
9696
}
9797

98+
var branchesToSync = make([]*repo_module.PushUpdateOptions, 0, len(updates))
9899
for _, update := range updates {
99100
if !update.RefFullName.IsBranch() {
100101
continue
@@ -116,31 +117,39 @@ func HookPostReceive(ctx *gitea_context.PrivateContext) {
116117
return
117118
}
118119
} else {
119-
if gitRepo == nil {
120-
var err error
121-
gitRepo, err = gitrepo.OpenRepository(ctx, repo)
122-
if err != nil {
123-
log.Error("Failed to open repository: %s/%s Error: %v", ownerName, repoName, err)
124-
ctx.JSON(http.StatusInternalServerError, private.HookPostReceiveResult{
125-
Err: fmt.Sprintf("Failed to open repository: %s/%s Error: %v", ownerName, repoName, err),
126-
})
127-
return
128-
}
129-
}
130-
commit, err := gitRepo.GetCommit(update.NewCommitID)
120+
branchesToSync = append(branchesToSync, update)
121+
}
122+
}
123+
if len(branchesToSync) > 0 {
124+
if gitRepo == nil {
125+
var err error
126+
gitRepo, err = gitrepo.OpenRepository(ctx, repo)
131127
if err != nil {
128+
log.Error("Failed to open repository: %s/%s Error: %v", ownerName, repoName, err)
132129
ctx.JSON(http.StatusInternalServerError, private.HookPostReceiveResult{
133-
Err: fmt.Sprintf("Failed to get commit %s in repository: %s/%s Error: %v", update.NewCommitID, ownerName, repoName, err),
134-
})
135-
return
136-
}
137-
if err := repo_service.SyncBranchToDB(ctx, repo.ID, update.PusherID, update.RefFullName.BranchName(), commit); err != nil {
138-
ctx.JSON(http.StatusInternalServerError, private.HookPostReceiveResult{
139-
Err: fmt.Sprintf("Failed to sync branch to DB in repository: %s/%s Error: %v", ownerName, repoName, err),
130+
Err: fmt.Sprintf("Failed to open repository: %s/%s Error: %v", ownerName, repoName, err),
140131
})
141132
return
142133
}
143134
}
135+
136+
var (
137+
branchNames = make([]string, 0, len(branchesToSync))
138+
commitIDs = make([]string, 0, len(branchesToSync))
139+
)
140+
for _, update := range branchesToSync {
141+
branchNames = append(branchNames, update.RefFullName.BranchName())
142+
commitIDs = append(commitIDs, update.NewCommitID)
143+
}
144+
145+
if err := repo_service.SyncBranchesToDB(ctx, repo.ID, opts.UserID, branchNames, commitIDs, func(commitID string) (*git.Commit, error) {
146+
return gitRepo.GetCommit(commitID)
147+
}); err != nil {
148+
ctx.JSON(http.StatusInternalServerError, private.HookPostReceiveResult{
149+
Err: fmt.Sprintf("Failed to sync branch to DB in repository: %s/%s Error: %v", ownerName, repoName, err),
150+
})
151+
return
152+
}
144153
}
145154
}
146155

services/repository/branch.go

Lines changed: 81 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -222,44 +222,94 @@ func checkBranchName(ctx context.Context, repo *repo_model.Repository, name stri
222222
return err
223223
}
224224

225-
// SyncBranchToDB sync the branch information in the database. It will try to update the branch first,
225+
// SyncBranchesToDB sync the branch information in the database. It will try to update the branch first,
226226
// if updated success with affect records > 0, then all are done. Because that means the branch has been in the database.
227227
// If no record is affected, that means the branch does not exist in database. So there are two possibilities.
228228
// One is this is a new branch, then we just need to insert the record. Another is the branches haven't been synced,
229229
// then we need to sync all the branches into database.
230-
func SyncBranchToDB(ctx context.Context, repoID, pusherID int64, branchName string, commit *git.Commit) error {
231-
cnt, err := git_model.UpdateBranch(ctx, repoID, pusherID, branchName, commit)
232-
if err != nil {
233-
return fmt.Errorf("git_model.UpdateBranch %d:%s failed: %v", repoID, branchName, err)
234-
}
235-
if cnt > 0 { // This means branch does exist, so it's a normal update. It also means the branch has been synced.
236-
return nil
237-
}
230+
func SyncBranchesToDB(ctx context.Context, repoID, pusherID int64, branchNames []string, commitIDs []string, getCommit func(commitID string) (*git.Commit, error)) error {
231+
// Some designs that make the code look strange but are made for performance optimization purposes:
232+
// 1. Sync branches in a batch to reduce the number of DB queries.
233+
// 2. Lazy load commit information since it may be not necessary.
234+
// 3. Exit early if synced all branches of git repo when there's no branch in DB.
235+
// 4. Check the branches in DB if they are already synced.
236+
//
237+
// If the user pushes many branches at once, the Git hook will call the internal API in batches, rather than all at once.
238+
// See https://github.com/go-gitea/gitea/blob/cb52b17f92e2d2293f7c003649743464492bca48/cmd/hook.go#L27
239+
// For the first batch, it will hit optimization 3.
240+
// For other batches, it will hit optimization 4.
241+
242+
if len(branchNames) != len(commitIDs) {
243+
return fmt.Errorf("branchNames and commitIDs length not match")
244+
}
245+
246+
return db.WithTx(ctx, func(ctx context.Context) error {
247+
branches, err := git_model.GetBranches(ctx, repoID, branchNames)
248+
if err != nil {
249+
return fmt.Errorf("git_model.GetBranches: %v", err)
250+
}
251+
branchMap := make(map[string]*git_model.Branch, len(branches))
252+
for _, branch := range branches {
253+
branchMap[branch.Name] = branch
254+
}
238255

239-
// if user haven't visit UI but directly push to a branch after upgrading from 1.20 -> 1.21,
240-
// we cannot simply insert the branch but need to check we have branches or not
241-
hasBranch, err := db.Exist[git_model.Branch](ctx, git_model.FindBranchOptions{
242-
RepoID: repoID,
243-
IsDeletedBranch: optional.Some(false),
244-
}.ToConds())
245-
if err != nil {
246-
return err
247-
}
248-
if !hasBranch {
249-
if _, err = repo_module.SyncRepoBranches(ctx, repoID, pusherID); err != nil {
250-
return fmt.Errorf("repo_module.SyncRepoBranches %d:%s failed: %v", repoID, branchName, err)
256+
hasBranch := len(branches) > 0
257+
258+
newBranches := make([]*git_model.Branch, 0, len(branchNames))
259+
260+
for i, branchName := range branchNames {
261+
commitID := commitIDs[i]
262+
if branch, ok := branchMap[branchName]; ok && branch.CommitID == commitID {
263+
continue
264+
}
265+
266+
commit, err := getCommit(branchName)
267+
if err != nil {
268+
return fmt.Errorf("get commit of %s failed: %v", branchName, err)
269+
}
270+
271+
cnt, err := git_model.UpdateBranch(ctx, repoID, pusherID, branchName, commit)
272+
if err != nil {
273+
return fmt.Errorf("git_model.UpdateBranch %d:%s failed: %v", repoID, branchName, err)
274+
}
275+
if cnt > 0 { // This means branch does exist, so it's a normal update. It also means the branch has been synced.
276+
hasBranch = true
277+
continue
278+
}
279+
280+
if !hasBranch {
281+
// if user haven't visit UI but directly push to a branch after upgrading from 1.20 -> 1.21,
282+
// we cannot simply insert the branch but need to check we have branches or not
283+
hasBranch, err = db.Exist[git_model.Branch](ctx, git_model.FindBranchOptions{
284+
RepoID: repoID,
285+
IsDeletedBranch: optional.Some(false),
286+
}.ToConds())
287+
if err != nil {
288+
return err
289+
}
290+
if !hasBranch {
291+
if _, err = repo_module.SyncRepoBranches(ctx, repoID, pusherID); err != nil {
292+
return fmt.Errorf("repo_module.SyncRepoBranches %d:%s failed: %v", repoID, branchName, err)
293+
}
294+
return nil
295+
}
296+
}
297+
298+
// if database have branches but not this branch, it means this is a new branch
299+
newBranches = append(newBranches, &git_model.Branch{
300+
RepoID: repoID,
301+
Name: branchName,
302+
CommitID: commit.ID.String(),
303+
CommitMessage: commit.Summary(),
304+
PusherID: pusherID,
305+
CommitTime: timeutil.TimeStamp(commit.Committer.When.Unix()),
306+
})
251307
}
252-
return nil
253-
}
254308

255-
// if database have branches but not this branch, it means this is a new branch
256-
return db.Insert(ctx, &git_model.Branch{
257-
RepoID: repoID,
258-
Name: branchName,
259-
CommitID: commit.ID.String(),
260-
CommitMessage: commit.Summary(),
261-
PusherID: pusherID,
262-
CommitTime: timeutil.TimeStamp(commit.Committer.When.Unix()),
309+
if len(newBranches) > 0 {
310+
return db.Insert(ctx, newBranches)
311+
}
312+
return nil
263313
})
264314
}
265315

0 commit comments

Comments
 (0)