Skip to content

Commit

Permalink
shards: handle failed cached List for selectRepoSet (#864)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
keegancsmith authored Nov 21, 2024
1 parent c1295f9 commit c4b0bc4
Showing 1 changed file with 10 additions and 4 deletions.
14 changes: 10 additions & 4 deletions shards/shards.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,8 @@ type rankedShard struct {
// We have out of band ranking on compound shards which can change even if
// the shard file does not. So we compute a rank in getShards. We store
// repos here to avoid the cost of List in the search request path.
//
// repos is nil only if that call failed.
repos []*zoekt.Repository
}

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

for _, s := range shards {
if any, all := hasRepos(s.repos); any {
if s.repos == nil {
// repos is nil if we failed to List the shard. This shouldn't
// happen, but if it does we don't know what is in it and must search
// it without simplifying the query.
filtered = append(filtered, s)
filteredAll = false
} else if any, all := hasRepos(s.repos); any {
filtered = append(filtered, s)
filteredAll = filteredAll && all
}
Expand Down Expand Up @@ -1066,9 +1074,7 @@ func mkRankedShard(s zoekt.Searcher) *rankedShard {
q := query.Const{Value: true}
result, err := s.List(context.Background(), &q, nil)
if err != nil {
return &rankedShard{Searcher: s}
}
if len(result.Repos) == 0 {
log.Printf("mkRankedShard(%s): failed to cache repository list: %v", s, err)
return &rankedShard{Searcher: s}
}

Expand Down

0 comments on commit c4b0bc4

Please sign in to comment.