Improve /api/v1/repos/issues/search by just getting repo ids (#15179)

/api/v1/repos/issues/search is a highly inefficient search which is unfortunately
the basis for our dependency searching algorithm. In particular it currently loads
all of the repositories and their owners and their primary coding language all of
which is immediately thrown away.

This PR makes one simple change - just get the IDs.

Related #14560
Related #12827

Signed-off-by: Andrew Thornton <art27@cantab.net>
This commit is contained in:
zeripath 2021-03-29 18:12:21 +01:00 committed by GitHub
parent 2b9e0b4d1b
commit c1ca4a8313
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 72 additions and 38 deletions

View file

@ -12,6 +12,7 @@ import (
"code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/modules/util"
"xorm.io/builder" "xorm.io/builder"
"xorm.io/xorm"
) )
// RepositoryListDefaultPageSize is the default number of repositories // RepositoryListDefaultPageSize is the default number of repositories
@ -363,6 +364,35 @@ func SearchRepository(opts *SearchRepoOptions) (RepositoryList, int64, error) {
// SearchRepositoryByCondition search repositories by condition // SearchRepositoryByCondition search repositories by condition
func SearchRepositoryByCondition(opts *SearchRepoOptions, cond builder.Cond, loadAttributes bool) (RepositoryList, int64, error) { func SearchRepositoryByCondition(opts *SearchRepoOptions, cond builder.Cond, loadAttributes bool) (RepositoryList, int64, error) {
sess, count, err := searchRepositoryByCondition(opts, cond)
if err != nil {
return nil, 0, err
}
defer sess.Close()
defaultSize := 50
if opts.PageSize > 0 {
defaultSize = opts.PageSize
}
repos := make(RepositoryList, 0, defaultSize)
if err := sess.Find(&repos); err != nil {
return nil, 0, fmt.Errorf("Repo: %v", err)
}
if opts.PageSize <= 0 {
count = int64(len(repos))
}
if loadAttributes {
if err := repos.loadAttributes(sess); err != nil {
return nil, 0, fmt.Errorf("LoadAttributes: %v", err)
}
}
return repos, count, nil
}
func searchRepositoryByCondition(opts *SearchRepoOptions, cond builder.Cond) (*xorm.Session, int64, error) {
if opts.Page <= 0 { if opts.Page <= 0 {
opts.Page = 1 opts.Page = 1
} }
@ -376,31 +406,24 @@ func SearchRepositoryByCondition(opts *SearchRepoOptions, cond builder.Cond, loa
} }
sess := x.NewSession() sess := x.NewSession()
defer sess.Close()
count, err := sess. var count int64
Where(cond). if opts.PageSize > 0 {
Count(new(Repository)) var err error
if err != nil { count, err = sess.
return nil, 0, fmt.Errorf("Count: %v", err) Where(cond).
Count(new(Repository))
if err != nil {
_ = sess.Close()
return nil, 0, fmt.Errorf("Count: %v", err)
}
} }
repos := make(RepositoryList, 0, opts.PageSize)
sess.Where(cond).OrderBy(opts.OrderBy.String()) sess.Where(cond).OrderBy(opts.OrderBy.String())
if opts.PageSize > 0 { if opts.PageSize > 0 {
sess.Limit(opts.PageSize, (opts.Page-1)*opts.PageSize) sess.Limit(opts.PageSize, (opts.Page-1)*opts.PageSize)
} }
if err = sess.Find(&repos); err != nil { return sess, count, nil
return nil, 0, fmt.Errorf("Repo: %v", err)
}
if loadAttributes {
if err = repos.loadAttributes(sess); err != nil {
return nil, 0, fmt.Errorf("LoadAttributes: %v", err)
}
}
return repos, count, nil
} }
// accessibleRepositoryCondition takes a user a returns a condition for checking if a repository is accessible // accessibleRepositoryCondition takes a user a returns a condition for checking if a repository is accessible
@ -456,6 +479,33 @@ func SearchRepositoryByName(opts *SearchRepoOptions) (RepositoryList, int64, err
return SearchRepository(opts) return SearchRepository(opts)
} }
// SearchRepositoryIDs takes keyword and part of repository name to search,
// it returns results in given range and number of total results.
func SearchRepositoryIDs(opts *SearchRepoOptions) ([]int64, int64, error) {
opts.IncludeDescription = false
cond := SearchRepositoryCondition(opts)
sess, count, err := searchRepositoryByCondition(opts, cond)
if err != nil {
return nil, 0, err
}
defer sess.Close()
defaultSize := 50
if opts.PageSize > 0 {
defaultSize = opts.PageSize
}
ids := make([]int64, 0, defaultSize)
err = sess.Select("id").Table("repository").Find(&ids)
if opts.PageSize <= 0 {
count = int64(len(ids))
}
return ids, count, err
}
// AccessibleRepoIDsQuery queries accessible repository ids. Usable as a subquery wherever repo ids need to be filtered. // AccessibleRepoIDsQuery queries accessible repository ids. Usable as a subquery wherever repo ids need to be filtered.
func AccessibleRepoIDsQuery(user *User) *builder.Builder { func AccessibleRepoIDsQuery(user *User) *builder.Builder {
// NB: Please note this code needs to still work if user is nil // NB: Please note this code needs to still work if user is nil

View file

@ -16,7 +16,6 @@ import (
"code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/context"
"code.gitea.io/gitea/modules/convert" "code.gitea.io/gitea/modules/convert"
issue_indexer "code.gitea.io/gitea/modules/indexer/issues" issue_indexer "code.gitea.io/gitea/modules/indexer/issues"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/notification" "code.gitea.io/gitea/modules/notification"
"code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/setting"
api "code.gitea.io/gitea/modules/structs" api "code.gitea.io/gitea/modules/structs"
@ -113,11 +112,7 @@ func SearchIssues(ctx *context.APIContext) {
} }
// find repos user can access (for issue search) // find repos user can access (for issue search)
repoIDs := make([]int64, 0)
opts := &models.SearchRepoOptions{ opts := &models.SearchRepoOptions{
ListOptions: models.ListOptions{
PageSize: 15,
},
Private: false, Private: false,
AllPublic: true, AllPublic: true,
TopicOnly: false, TopicOnly: false,
@ -132,21 +127,10 @@ func SearchIssues(ctx *context.APIContext) {
opts.AllLimited = true opts.AllLimited = true
} }
for page := 1; ; page++ { repoIDs, _, err := models.SearchRepositoryIDs(opts)
opts.Page = page if err != nil {
repos, count, err := models.SearchRepositoryByName(opts) ctx.Error(http.StatusInternalServerError, "SearchRepositoryByName", err)
if err != nil { return
ctx.Error(http.StatusInternalServerError, "SearchRepositoryByName", err)
return
}
if len(repos) == 0 {
break
}
log.Trace("Processing next %d repos of %d", len(repos), count)
for _, repo := range repos {
repoIDs = append(repoIDs, repo.ID)
}
} }
var issues []*models.Issue var issues []*models.Issue