From d51493274df1ba84cdd3b330208ff875595d3e42 Mon Sep 17 00:00:00 2001 From: Tomas Touceda Date: Tue, 10 Aug 2021 18:17:06 -0300 Subject: [PATCH] Issue 1570 stats perf (#1598) * Dont delete pack stats before inserting new ones to prevent deadlocks * Remove fk for scheduled_query_stats * Remove fk removal * Fix tests * Remove unneeded comment --- changes/issue-1570-improve-performance | 1 + go.sum | 1 + server/datastore/mysql/hosts.go | 10 +- server/datastore/mysql/hosts_test.go | 7 -- server/datastore/mysql/packs_test.go | 136 +++++++++++++++++++++++++ 5 files changed, 139 insertions(+), 16 deletions(-) create mode 100644 changes/issue-1570-improve-performance diff --git a/changes/issue-1570-improve-performance b/changes/issue-1570-improve-performance new file mode 100644 index 0000000000..fa5a97777c --- /dev/null +++ b/changes/issue-1570-improve-performance @@ -0,0 +1 @@ +* Improve performance of pack statistic insertions in the database. diff --git a/go.sum b/go.sum index c0a0c7a1c5..b114dbd9eb 100644 --- a/go.sum +++ b/go.sum @@ -490,6 +490,7 @@ github.com/yuin/goldmark v1.1.32/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9de github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.3.5/go.mod h1:mwnBkeHKe2W/ZEtQ+71ViKU8L12m81fl3OWwC1Zlc8k= github.com/zenazn/goji v0.9.0/go.mod h1:7S9M489iMyHBNxwZnk9/EHS098H4/F6TATF2mIxtB1Q= +github.com/ziutek/mymysql v1.5.4 h1:GB0qdRGsTwQSBVYuVShFBKaXSnSnYYC2d9knnE1LHFs= github.com/ziutek/mymysql v1.5.4/go.mod h1:LMSpPZ6DbqWFxNCHW77HeMg9I646SAhApZ/wKdgO/C0= github.com/zwass/kit v0.0.0-20210625184505-ec5b5c5cce9c h1:TWQ2UvXPkhPxI2KmApKBOCaV6yD2N4mlvqFQ/DlPtpQ= github.com/zwass/kit v0.0.0-20210625184505-ec5b5c5cce9c/go.mod h1:OYYulo9tUqRadRLwB0+LE914sa1ui2yL7OrcU3Q/1XY= diff --git a/server/datastore/mysql/hosts.go b/server/datastore/mysql/hosts.go index e0fb40efd6..b494174f95 100644 --- a/server/datastore/mysql/hosts.go +++ b/server/datastore/mysql/hosts.go @@ -162,14 +162,6 @@ func (d *Datastore) SaveHost(host *fleet.Host) error { func (d *Datastore) saveHostPackStats(host *fleet.Host) error { if err := d.withRetryTxx(func(tx *sqlx.Tx) error { - sql := ` - DELETE FROM scheduled_query_stats - WHERE host_id = ? - ` - if _, err := tx.Exec(sql, host.ID); err != nil { - return errors.Wrap(err, "delete old stats") - } - // Bulk insert software entries var args []interface{} queryCount := 0 @@ -199,7 +191,7 @@ func (d *Datastore) saveHostPackStats(host *fleet.Host) error { } values := strings.TrimSuffix(strings.Repeat("((SELECT sq.id FROM scheduled_queries sq JOIN packs p ON (sq.pack_id = p.id) WHERE p.name = ? AND sq.name = ?),?,?,?,?,?,?,?,?,?,?),", queryCount), ",") - sql = fmt.Sprintf(` + sql := fmt.Sprintf(` INSERT IGNORE INTO scheduled_query_stats ( scheduled_query_id, host_id, diff --git a/server/datastore/mysql/hosts_test.go b/server/datastore/mysql/hosts_test.go index 1ad21a3130..2db68114c0 100644 --- a/server/datastore/mysql/hosts_test.go +++ b/server/datastore/mysql/hosts_test.go @@ -209,13 +209,6 @@ func TestSaveHostPackStats(t *testing.T) { host, err = ds.Host(host.ID) require.NoError(t, err) require.Len(t, host.PackStats, 2) - - // Set to empty should make it empty - host.PackStats = []fleet.PackStats{} - require.NoError(t, ds.SaveHost(host)) - host, err = ds.Host(host.ID) - require.NoError(t, err) - require.Len(t, host.PackStats, 0) } func TestDeleteHost(t *testing.T) { diff --git a/server/datastore/mysql/packs_test.go b/server/datastore/mysql/packs_test.go index fa2a069282..a16c3c194c 100644 --- a/server/datastore/mysql/packs_test.go +++ b/server/datastore/mysql/packs_test.go @@ -1,8 +1,11 @@ package mysql import ( + "context" "fmt" + "math/rand" "testing" + "time" "github.com/WatchBeam/clock" "github.com/fleetdm/fleet/v4/server/fleet" @@ -513,3 +516,136 @@ func TestApplyPackSpecFailsOnTargetIDNull(t *testing.T) { err := ds.ApplyPackSpecs(specs) require.Error(t, err) } + +func randomPackStatsForHost(hostID, packID uint, scheduledQueries []*fleet.ScheduledQuery) *fleet.Host { + var queryStats []fleet.ScheduledQueryStats + + amount := rand.Intn(5000) + + for i := 0; i < amount; i++ { + sq := scheduledQueries[rand.Intn(len(scheduledQueries))] + queryStats = append(queryStats, fleet.ScheduledQueryStats{ + ScheduledQueryName: sq.Name, + ScheduledQueryID: sq.ID, + QueryName: sq.QueryName, + Description: sq.Description, + PackID: packID, + AverageMemory: rand.Intn(100), + Denylisted: false, + Executions: rand.Intn(100), + Interval: rand.Intn(100), + LastExecuted: time.Now(), + OutputSize: rand.Intn(1000), + SystemTime: rand.Intn(1000), + UserTime: rand.Intn(1000), + WallTime: rand.Intn(1000), + }) + } + return &fleet.Host{ + ID: hostID, + PackStats: []fleet.PackStats{ + { + PackID: packID, + QueryStats: queryStats, + }, + }, + } +} + +func TestPackApplyStatsNotLocking(t *testing.T) { + t.Skip("This can be too much for the test db if you're running all tests") + + ds := CreateMySQLDS(t) + defer ds.Close() + + specs := setupPackSpecsTest(t, ds) + + host, err := ds.NewHost(&fleet.Host{ + DetailUpdatedAt: time.Now(), + LabelUpdatedAt: time.Now(), + SeenTime: time.Now(), + NodeKey: "1", + UUID: "1", + Hostname: "foo.local", + PrimaryIP: "192.168.1.1", + PrimaryMac: "30-65-EC-6F-C4-58", + }) + require.NoError(t, err) + require.NotNil(t, host) + + ctx, cancelFunc := context.WithCancel(context.Background()) + go func() { + ticker := time.NewTicker(100 * time.Millisecond) + for { + select { + case <-ctx.Done(): + return + case <-ticker.C: + pack, _, err := ds.PackByName("test_pack") + require.NoError(t, err) + schedQueries, err := ds.ListScheduledQueriesInPack(pack.ID, fleet.ListOptions{}) + require.NoError(t, err) + + require.NoError(t, ds.saveHostPackStats(randomPackStatsForHost(host.ID, pack.ID, schedQueries))) + } + } + }() + + time.Sleep(1 * time.Second) + for i := 0; i < 1000; i++ { + require.NoError(t, ds.ApplyPackSpecs(specs)) + time.Sleep(77 * time.Millisecond) + } + + cancelFunc() +} + +func TestPackApplyStatsNotLockingTryTwo(t *testing.T) { + t.Skip("This can be too much for the test db if you're running all tests") + + ds := CreateMySQLDS(t) + defer ds.Close() + + setupPackSpecsTest(t, ds) + + host, err := ds.NewHost(&fleet.Host{ + DetailUpdatedAt: time.Now(), + LabelUpdatedAt: time.Now(), + SeenTime: time.Now(), + NodeKey: "1", + UUID: "1", + Hostname: "foo.local", + PrimaryIP: "192.168.1.1", + PrimaryMac: "30-65-EC-6F-C4-58", + }) + require.NoError(t, err) + require.NotNil(t, host) + + ctx, cancelFunc := context.WithCancel(context.Background()) + for i := 0; i < 2; i++ { + go func() { + ms := rand.Intn(100) + if ms == 0 { + ms = 10 + } + ticker := time.NewTicker(time.Duration(ms) * time.Millisecond) + for { + select { + case <-ctx.Done(): + return + case <-ticker.C: + pack, _, err := ds.PackByName("test_pack") + require.NoError(t, err) + schedQueries, err := ds.ListScheduledQueriesInPack(pack.ID, fleet.ListOptions{}) + require.NoError(t, err) + + require.NoError(t, ds.saveHostPackStats(randomPackStatsForHost(host.ID, pack.ID, schedQueries))) + } + } + }() + } + + time.Sleep(60 * time.Second) + + cancelFunc() +}