From 2554d823b0e8d9b522d9e9645cdd1fe824ded193 Mon Sep 17 00:00:00 2001 From: masseselsev Date: Fri, 18 Sep 2026 20:10:25 +0000 Subject: [PATCH] fix: stop the backfill from starving the collector's write lock The background backfill runs hourly-sized chunks with a short pause between them; a day-sized chunk held the SQLite write lock past the 5 s busy_timeout on router-class storage, and every collector tick that landed during a chunk rolled back and lost its raw samples (SQLITE_BUSY warnings on the live router). The pause is a var so tests shrink it, matching the retention batch pattern. --- backend-go/internal/db/metric_buckets.go | 25 +++++++++++++++---- backend-go/internal/db/metric_buckets_test.go | 11 ++++++++ 2 files changed, 31 insertions(+), 5 deletions(-) diff --git a/backend-go/internal/db/metric_buckets.go b/backend-go/internal/db/metric_buckets.go index e48f073..a3370b6 100644 --- a/backend-go/internal/db/metric_buckets.go +++ b/backend-go/internal/db/metric_buckets.go @@ -230,11 +230,23 @@ const MetricBucketBackfillMarker = "metric_buckets_backfilled" // shipped in a release yet, so "1" is the only version that will ever have been stored. const MetricBucketBackfillVersion = "1" -// backfillChunkSeconds is the width of one backfill chunk: a day, a whole multiple of the -// bucket grid. Chunking bounds the size of a single statement (and of SQLite's temporary -// sort) so an install with months of history does not build one enormous aggregate, and -// it lets an interrupted backfill resume without having written partial buckets. -const backfillChunkSeconds = 24 * 60 * 60 +// backfillChunkSeconds is the width of one backfill chunk: an hour, a whole multiple of +// the bucket grid. Chunking bounds the size of a single statement (and of SQLite's +// temporary sort) so an install with months of history does not build one enormous +// aggregate, and it lets an interrupted backfill resume without having written partial +// buckets. The size is a lock-contention budget, not just a memory one: each chunk is a +// write transaction, and the collector's 10 s tick needs the write lock too. A day-sized +// chunk held it past the 5 s busy_timeout on router-class storage, so every tick that +// landed during a chunk rolled back and its raw samples were lost (SQLITE_BUSY); an +// hour-sized chunk plus a pause between chunks keeps each lock window short enough for +// the tick to slip in. +const backfillChunkSeconds = 60 * 60 + +// backfillChunkPause is how long the backfill yields the database between chunks. It +// does not have to be long — the collector ticks every 10 s — but zero pause lets one +// connection re-acquire the write lock immediately and starve everyone else. (Kept a +// var: tests shrink it, as with rawMetricPruneBatch.) +var backfillChunkPause = 200 * time.Millisecond // BackfillMetricBuckets builds metric buckets from the raw samples already in the // database, so charts are populated the moment an existing install is upgraded instead of @@ -284,6 +296,9 @@ func BackfillMetricBuckets(database *DB, retentionDays int) (bool, error) { }); err != nil { return false, fmt.Errorf("backfill metric buckets %d..%d: %w", chunkStart, chunkEnd, err) } + // Yield between chunks so the collector's tick can take the write lock; see + // backfillChunkPause. + time.Sleep(backfillChunkPause) } if err := database.SetSetting(MetricBucketBackfillMarker, MetricBucketBackfillVersion, diff --git a/backend-go/internal/db/metric_buckets_test.go b/backend-go/internal/db/metric_buckets_test.go index f9dffa0..d46ee6c 100644 --- a/backend-go/internal/db/metric_buckets_test.go +++ b/backend-go/internal/db/metric_buckets_test.go @@ -268,6 +268,13 @@ func TestMetricBucketBackfillRunsOnceAndIsIdempotent(t *testing.T) { database := openBucketTestDB(t) defer database.Close() + // The backfill window spans the retention horizon (30 days of hourly chunks); + // the production inter-chunk pause would dominate the runtime here. Shrink it the + // same way rawMetricPruneBatch is shrunk in retention tests. + restore := backfillChunkPause + backfillChunkPause = time.Millisecond + defer func() { backfillChunkPause = restore }() + base := time.Date(2026, 9, 18, 12, 0, 0, 0, time.UTC) for i := 0; i < 4; i++ { insertSystemRaw(t, database, base.Add(time.Duration(i)*10*time.Second), float64(i+1), 50, 100, 256, nil, nil) @@ -331,6 +338,10 @@ func TestMetricBucketBackfillEmptyDatabase(t *testing.T) { database := openBucketTestDB(t) defer database.Close() + restore := backfillChunkPause + backfillChunkPause = time.Millisecond + defer func() { backfillChunkPause = restore }() + ran, err := BackfillMetricBuckets(database, 30) if err != nil { t.Fatalf("backfill on empty database failed: %v", err)