Found during the 7-way review of #115.
benchmark_runner.go's recording loop calls l.totalHistogram.RecordValue(...) and the per-label histograms (writeHistogram, readHistogram, …) from every worker goroutine without a lock. hdrhistogram's RecordValue is not concurrency-safe. Under the default -workers 8 this races: go build -race reports ~13 data races on the RecordValue → setCountAtIndex path, and increments are lost.
Note detailedMapHistograms and perSecondHistograms are mutex-protected, but totalHistogram and the per-label histograms are not — an internal inconsistency.
Impact:
Fix options:
- Guard the shared histograms with a mutex on the record path (simple; adds contention to the hot loop), or
- Record into per-worker histograms and merge at the end (no hot-path contention; preferred).
Found during the 7-way review of #115.
benchmark_runner.go's recording loop callsl.totalHistogram.RecordValue(...)and the per-label histograms (writeHistogram,readHistogram, …) from every worker goroutine without a lock.hdrhistogram'sRecordValueis not concurrency-safe. Under the default-workers 8this races:go build -racereports ~13 data races on theRecordValue→setCountAtIndexpath, and increments are lost.Note
detailedMapHistogramsandperSecondHistogramsare mutex-protected, buttotalHistogramand the per-label histograms are not — an internal inconsistency.Impact:
OverallQuantiles.*) computed from these histograms can be wrong/under-populated under concurrency.TotalOps/overallOpsRatewere also affected — but Fix pipeline command accounting: flush trailing window + exact TotalOps #115 fixed those specifically by moving them to an atomic counter. The percentiles remain exposed.Fix options: