Merge #158474

158474: schedulerlatency: Increase the go scheduler latency metric time coverage r=iskettaneh a=iskettaneh

Before this commit, the go scheduler latency metric was publish once every 10 seconds, and it was based on 2.5 seconds worth of data. That meant that there was 75% blind spot in that metric. This is especially important for short-lived overload that might not have been detected with this metric.

This commit builds on the current interval at which we measure the scheduler latency (100ms), and keeps adding these 100ms measurements into a histogram that gets published (and cleared) every 10s.

The figure below shows the Before/After metric on 2 clusters with the old and the new metric when running the following command: `while true; do timeout 3.5 roachprod run $CLUSTER:4 -- './cockroach workload run kv --concurrency=256 --read-percent=95 --duration=120m {pgurl:1}'; sleep 57.5; done`

<img width="1920" height="1440" alt="schedLatencyBefAft" src="https://github.com/user-attachments/assets/608994df-6f19-4e0c-ab36-a88f31caacec" />

You can see that in the Before figure, many of these spikes are missed. While they are visible in the new metric.

Release note: None

Fixes: #158475

Co-authored-by: iskettaneh <173953022+iskettaneh@users.noreply.github.com>

Author: craig[bot]

pkg/util/schedulerlatency/sampler.go

@@ -98,12 +98,11 @@ func StartSampler(
 				case <-stopper.ShouldQuiesce():
 					return
 				case <-ticker.C:
-					lastIntervalHistogram := s.lastIntervalHistogram()
-					if lastIntervalHistogram == nil {
+					schedulingLatenciesHistogram := s.getAndClearLastStatsHistogram()
+					if schedulingLatenciesHistogram == nil {
 						continue
 					}
-
-					schedulerLatencyHistogram.update(lastIntervalHistogram)
+					schedulerLatencyHistogram.update(schedulingLatenciesHistogram)
 				}
 			}
 		})
@@ -149,8 +148,10 @@ type sampler struct {
 	listener LatencyObserver
 	mu       struct {
 		syncutil.Mutex
-		ringBuffer            ring.Buffer[*metrics.Float64Histogram]
-		lastIntervalHistogram *metrics.Float64Histogram
+		ringBuffer ring.Buffer[*metrics.Float64Histogram]
+		// schedulerLatencyAccumulator accumulates per-sample histogram deltas of
+		// the go scheduler latencies.
+		schedulerLatencyAccumulator *metrics.Float64Histogram
 	}
 }
 
@@ -170,7 +171,7 @@ func (s *sampler) setPeriodAndDuration(period, duration time.Duration) {
 		numSamples = 1 // we need at least one sample to compare (also safeguards against integer division)
 	}
 	s.mu.ringBuffer.Resize(numSamples)
-	s.mu.lastIntervalHistogram = nil
+	s.mu.schedulerLatencyAccumulator = nil
 }
 
 // sampleOnTickAndInvokeCallbacks samples scheduler latency stats as the ticker
@@ -179,16 +180,32 @@ func (s *sampler) sampleOnTickAndInvokeCallbacks(period time.Duration) {
 	s.mu.Lock()
 	defer s.mu.Unlock()
 
+	// Capture the previous sample before adding the new one.
+	var prevSample *metrics.Float64Histogram
+	if s.mu.ringBuffer.Len() > 0 {
+		prevSample = s.mu.ringBuffer.GetFirst()
+	}
+
 	latestCumulative := sample()
 	oldestCumulative, ok := s.recordLocked(latestCumulative)
 	if !ok {
 		return
 	}
-	s.mu.lastIntervalHistogram = sub(latestCumulative, oldestCumulative)
-	p99 := time.Duration(int64(percentile(s.mu.lastIntervalHistogram, 0.99) * float64(time.Second.Nanoseconds())))
+
+	// Compute the delta since the previous sample for stats accumulation.
+	if prevSample != nil {
+		sampleDelta := sub(latestCumulative, prevSample)
+		if s.mu.schedulerLatencyAccumulator == nil {
+			s.mu.schedulerLatencyAccumulator = sampleDelta
+		} else {
+			s.mu.schedulerLatencyAccumulator = add(s.mu.schedulerLatencyAccumulator, sampleDelta)
+		}
+	}
 
 	// Perform the callback if there's a listener.
 	if s.listener != nil {
+		p99 := time.Duration(int64(percentile(sub(latestCumulative, oldestCumulative),
+			0.99) * float64(time.Second.Nanoseconds())))
 		s.listener.SchedulerLatency(p99, period)
 	}
 }
@@ -204,10 +221,12 @@ func (s *sampler) recordLocked(
 	return oldest, oldest != nil
 }
 
-func (s *sampler) lastIntervalHistogram() *metrics.Float64Histogram {
+func (s *sampler) getAndClearLastStatsHistogram() *metrics.Float64Histogram {
 	s.mu.Lock()
 	defer s.mu.Unlock()
-	return s.mu.lastIntervalHistogram
+	res := s.mu.schedulerLatencyAccumulator
+	s.mu.schedulerLatencyAccumulator = nil
+	return res
 }
 
 // sample the cumulative (since process start) scheduler latency histogram from
@@ -249,6 +268,16 @@ func sub(a, b *metrics.Float64Histogram) *metrics.Float64Histogram {
 	return res
 }
 
+// add adds the counts of one histogram to another, assuming the bucket
+// boundaries are the same.
+func add(a, b *metrics.Float64Histogram) *metrics.Float64Histogram {
+	res := clone(a)
+	for i := 0; i < len(res.Counts); i++ {
+		res.Counts[i] += b.Counts[i]
+	}
+	return res
+}
+
 // percentile computes a specific percentile value of the given histogram.
 //
 // TODO(irfansharif): Deduplicate this with the quantile computation in