diff --git a/changes/issue-5983-performance-issues-when-listing-software b/changes/issue-5983-performance-issues-when-listing-software new file mode 100644 index 0000000000..099de473d5 --- /dev/null +++ b/changes/issue-5983-performance-issues-when-listing-software @@ -0,0 +1 @@ +- Refactored method used when listing software to address performance issues. diff --git a/server/datastore/mysql/software.go b/server/datastore/mysql/software.go index 5a6582cb00..57e50033bb 100644 --- a/server/datastore/mysql/software.go +++ b/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( diff --git a/server/datastore/mysql/software_test.go b/server/datastore/mysql/software_test.go index df006f05af..e1725abb57 100644 --- a/server/datastore/mysql/software_test.go +++ b/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) { diff --git a/server/service/integration_core_test.go b/server/service/integration_core_test.go index 22a46b780b..1f874afc0a 100644 --- a/server/service/integration_core_test.go +++ b/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")