convert commit/diff search to job (#25807)

This creates the type `commit.CommitSearch` which captures the minimal set
of validated arguments for commit and diff search and satisfies the
`run.Job` interface. This cuts out all dependency on
`search.TextParameters`, constructing the job directly from a search
query.

This is created in `doResults` instead of `toSearchInputs` because it
depends on having already resolved the list of repos.
This commit is contained in:
Camden Cheek 2021-10-08 09:07:42 -06:00 committed by GitHub
parent 2968296a6c
commit 20d09ac8ca
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 91 additions and 59 deletions

View File

@ -33,6 +33,7 @@ import (
"github.com/sourcegraph/sourcegraph/internal/honey"
"github.com/sourcegraph/sourcegraph/internal/rcache"
"github.com/sourcegraph/sourcegraph/internal/search"
"github.com/sourcegraph/sourcegraph/internal/search/commit"
"github.com/sourcegraph/sourcegraph/internal/search/filter"
"github.com/sourcegraph/sourcegraph/internal/search/query"
searchrepos "github.com/sourcegraph/sourcegraph/internal/search/repos"
@ -1602,28 +1603,61 @@ func (r *searchResolver) doResults(ctx context.Context, args *search.TextParamet
}
}
if args.ResultTypes.Has(result.TypeDiff) {
wg := waitGroup(args.ResultTypes.Without(result.TypeDiff) == 0)
wg.Add(1)
goroutine.Go(func() {
defer wg.Done()
_ = agg.DoDiffSearch(ctx, args)
})
if featureflag.FromContext(ctx).GetBoolOr("cc_commit_search", false) {
if args.ResultTypes.Has(result.TypeCommit) {
j, err := commit.NewSearchJob(args.Query, args.Repos, false, int(args.PatternInfo.FileMatchLimit))
if err != nil {
agg.Error(err)
} else {
jobs = append(jobs, j)
}
}
if args.ResultTypes.Has(result.TypeDiff) {
j, err := commit.NewSearchJob(args.Query, args.Repos, true, int(args.PatternInfo.FileMatchLimit))
if err != nil {
agg.Error(err)
} else {
jobs = append(jobs, j)
}
}
} else {
if args.ResultTypes.Has(result.TypeDiff) {
wg := waitGroup(args.ResultTypes.Without(result.TypeDiff) == 0)
wg.Add(1)
goroutine.Go(func() {
defer wg.Done()
_ = agg.DoDiffSearch(ctx, args)
})
}
if args.ResultTypes.Has(result.TypeCommit) {
wg := waitGroup(args.ResultTypes.Without(result.TypeCommit) == 0)
wg.Add(1)
goroutine.Go(func() {
defer wg.Done()
_ = agg.DoCommitSearch(ctx, args)
})
}
}
if args.ResultTypes.Has(result.TypeCommit) {
wg := waitGroup(args.ResultTypes.Without(result.TypeCommit) == 0)
wg.Add(1)
goroutine.Go(func() {
defer wg.Done()
_ = agg.DoCommitSearch(ctx, args)
})
wgForJob := func(job run.Job) *sync.WaitGroup {
switch job.Name() {
case "Diff":
return waitGroup(args.ResultTypes.Without(result.TypeDiff) == 0)
case "Commit":
return waitGroup(args.ResultTypes.Without(result.TypeCommit) == 0)
case "Structural":
return waitGroup(true)
default:
panic("unknown job name " + job.Name())
}
}
// Start all specific search jobs, if any.
for _, job := range jobs {
wg := waitGroup(true)
wg := wgForJob(job)
wg.Add(1)
goroutine.Go(func() {
defer wg.Done()

View File

@ -18,7 +18,6 @@ import (
"github.com/sourcegraph/sourcegraph/internal/database"
"github.com/sourcegraph/sourcegraph/internal/database/dbutil"
"github.com/sourcegraph/sourcegraph/internal/errcode"
"github.com/sourcegraph/sourcegraph/internal/featureflag"
"github.com/sourcegraph/sourcegraph/internal/search"
"github.com/sourcegraph/sourcegraph/internal/search/query"
"github.com/sourcegraph/sourcegraph/internal/search/repos"
@ -31,18 +30,6 @@ import (
// SearchCommitDiffsInRepos searches a set of repos for matching commit diffs.
func SearchCommitDiffsInRepos(ctx context.Context, db dbutil.DB, args *search.TextParametersForCommitParameters, resultChannel streaming.Sender) error {
if featureflag.FromContext(ctx).GetBoolOr("cc_commit_search", false) {
return searchInReposNew(ctx, db, args, searchCommitsInReposParameters{
TraceName: "SearchCommitDiffsInRepos",
ResultChannel: resultChannel,
CommitParams: search.CommitParameters{
PatternInfo: args.PatternInfo,
Query: args.Query,
Diff: true,
},
})
}
return searchInRepos(ctx, db, args, searchCommitsInReposParameters{
TraceName: "SearchCommitDiffsInRepos",
ResultChannel: resultChannel,
@ -61,19 +48,6 @@ func SearchCommitLogInRepos(ctx context.Context, db dbutil.DB, args *search.Text
terms = append(terms, args.PatternInfo.Pattern)
}
if featureflag.FromContext(ctx).GetBoolOr("cc_commit_search", false) {
return searchInReposNew(ctx, db, args, searchCommitsInReposParameters{
TraceName: "searchCommitLogsInRepos",
ResultChannel: resultChannel,
CommitParams: search.CommitParameters{
PatternInfo: args.PatternInfo,
Query: args.Query,
Diff: false,
ExtraMessageValues: terms,
},
})
}
return searchInRepos(ctx, db, args, searchCommitsInReposParameters{
TraceName: "searchCommitLogsInRepos",
ResultChannel: resultChannel,

View File

@ -10,7 +10,6 @@ import (
"golang.org/x/sync/errgroup"
"github.com/sourcegraph/sourcegraph/internal/conf"
"github.com/sourcegraph/sourcegraph/internal/database/dbutil"
"github.com/sourcegraph/sourcegraph/internal/gitserver"
"github.com/sourcegraph/sourcegraph/internal/gitserver/protocol"
gitprotocol "github.com/sourcegraph/sourcegraph/internal/gitserver/protocol"
@ -22,40 +21,42 @@ import (
"github.com/sourcegraph/sourcegraph/internal/vcs/git/gitapi"
)
func searchInReposNew(ctx context.Context, db dbutil.DB, textParams *search.TextParametersForCommitParameters, params searchCommitsInReposParameters) error {
type CommitSearch struct {
Query gitprotocol.Node
Repos []*search.RepositoryRevisions
Diff bool
Limit int
}
func (j CommitSearch) Run(ctx context.Context, stream streaming.Sender) error {
g, ctx := errgroup.WithContext(ctx)
for _, repoRev := range textParams.Repos {
for _, repoRev := range j.Repos {
// Skip the repo if no revisions were resolved for it
if len(repoRev.Revs) == 0 {
continue
}
rr := repoRev
query := params.CommitParams.Query
diff := params.CommitParams.Diff
limit := int(textParams.PatternInfo.FileMatchLimit)
args := &protocol.SearchRequest{
Repo: rr.Repo.Name,
Revisions: searchRevsToGitserverRevs(rr.Revs),
Query: gitprotocol.Reduce(gitprotocol.NewAnd(queryNodesToPredicates(query, query.IsCaseSensitive(), diff)...)),
IncludeDiff: diff,
Limit: limit,
Repo: repoRev.Repo.Name,
Revisions: searchRevsToGitserverRevs(repoRev.Revs),
Query: j.Query,
IncludeDiff: j.Diff,
Limit: j.Limit,
}
onMatches := func(in []protocol.CommitMatch) {
res := make([]result.Match, 0, len(in))
for _, protocolMatch := range in {
res = append(res, protocolMatchToCommitMatch(rr.Repo, diff, protocolMatch))
res = append(res, protocolMatchToCommitMatch(repoRev.Repo, j.Diff, protocolMatch))
}
params.ResultChannel.Send(streaming.SearchEvent{
stream.Send(streaming.SearchEvent{
Results: res,
})
}
g.Go(func() error {
limitHit, err := gitserver.DefaultClient.Search(ctx, args, onMatches)
params.ResultChannel.Send(streaming.SearchEvent{
stream.Send(streaming.SearchEvent{
Stats: streaming.Stats{
IsLimitHit: limitHit,
},
@ -63,10 +64,33 @@ func searchInReposNew(ctx context.Context, db dbutil.DB, textParams *search.Text
return err
})
}
return g.Wait()
}
func (j CommitSearch) Name() string {
if j.Diff {
return "Diff"
}
return "Commit"
}
func NewSearchJob(q query.Q, repos []*search.RepositoryRevisions, diff bool, limit int) (*CommitSearch, error) {
resultType := "commit"
if diff {
resultType = "diff"
}
if err := CheckSearchLimits(q, len(repos), resultType); err != nil {
return nil, err
}
return &CommitSearch{
Query: gitprotocol.NewAnd(queryNodesToPredicates(q, q.IsCaseSensitive(), diff)...),
Repos: repos,
Diff: diff,
Limit: limit,
}, nil
}
func searchRevsToGitserverRevs(in []search.RevisionSpecifier) []gitprotocol.RevisionSpecifier {
out := make([]gitprotocol.RevisionSpecifier, 0, len(in))
for _, rev := range in {