shards: populate RepoList.Stats.Repos (#605)

keegancsmith · web-flow · commit 7078a585a9e9 · 2023-06-23T13:15:05.000+02:00
Previously this was only populated by the web package for serving a
specific endpoint. We now populate it in all places we do aggregation.

Test Plan: updated unit tests. Additionally loaded zoekt-webserver then
added to the index a repository which required 2 shards to index. The
repository count shown only increased by 1.
diff --git a/api.go b/api.go
@@ -684,6 +684,9 @@ type IndexMetadata struct {
 // Statistics of a (collection of) repositories.
 type RepoStats struct {
 	// Repos is used for aggregrating the number of repositories.
+	//
+	// Note: This field is not populated on RepoListEntry.Stats (individual) but
+	// only for RepoList.Stats (aggregate).
 	Repos int
 
 	// Shards is the total number of search shards.
diff --git a/eval.go b/eval.go
@@ -731,6 +731,10 @@ func (d *indexData) List(ctx context.Context, q query.Q, opts *ListOptions) (rl
 
 	}
 
+	// Only one of these fields is populated and in all cases the size of that
+	// field is the number of Repos in this shard.
+	l.Stats.Repos = len(l.Repos) + len(l.Minimal) + len(l.ReposMap)
+
 	return &l, nil
 }
 
diff --git a/index_test.go b/index_test.go
@@ -1711,6 +1711,7 @@ func TestListRepos(t *testing.T) {
 						},
 					}},
 					Stats: RepoStats{
+						Repos:        1,
 						Documents:    4,
 						ContentBytes: 68,
 						Shards:       1,
@@ -1772,6 +1773,7 @@ func TestListRepos(t *testing.T) {
 				},
 			},
 			Stats: RepoStats{
+				Repos:                      1,
 				Shards:                     1,
 				Documents:                  4,
 				IndexBytes:                 412,
diff --git a/shards/shards.go b/shards/shards.go
@@ -987,6 +987,13 @@ func (ss *shardedSearcher) List(ctx context.Context, r query.Q, opts *zoekt.List
 		agg.Repos = append(agg.Repos, r)
 	}
 
+	// Only one of these fields is populated and in all cases the size of that
+	// field is the number of Repos.
+	//
+	// Note: we don't just add individual Stats.Repos since a repository can
+	// have multiple shards.
+	agg.Stats.Repos = len(uniq) + len(agg.Minimal) + len(agg.ReposMap)
+
 	if isAll && len(agg.Repos) > 0 {
 		reportListAllMetrics(agg.Repos)
 	}
diff --git a/shards/shards_test.go b/shards/shards_test.go
@@ -472,6 +472,7 @@ func TestShardedSearcher_List(t *testing.T) {
 
 	aggStats := stats
 	aggStats.Add(&aggStats) // since both repos have the exact same stats, this works
+	aggStats.Repos = 2      // Add doesn't populate Repos, this is done in Shards	based on the response sizes.
 
 	for _, tc := range []struct {
 		name string
diff --git a/web/server.go b/web/server.go
@@ -349,13 +349,7 @@ func (s *Server) fetchStats(ctx context.Context) (*zoekt.RepoStats, error) {
 		return nil, err
 	}
 
-	stats = &zoekt.RepoStats{}
-	names := map[string]struct{}{}
-	for _, r := range repos.Repos {
-		stats.Add(&r.Stats)
-		names[r.Repository.Name] = struct{}{}
-	}
-	stats.Repos = len(names)
+	stats = &repos.Stats
 
 	s.lastStatsMu.Lock()
 	s.lastStatsTS = time.Now()

Original file line number	Diff line number	Diff line change
`@@ -731,6 +731,10 @@ func (d indexData) List(ctx context.Context, q query.Q, opts ListOptions) (rl`
`731`	`731`
`732`	`732`	`}`
`733`	`733`
	`734`	`+ // Only one of these fields is populated and in all cases the size of that`
	`735`	`+ // field is the number of Repos in this shard.`
	`736`	`+ l.Stats.Repos = len(l.Repos) + len(l.Minimal) + len(l.ReposMap)`
	`737`	`+`
`734`	`738`	`return &l, nil`
`735`	`739`	`}`
`736`	`740`
Original file line number	Diff line number	Diff line change
`@@ -987,6 +987,13 @@ func (ss shardedSearcher) List(ctx context.Context, r query.Q, opts zoekt.List`
`987`	`987`	`agg.Repos = append(agg.Repos, r)`
`988`	`988`	`}`
`989`	`989`
	`990`	`+ // Only one of these fields is populated and in all cases the size of that`
	`991`	`+ // field is the number of Repos.`
	`992`	`+ //`
	`993`	`+ // Note: we don't just add individual Stats.Repos since a repository can`
	`994`	`+ // have multiple shards.`
	`995`	`+ agg.Stats.Repos = len(uniq) + len(agg.Minimal) + len(agg.ReposMap)`
	`996`	`+`
`990`	`997`	`if isAll && len(agg.Repos) > 0 {`
`991`	`998`	`reportListAllMetrics(agg.Repos)`
`992`	`999`	`}`