Automattic / Automattic/pingo

Crash on parallel requests with triggered dump

Open
#4 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
39
Forks
4
PR merge metrics
No merged PRs in 30d

Description

If you enable triggered dump (and it actually gets enabled, see #3), parallel requests to the `/metrics` endpoint can potentially result in a crash of the whole Pingo process, caused by a double-unlock on the `dumpLock`. This should normally not be a big problem, since for triggered dump it does not make sense to have more than one scraper, as they would be "stealing" data from each other.

For non-triggered dump parallel requests work fine. Especially since there the scrape is decoupled from the actual metrics generation, and multiple scrapers just get the same data.

```
fatal error: sync: Unlock of unlocked RWMutex

goroutine 95 [running]:
sync.fatal({0x8cc409?, 0xc000068a40?})
runtime/panic.go:1038 +0x18
sync.(*RWMutex).Unlock(0xc83eb0)
sync/rwmutex.go:212 +0x45
main.main.promScrapeMiddleware.func1({0x9651b8, 0xc00025e0e0}, 0xc000250140)
github.com/Automattic/pingo/pingo.go:746 +0x6b
net/http.HandlerFunc.ServeHTTP(0xc61780?, {0x9651b8?, 0xc00025e0e0?}, 0x716e16?)
net/http/server.go:2294 +0x29
net/http.(*ServeMux).ServeHTTP(0x4701b9?, {0x9651b8, 0xc00025e0e0}, 0xc000250140)
net/http/server.go:2822 +0x1c4
net/http.serverHandler.ServeHTTP({0xc00020cbd0?}, {0x9651b8?, 0xc00025e0e0?}, 0x1?)
net/http/server.go:3301 +0x8e
net/http.(*conn).serve(0xc00023e240, {0x9655b8, 0xc00012c450})
net/http/server.go:2102 +0x625
created by net/http.(*Server).Serve in goroutine 1
net/http/server.go:3454 +0x485

[…]

goroutine 93 [sleep]:
time.Sleep(0x5f5e100)
runtime/time.go:338 +0x165
main.main.promScrapeMiddleware.func1({0x9651b8, 0xc00025e000}, 0xc000250000)
github.com/Automattic/pingo/pingo.go:747 +0x75
net/http.HandlerFunc.ServeHTTP(0xc61780?, {0x9651b8?, 0xc00025e000?}, 0x716e16?)
net/http/server.go:2294 +0x29
net/http.(*ServeMux).ServeHTTP(0x4701b9?, {0x9651b8, 0xc00025e000}, 0xc000250000)
net/http/server.go:2822 +0x1c4
net/http.serverHandler.ServeHTTP({0xc00020c840?}, {0x9651b8?, 0xc00025e000?}, 0x1?)
net/http/server.go:3301 +0x8e
net/http.(*conn).serve(0xc00023e090, {0x9655b8, 0xc00012c450})
net/http/server.go:2102 +0x625
created by net/http.(*Server).Serve in goroutine 1
net/http/server.go:3454 +0x485

[…]
```

Contributor guide

No contributing guide indexed for this repository

Research direction

Start in pingo.go around the promScrapeMiddleware stack trace at lines 746-747, and inspect how dumpLock is handled when triggered dump is enabled. Reproduce the issue with parallel requests to /metrics and triggered dump enabled. Done means parallel requests no longer crash the Pingo process, while non-triggered dump behavior remains working.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
backend, observability
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
57/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.