Skip to content

Commit c4b0bc4

Browse files
authored
shards: handle failed cached List for selectRepoSet (sourcegraph#864)
If we failed to List the repositories when loading a shard we would never search it due to selectRepoSet optimization. In practice this feels very rare to happen (only for logic error or disk corruption). However, in those cases we should surface these crashes searches by attempting to search the shard. Additionally I add logging so we can notice when this happens. I didn't add a metric since this is the sort of thing that I think is so rare we would never think to check the metric (but may notice logs). Note: I used the slightly tricky invariant that repos being nil means error. If the shard is actually empty (eg all repos tombstoned) then we still correctly apply the optimization. In practice having an empty shard shouldn't really happen so I'm open to just treating empty repos list as something we have to search.
1 parent c1295f9 commit c4b0bc4

File tree

1 file changed

+10
-4
lines changed

1 file changed

+10
-4
lines changed

shards/shards.go

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,8 @@ type rankedShard struct {
180180
// We have out of band ranking on compound shards which can change even if
181181
// the shard file does not. So we compute a rank in getShards. We store
182182
// repos here to avoid the cost of List in the search request path.
183+
//
184+
// repos is nil only if that call failed.
183185
repos []*zoekt.Repository
184186
}
185187

@@ -447,7 +449,13 @@ func doSelectRepoSet(shards []*rankedShard, and *query.And) ([]*rankedShard, que
447449
filteredAll := true
448450

449451
for _, s := range shards {
450-
if any, all := hasRepos(s.repos); any {
452+
if s.repos == nil {
453+
// repos is nil if we failed to List the shard. This shouldn't
454+
// happen, but if it does we don't know what is in it and must search
455+
// it without simplifying the query.
456+
filtered = append(filtered, s)
457+
filteredAll = false
458+
} else if any, all := hasRepos(s.repos); any {
451459
filtered = append(filtered, s)
452460
filteredAll = filteredAll && all
453461
}
@@ -1066,9 +1074,7 @@ func mkRankedShard(s zoekt.Searcher) *rankedShard {
10661074
q := query.Const{Value: true}
10671075
result, err := s.List(context.Background(), &q, nil)
10681076
if err != nil {
1069-
return &rankedShard{Searcher: s}
1070-
}
1071-
if len(result.Repos) == 0 {
1077+
log.Printf("mkRankedShard(%s): failed to cache repository list: %v", s, err)
10721078
return &rankedShard{Searcher: s}
10731079
}
10741080

0 commit comments

Comments
 (0)