Merge #37165

37165: raftentry: deflake TestConcurrentUpdates r=ajwerner a=ajwerner

I spent way too long looking at this concerned that it was a correctness
bug before I remembered that the gauges are updated with a snapshot of the
cache state but the writes are not totally ordered and thus can become out of sync
in the face of concurrent updates. Later updates will update to later values. This
isn't a correctness problem, just an artifact of the design of the metrics which the
test relied on. This PR adds logic to sync the gauges before verifying their state
which deflakes the test when run under stress.

Fixes #37111.

Release note: None

Co-authored-by: Andrew Werner <[email protected]>

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)

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