Fix a data race in a metrics registry without serialising every read

1. 0bashexit 031 ms$0.00407/43 tok18:49:42.133

   Read the contract before the code. Point four of it is the whole task: it rules out the fix that every tutorial on this error reaches for.

   bash commandno wrap

   ls -R . && cat README.md

   outputwrapexpand 51 lines

   .:
   README.md
   agg
   cmd
   go.mod
   
   ./agg:
   aggregator.go
   aggregator_test.go
   histogram.go
   
   ./cmd:
   aggd
   
   ./cmd/aggd:
   main.go

   showing 16 of 51 lines

2. 1grep_filesno exit codeschema violationharness error0 ms$0.00916/11 tok18:49:42.133

   F01 tool schema violationrule 1.00

   Searching the tree for the failing assertion.

   arguments the model emittedunparsedno wrap

   {"pattern": "assert", "glob": 

   outputwrap

   Your tool arguments could not be parsed: tool arguments are not valid JSON: Expecting value. Send valid JSON matching the tool schema.

   Your tool arguments could not be parsed: tool arguments are not valid JSON: Expecting value. Send valid JSON matching the tool schema.

3. 2bashexit 0468 ms$0.001027/71 tok18:49:42.612

   Reproduce it and read the report properly. The detector names the field, both stacks and the goroutines, which is a more reliable map of what is unsynchronised than reading the file will be.

   bash commandno wrap

   export PATH=/usr/local/go/bin:$PATH; go test -race -count=1 ./agg/ 2>&1 | head -60

   outputwrapexpand 61 lines

   ==================
   WARNING: DATA RACE
   Read at 0x00c000198088 by goroutine 13:
     example.internal/aggregator/agg.(*Registry).lookup()
         /workspace/agg/aggregator.go:79 +0x44
     example.internal/aggregator/agg.(*Registry).Observe()
         /workspace/agg/aggregator.go:67 +0x46
     example.internal/aggregator/agg_test.TestConcurrentObserveKeepsEveryUpdate.func1()
         /workspace/agg/aggregator_test.go:78 +0x117
     example.internal/aggregator/agg_test.TestConcurrentObserveKeepsEveryUpdate.gowrap1()
         /workspace/agg/aggregator_test.go:80 +0x41
   
   Previous write at 0x00c000198088 by goroutine 17:
     example.internal/aggregator/agg.(*Registry).lookup()
         /workspace/agg/aggregator.go:90 +0x1e5
     example.internal/aggregator/agg.(*Registry).Observe()

   showing 16 of 61 lines

4. 3read_fileno exit codeharness error31 ms$0.001874/26 tok18:49:42.644

   F02 path hallucinationrule 1.00

   The configuration is probably in src/config/settings.local.yaml.

   argumentswrap

   {
     "path": "src/config/settings.local.yaml"
   }

   3 lines, 46 B

   outputwrap

   No such file or directory: src/config/settings.local.yaml

   src/config/settings.local.yaml

5. 4read_fileno exit code31 ms$0.001971/56 tok18:49:42.675

   Now the code. mu covers the map and nothing else: the bucket fields, the histogram and the observed counter are all touched outside it, and lookup reads and writes the hot cache with nothing held.

   argumentswrap

   {
     "path": "agg/aggregator.go"
   }

   3 lines, 33 B

   outputwrapexpand 146 lines

   // Package agg aggregates per endpoint request latency for a service that serves a few
   // thousand requests a second across a handful of endpoints.
   //
   // One Registry is shared by every request goroutine and by the exporter goroutine that
   // scrapes it once a second. Observe is on the request path. Snapshot, SnapshotAll,
   // Endpoints and Observed are on the exporter's path. Neither path is allowed to make the
   // other wait longer than it has to, and two exporter reads of the same endpoint must be
   // able to run at the same time: quantile walks the histogram in place, so a read is not
   // cheap and serialising reads behind one lock turns a one second scrape into a stall.
   //
   // This started life single threaded. A mutex was added later, around the map, which is
   // where it stopped.
   package agg
   
   import (
   	"sort"

   showing 16 of 146 lines

6. 5read_fileno exit code31 ms$0.003112/68 tok18:49:42.705

   This file is out of bounds, so read it for the constraint rather than the fix. It says a histogram carries no lock of its own and that quantile reads it in place, which means the read path has to hold the same lock the writers take while it runs.

   argumentswrap

   {
     "path": "agg/histogram.go"
   }

   3 lines, 32 B

   outputwrapexpand 65 lines

   package agg
   
   import (
   	"math"
   	"time"
   )
   
   // bucketCount buckets cover one microsecond to just over eight seconds, in powers of two.
   const bucketCount = 24
   
   // quantileCost stands in for the interpolation pass the real exporter runs across the
   // buckets and the reservoir it keeps beside them.
   //
   // It is a sleep rather than a busy loop on purpose. What matters about the read path is
   // whether two readers can be inside it at the same time, and a busy loop would measure
   // how many cores the box has instead.

   showing 16 of 65 lines

7. 6write_fileno exit code46 ms$0.003728/1309 tok18:49:42.740

   One lock per bucket, RWMutex so the exporter's reads share, the map lock held only long enough to resolve a name, and the counter moved to an atomic. The hot cache goes: it existed to skip a map lookup that now costs a shared read lock, locking it would reintroduce the contention it was added to avoid, and it was what made Reset leave a bucket behind.

   contentwrapexpand 163 lines

   // Package agg aggregates per endpoint request latency for a service that serves a few
   // thousand requests a second across a handful of endpoints.
   //
   // One Registry is shared by every request goroutine and by the exporter goroutine that
   // scrapes it once a second. Observe is on the request path. Snapshot, SnapshotAll,
   // Endpoints and Observed are on the exporter's path. Neither path is allowed to make the
   // other wait longer than it has to, and two exporter reads of the same endpoint must be
   // able to run at the same time: quantile walks the histogram in place, so a read is not
   // cheap and serialising reads behind one lock turns a one second scrape into a stall.
   //
   // Locking, from the outside in. Registry.mu guards the map itself and nothing else, held
   // for a map lookup and released. Each bucket carries its own RWMutex, which is the lock
   // the histogram is documented to need: writers take it exclusively, readers share it, so
   // two Snapshots of the same endpoint overlap and a Snapshot of one endpoint never waits
   // on writes to another. observed is an atomic because it is touched by every writer and
   // read by the exporter, and a counter is the one thing that does not need a lock at all.
   package agg
   

   showing 18 of 163 lines

   argumentswrap

   {
     "path": "agg/aggregator.go"
   }

   3 lines, 33 B

   outputwrap

   Wrote 4577 bytes (163 lines) to agg/aggregator.go.

8. 7bashexit 094 ms$0.005177/45 tok18:49:42.845

   Cheap checks first. A vet finding or a formatting diff is not worth a race detector run.

   bash commandno wrap

   export PATH=/usr/local/go/bin:$PATH; gofmt -l . && go vet ./... && echo clean

   outputwrap

   clean
   

   2 lines, 6 B

9. 8bashexit 01.34 s$0.005282/87 tok18:49:44.192

   The suite under the detector, which is the only evidence that counts here. Note that a passing run proves nothing about interleavings it did not take, which is why the reasoning above is about which lock covers which field rather than about the result.

   bash commandno wrap

   export PATH=/usr/local/go/bin:$PATH; go test -race -count=1 -v ./... 2>&1 | tail -20

   outputwrap

   ?   	example.internal/aggregator/cmd/aggd	[no test files]
   === RUN   TestObserveThenSnapshot
   --- PASS: TestObserveThenSnapshot (0.01s)
   === RUN   TestSnapshotAllCoversEveryEndpoint
   --- PASS: TestSnapshotAllCoversEveryEndpoint (0.02s)
   === RUN   TestConcurrentObserveKeepsEveryUpdate
   --- PASS: TestConcurrentObserveKeepsEveryUpdate (0.03s)
   === RUN   TestExporterScrapesWhileRequestsLand
   --- PASS: TestExporterScrapesWhileRequestsLand (0.00s)
   PASS
   ok  	example.internal/aggregator/agg	1.069s
   

   12 lines, 487 B

10. 9bashexit 0171 ms$0.005554/56 tok18:49:44.358

    Run the thing. The report has to show every observation accounted for, which is the lost update half of the contract and is not something the detector checks.

    bash commandno wrap

    export PATH=/usr/local/go/bin:$PATH; go run ./cmd/aggd

    outputwrap

    observed 3000 of 3000
    /v1/cart         count=1000   mean=319    max=549    p95=1024
    /v1/checkout     count=1000   mean=319    max=549    p95=1024
    /v1/inventory    count=1000   mean=319    max=549    p95=1024
    

    5 lines, 208 B

11. 10bashexit 231 ms$0.005722/23 tok18:49:44.392

    F05 shell quoting errorrule 1.00

    Locating the function that owns the range logic.

    bash commandno wrap

    grep -rn "def parse_range( src/

    outputwrap

    bash: -c: line 1: unexpected EOF while looking for matching `"'
    

    2 lines, 64 B

12. 11finishno exit code0 ms$0.005821/193 tok18:49:44.393

    Close with which lock covers what, because that is the part a reviewer has to check.

    summarywrap

    Registry.mu guarded the map only. The per endpoint aggregate fields, the histogram behind them and the observed counter were all mutated with no lock held, and the hot endpoint cache in front of the map was read and written unsynchronised, which is also why Reset left a stale bucket live. Fixed by giving each bucket its own RWMutex (writers exclusive, readers shared, held across quantile because it reads the histogram in place), holding the map's RWMutex only for the lookup, making the counter an atomic, and dropping the cache. Readers of one endpoint now overlap each other and never wait on writes to another endpoint. go test -race ./... and go vet ./... are clean.
    

    2 lines, 675 B

    outputwrap

    Run ended by the agent.

12 steps6 commands1 schema violations1 failed commands0 destructive attempts