From 25c37a178d743bd152eee61d3721ffd08117d7dc Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 22 Aug 2023 14:49:10 +0000 Subject: [PATCH 1/5] fix(coderd/batchststs): fix init race and close flush From 2adfd5c01aadf5d23074aa80169346cde506a89c Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 22 Aug 2023 13:06:35 +0000 Subject: [PATCH 2/5] fix race in batchstats batcher init --- coderd/batchstats/batcher.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/coderd/batchstats/batcher.go b/coderd/batchstats/batcher.go index e53a24cb4c1d1..0413ba4dfb167 100644 --- a/coderd/batchstats/batcher.go +++ b/coderd/batchstats/batcher.go @@ -105,6 +105,8 @@ func New(ctx context.Context, opts ...Option) (*Batcher, func(), error) { b.tickCh = b.ticker.C } + b.initBuf(b.batchSize) + cancelCtx, cancelFunc := context.WithCancel(ctx) done := make(chan struct{}) go func() { @@ -172,7 +174,6 @@ func (b *Batcher) Add( // Run runs the batcher. func (b *Batcher) run(ctx context.Context) { - b.initBuf(b.batchSize) // nolint:gocritic // This is only ever used for one thing - inserting agent stats. authCtx := dbauthz.AsSystemRestricted(ctx) for { From ed069e8a435e7dacc1c1f01a9de8278a237ce1f8 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 22 Aug 2023 14:47:35 +0000 Subject: [PATCH 3/5] fix flush after close in statsbatcher --- coderd/batchstats/batcher.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coderd/batchstats/batcher.go b/coderd/batchstats/batcher.go index 0413ba4dfb167..8b8c9f8db8960 100644 --- a/coderd/batchstats/batcher.go +++ b/coderd/batchstats/batcher.go @@ -175,7 +175,7 @@ func (b *Batcher) Add( // Run runs the batcher. func (b *Batcher) run(ctx context.Context) { // nolint:gocritic // This is only ever used for one thing - inserting agent stats. - authCtx := dbauthz.AsSystemRestricted(ctx) + authCtx := dbauthz.AsSystemRestricted(context.Background()) for { select { case <-b.tickCh: From abab11c7fe40a5034e7f702552e0164cab9eeeba Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 22 Aug 2023 17:27:20 +0000 Subject: [PATCH 4/5] better fix --- coderd/batchstats/batcher.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/coderd/batchstats/batcher.go b/coderd/batchstats/batcher.go index 8b8c9f8db8960..b3b881f2133e9 100644 --- a/coderd/batchstats/batcher.go +++ b/coderd/batchstats/batcher.go @@ -175,7 +175,7 @@ func (b *Batcher) Add( // Run runs the batcher. func (b *Batcher) run(ctx context.Context) { // nolint:gocritic // This is only ever used for one thing - inserting agent stats. - authCtx := dbauthz.AsSystemRestricted(context.Background()) + authCtx := dbauthz.AsSystemRestricted(ctx) for { select { case <-b.tickCh: @@ -185,7 +185,13 @@ func (b *Batcher) run(ctx context.Context) { b.flush(authCtx, true, "reaching capacity") case <-ctx.Done(): b.log.Debug(ctx, "context done, flushing before exit") - b.flush(authCtx, true, "exit") + + // We must create a new context here as the parent context is done. + ctxTimeout, cancel := context.WithTimeout(context.Background(), 15*time.Second) + defer cancel() //nolint:revive // We're returning, defer is fine. + + // nolint:gocritic // This is only ever used for one thing - inserting agent stats. + b.flush(dbauthz.AsSystemRestricted(ctxTimeout), true, "exit") return } } From e4b7d56d5b1b74d631a6f73307cab7531cfa6b84 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Wed, 23 Aug 2023 08:34:30 +0000 Subject: [PATCH 5/5] fix test via buffered flush channel --- coderd/batchstats/batcher_internal_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coderd/batchstats/batcher_internal_test.go b/coderd/batchstats/batcher_internal_test.go index b5d18e81327fd..8c1c367f7db5b 100644 --- a/coderd/batchstats/batcher_internal_test.go +++ b/coderd/batchstats/batcher_internal_test.go @@ -31,7 +31,7 @@ func TestBatchStats(t *testing.T) { deps1 := setupDeps(t, store) deps2 := setupDeps(t, store) tick := make(chan time.Time) - flushed := make(chan int) + flushed := make(chan int, 1) b, closer, err := New(ctx, WithStore(store),