Bug 5983: Performance issues when listing software (#6879)

Improve performance when listing software by using the software_host_counts aggregate table.

changes/issue-5983-performance-issues-when-listing-software

@@ -0,0 +1 @@
+- Refactored method used when listing software to address performance issues.

server/datastore/mysql/software.go

@@ -415,12 +415,6 @@ func selectSoftwareSQL(opts fleet.SoftwareListOptions) (string, []interface{}, e
 			"s.arch",
 			goqu.I("scp.cpe").As("generated_cpe"),
 		).
-		Join( // filter software that is not associated with any hosts
-			goqu.I("host_software").As("hs"),
-			goqu.On(
-				goqu.I("hs.software_id").Eq(goqu.I("s.id")),
-			),
-		).
 		// Include this in the sub-query in case we want to sort by 'generated_cpe'
 		LeftJoin(
 			goqu.I("software_cpe").As("scp"),
@@ -430,20 +424,47 @@ func selectSoftwareSQL(opts fleet.SoftwareListOptions) (string, []interface{}, e
 		)
 
 	if opts.HostID != nil {
-		ds = ds.
-			SelectAppend("hs.last_opened_at").
-			Where(goqu.I("hs.host_id").Eq(opts.HostID))
-	}
-
-	if opts.TeamID != nil {
 		ds = ds.
 			Join(
-				goqu.I("hosts").As("h"),
+				goqu.I("host_software").As("hs"),
 				goqu.On(
-					goqu.I("hs.host_id").Eq(goqu.I("h.id")),
+					goqu.I("hs.software_id").Eq(goqu.I("s.id")),
+					goqu.I("hs.host_id").Eq(opts.HostID),
 				),
 			).
-			Where(goqu.I("h.team_id").Eq(opts.TeamID))
+			SelectAppend("hs.last_opened_at")
+		if opts.TeamID != nil {
+			ds = ds.
+				Join(
+					goqu.I("hosts").As("h"),
+					goqu.On(
+						goqu.I("hs.host_id").Eq(goqu.I("h.id")),
+						goqu.I("h.team_id").Eq(opts.TeamID),
+					),
+				)
+		}
+
+	} else {
+		// When loading software from all hosts, filter out software that is not associated with any
+		// hosts.
+		ds = ds.
+			Join(
+				goqu.I("software_host_counts").As("shc"),
+				goqu.On(
+					goqu.I("s.id").Eq(goqu.I("shc.software_id")),
+					goqu.I("shc.hosts_count").Gt(0),
+				),
+			).
+			GroupByAppend(
+				"shc.hosts_count",
+				"shc.updated_at",
+			)
+
+		if opts.TeamID != nil {
+			ds = ds.Where(goqu.I("shc.team_id").Eq(opts.TeamID))
+		} else {
+			ds = ds.Where(goqu.I("shc.team_id").Eq(0))
+		}
 	}
 
 	if opts.VulnerableOnly {
@@ -486,21 +507,10 @@ func selectSoftwareSQL(opts fleet.SoftwareListOptions) (string, []interface{}, e
 
 	if opts.WithHostCounts {
 		ds = ds.
-			Join(
-				goqu.I("software_host_counts").As("shc"),
-				goqu.On(goqu.I("s.id").Eq(goqu.I("shc.software_id"))),
-			).
-			Where(goqu.I("shc.hosts_count").Gt(0)).
 			SelectAppend(
 				goqu.I("shc.hosts_count"),
 				goqu.I("shc.updated_at").As("counts_updated_at"),
 			)
-
-		if opts.TeamID != nil {
-			ds = ds.Where(goqu.I("shc.team_id").Eq(opts.TeamID))
-		} else {
-			ds = ds.Where(goqu.I("shc.team_id").Eq(0))
-		}
 	}
 
 	ds = ds.GroupBy(

server/datastore/mysql/software_test.go

@@ -582,6 +582,8 @@ func testSoftwareList(t *testing.T, ds *Datastore) {
 		},
 	}
 
+	require.NoError(t, ds.SyncHostsSoftware(context.Background(), time.Now()))
+
 	t.Run("lists everything", func(t *testing.T) {
 		opts := fleet.SoftwareListOptions{
 			ListOptions: fleet.ListOptions{
@@ -621,6 +623,8 @@ func testSoftwareList(t *testing.T, ds *Datastore) {
 		require.NoError(t, err)
 		require.NoError(t, ds.AddHostsToTeam(context.Background(), &team1.ID, []uint{host1.ID}))
 
+		require.NoError(t, ds.SyncHostsSoftware(context.Background(), time.Now()))
+
 		opts := fleet.SoftwareListOptions{
 			ListOptions: fleet.ListOptions{
 				OrderKey: "version",
@@ -638,6 +642,8 @@ func testSoftwareList(t *testing.T, ds *Datastore) {
 		require.NoError(t, err)
 		require.NoError(t, ds.AddHostsToTeam(context.Background(), &team1.ID, []uint{host1.ID}))
 
+		require.NoError(t, ds.SyncHostsSoftware(context.Background(), time.Now()))
+
 		opts := fleet.SoftwareListOptions{
 			ListOptions: fleet.ListOptions{
 				PerPage:  1,
@@ -735,11 +741,6 @@ func testSoftwareList(t *testing.T, ds *Datastore) {
 	})
 
 	t.Run("order by hosts_count", func(t *testing.T) {
-		defer TruncateTables(t, ds, "software_host_counts")
-		listSoftwareCheckCount(t, ds, 0, 0, fleet.SoftwareListOptions{WithHostCounts: true}, false)
-
-		// create the counts for those software and re-run
-		require.NoError(t, ds.SyncHostsSoftware(context.Background(), time.Now()))
 		software := listSoftwareCheckCount(t, ds, 5, 5, fleet.SoftwareListOptions{ListOptions: fleet.ListOptions{OrderKey: "hosts_count", OrderDirection: fleet.OrderDescending}, WithHostCounts: true}, false)
 		// ordered by counts descending, so foo003 is first
 		assert.Equal(t, foo003.Name, software[0].Name)
@@ -866,8 +867,7 @@ func testSoftwareSyncHostsSoftware(t *testing.T, ds *Datastore) {
 	require.NoError(t, ds.UpdateHostSoftware(ctx, host1.ID, software1))
 	require.NoError(t, ds.UpdateHostSoftware(ctx, host2.ID, software2))
 
-	err := ds.SyncHostsSoftware(ctx, time.Now())
-	require.NoError(t, err)
+	require.NoError(t, ds.SyncHostsSoftware(ctx, time.Now()))
 
 	globalOpts := fleet.SoftwareListOptions{WithHostCounts: true, ListOptions: fleet.ListOptions{OrderKey: "hosts_count", OrderDirection: fleet.OrderDescending}}
 	globalCounts := listSoftwareCheckCount(t, ds, 4, 4, globalOpts, false)
@@ -887,9 +887,7 @@ func testSoftwareSyncHostsSoftware(t *testing.T, ds *Datastore) {
 		{Name: "foo", Version: "0.0.3", Source: "chrome_extensions"},
 	}
 	require.NoError(t, ds.UpdateHostSoftware(ctx, host2.ID, software2))
-
-	err = ds.SyncHostsSoftware(ctx, time.Now())
-	require.NoError(t, err)
+	require.NoError(t, ds.SyncHostsSoftware(ctx, time.Now()))
 
 	globalCounts = listSoftwareCheckCount(t, ds, 3, 3, globalOpts, false)
 	want = []fleet.Software{
@@ -901,7 +899,7 @@ func testSoftwareSyncHostsSoftware(t *testing.T, ds *Datastore) {
 	checkTableTotalCount(3)
 
 	// create a software entry without any host and any counts
-	_, err = ds.writer.ExecContext(ctx, `INSERT INTO software (name, version, source) VALUES ('baz', '0.0.1', 'testing')`)
+	_, err := ds.writer.ExecContext(ctx, `INSERT INTO software (name, version, source) VALUES ('baz', '0.0.1', 'testing')`)
 	require.NoError(t, err)
 
 	// listing does not return the new software entry
@@ -953,8 +951,7 @@ func testSoftwareSyncHostsSoftware(t *testing.T, ds *Datastore) {
 	checkTableTotalCount(3)
 
 	// after a call to Calculate, the global counts are updated and the team counts appear
-	err = ds.SyncHostsSoftware(ctx, time.Now())
-	require.NoError(t, err)
+	require.NoError(t, ds.SyncHostsSoftware(ctx, time.Now()))
 
 	globalCounts = listSoftwareCheckCount(t, ds, 4, 4, globalOpts, false)
 	want = []fleet.Software{
@@ -988,9 +985,7 @@ func testSoftwareSyncHostsSoftware(t *testing.T, ds *Datastore) {
 		{Name: "foo", Version: "0.0.3", Source: "chrome_extensions"},
 	}
 	require.NoError(t, ds.UpdateHostSoftware(ctx, host4.ID, software4))
-
-	err = ds.SyncHostsSoftware(ctx, time.Now())
-	require.NoError(t, err)
+	require.NoError(t, ds.SyncHostsSoftware(ctx, time.Now()))
 
 	globalCounts = listSoftwareCheckCount(t, ds, 3, 3, globalOpts, false)
 	want = []fleet.Software{
@@ -1021,8 +1016,7 @@ func testSoftwareSyncHostsSoftware(t *testing.T, ds *Datastore) {
 	require.NoError(t, ds.DeleteTeam(ctx, team2.ID))
 
 	// this call will remove team2 from the software host counts table
-	err = ds.SyncHostsSoftware(ctx, time.Now())
-	require.NoError(t, err)
+	require.NoError(t, ds.SyncHostsSoftware(ctx, time.Now()))
 
 	globalCounts = listSoftwareCheckCount(t, ds, 3, 3, globalOpts, false)
 	want = []fleet.Software{
@@ -1152,6 +1146,8 @@ func insertVulnSoftwareForTest(t *testing.T, ds *Datastore) {
 
 	require.NoError(t, err)
 	require.Equal(t, 2, int(n))
+
+	require.NoError(t, ds.SyncHostsSoftware(context.Background(), time.Now()))
 }
 
 func testDeleteSoftwareVulnerabilities(t *testing.T, ds *Datastore) {

server/service/integration_core_test.go

@@ -520,11 +520,6 @@ func (s *integrationTestSuite) TestVulnerableSoftware() {
 	assert.Contains(t, string(bodyBytes), expectedJSONSoft2)
 	assert.Contains(t, string(bodyBytes), expectedJSONSoft1)
 
-	countReq := countSoftwareRequest{}
-	countResp := countSoftwareResponse{}
-	s.DoJSON("GET", "/api/latest/fleet/software/count", countReq, http.StatusOK, &countResp)
-	assert.Equal(t, 3, countResp.Count)
-
 	// no software host counts have been calculated yet, so this returns nothing
 	var lsResp listSoftwareResponse
 	resp = s.Do("GET", "/api/latest/fleet/software", nil, http.StatusOK, "vulnerable", "true", "order_key", "generated_cpe", "order_direction", "desc")
@@ -536,15 +531,20 @@ func (s *integrationTestSuite) TestVulnerableSoftware() {
 	require.Len(t, lsResp.Software, 0)
 	assert.Nil(t, lsResp.CountsUpdatedAt)
 
+	// calculate hosts counts
+	hostsCountTs := time.Now().UTC()
+	require.NoError(t, s.ds.SyncHostsSoftware(context.Background(), hostsCountTs))
+
+	countReq := countSoftwareRequest{}
+	countResp := countSoftwareResponse{}
+	s.DoJSON("GET", "/api/latest/fleet/software/count", countReq, http.StatusOK, &countResp)
+	assert.Equal(t, 3, countResp.Count)
+
 	// the software/count endpoint is different, it doesn't care about hosts counts
 	countResp = countSoftwareResponse{}
 	s.DoJSON("GET", "/api/latest/fleet/software/count", countReq, http.StatusOK, &countResp, "vulnerable", "true", "order_key", "generated_cpe", "order_direction", "desc")
 	assert.Equal(t, 1, countResp.Count)
 
-	// calculate hosts counts
-	hostsCountTs := time.Now().UTC()
-	require.NoError(t, s.ds.SyncHostsSoftware(context.Background(), hostsCountTs))
-
 	// now the list software endpoint returns the software
 	lsResp = listSoftwareResponse{}
 	s.DoJSON("GET", "/api/latest/fleet/software", nil, http.StatusOK, &lsResp, "vulnerable", "true", "order_key", "generated_cpe", "order_direction", "desc")