From 9026b0e76d275314f7f518213238b68b71c05982 Mon Sep 17 00:00:00 2001 From: Andrew Werner Date: Fri, 26 Apr 2019 14:53:59 -0400 Subject: [PATCH] raftentry: deflake TestConcurrentUpdates I spent way too long looking at this concerned that it was a correctness bug before I remembered that the gauges are a best effort mechanism updated on write operations asynchronously. They can become out of sync in particular when updates are reordered but this is okay because every update is based on the actual cache state, not the gauge state. Fixes #37111. Release note: None --- pkg/storage/raftentry/cache_test.go | 8 ++++++++ pkg/storage/raftentry/metrics.go | 2 ++ 2 files changed, 10 insertions(+) diff --git a/pkg/storage/raftentry/cache_test.go b/pkg/storage/raftentry/cache_test.go index 2ac67ad60da3..c990170889a8 100644 --- a/pkg/storage/raftentry/cache_test.go +++ b/pkg/storage/raftentry/cache_test.go @@ -149,6 +149,10 @@ func TestEntryCache(t *testing.T) { func verifyMetrics(t *testing.T, c *Cache, expectedEntries, expectedBytes int64) { t.Helper() + // NB: Cache gauges are updated asynchronously. In the face of concurrent + // updates gauges may not reflect the current cache state. Force + // synchrononization of the metrics before using them to validate cache state. + c.syncGauges() if got := c.Metrics().Size.Value(); got != expectedEntries { t.Errorf("expected cache to have %d entries, got %d", expectedEntries, got) } @@ -157,6 +161,10 @@ func verifyMetrics(t *testing.T, c *Cache, expectedEntries, expectedBytes int64) } } +func (c *Cache) syncGauges() { + c.updateGauges(c.addBytes(0), c.addEntries(0)) +} + func TestIgnoredAdd(t *testing.T) { defer leaktest.AfterTest(t)() rangeID := roachpb.RangeID(1) diff --git a/pkg/storage/raftentry/metrics.go b/pkg/storage/raftentry/metrics.go index c25e31b76c0f..4bb29f69359d 100644 --- a/pkg/storage/raftentry/metrics.go +++ b/pkg/storage/raftentry/metrics.go @@ -45,6 +45,8 @@ var ( // Metrics is the set of metrics for the raft entry cache. type Metrics struct { + // NB: the values in the gauges are updated asynchronously and may hold stale + // values in the face of concurrent updates. Size *metric.Gauge Bytes *metric.Gauge Accesses *metric.Counter