Skip to content
This repository has been archived by the owner on Jul 19, 2023. It is now read-only.

Fix panic in usagestats #706

Merged

Conversation

simonswine
Copy link
Collaborator

@simonswine simonswine commented May 23, 2023

NewCounter is part of the ingestion hot path. This can very likely cause a race condition, when first creating a expvar:

  |   | 2023-05-23 07:02:20.827 | net/http/server.go:2109 +0x2f |  
  |   | 2023-05-23 07:02:20.827 | net/http.HandlerFunc.ServeHTTP(0x40dc07?, {0x4a8a8c0?, 0xc000f61300?}, 0xc0013a8101?) |  
  |   | 2023-05-23 07:02:20.827 | github.com/grafana/phlare/pkg/util/http.go:165 +0x459 |  
  |   | 2023-05-23 07:02:20.827 | github.com/grafana/phlare/pkg/util.Log.Wrap.func1({0x4a8a8c0, 0xc000f61300}, 0xc0018be200) |  
  |   | 2023-05-23 07:02:20.827 | net/http/server.go:2109 +0x2f |  
  |   | 2023-05-23 07:02:20.827 | net/http.HandlerFunc.ServeHTTP(0x0?, {0x4a8a530?, 0xc000fd9450?}, 0x0?) |  
  |   | 2023-05-23 07:02:20.827 | github.com/weaveworks/[email protected]/middleware/instrument.go:69 +0x37f |  
  |   | 2023-05-23 07:02:20.827 | github.com/weaveworks/common/middleware.Instrument.Wrap.func1({0x4a8a530, 0xc000fd9450}, 0xc0018be200) |  
  |   | 2023-05-23 07:02:20.827 | github.com/felixge/[email protected]/capture_metrics.go:39 |  
  |   | 2023-05-23 07:02:20.827 | github.com/felixge/httpsnoop.CaptureMetricsFn(...) |  
  |   | 2023-05-23 07:02:20.827 | github.com/felixge/[email protected]/capture_metrics.go:84 +0x1ff |  
  |   | 2023-05-23 07:02:20.827 | github.com/felixge/httpsnoop.(*Metrics).CaptureMetrics(0xc000fbc4b0, {0x4a8a530, 0xc000fd9450}, 0xc000ca5ab8) |  
  |   | 2023-05-23 07:02:20.827 | github.com/weaveworks/[email protected]/middleware/instrument.go:70 +0x39 |  
  |   | 2023-05-23 07:02:20.827 | github.com/weaveworks/common/middleware.Instrument.Wrap.func1.2({0x4a8a530?, 0xc000fd94f0?}) |  
  |   | 2023-05-23 07:02:20.827 | github.com/gorilla/[email protected]/mux.go:210 +0x1cf |  
  |   | 2023-05-23 07:02:20.827 | github.com/gorilla/mux.(*Router).ServeHTTP(0xc0001a2840, {0x4a8a530, 0xc000fd94f0}, 0xc0018be200) |  
  |   | 2023-05-23 07:02:20.827 | net/http/server.go:2109 +0x2f |  
  |   | 2023-05-23 07:02:20.827 | net/http.HandlerFunc.ServeHTTP(0xc0018be300?, {0x4a8a530?, 0xc000fd94f0?}, 0xc000ca5830?) |  
  |   | 2023-05-23 07:02:20.827 | github.com/grafana/phlare/pkg/util/gziphandler/gzip.go:357 +0x235 |  
  |   | 2023-05-23 07:02:20.827 | github.com/grafana/phlare/pkg/util/gziphandler.GzipHandlerWithOpts.func1.1({0x4a8a530, 0xc000fd94f0}, 0xc001aea1b0?) |  
  |   | 2023-05-23 07:02:20.827 | net/http/server.go:2109 +0x2f |  
  |   | 2023-05-23 07:02:20.827 | net/http.HandlerFunc.ServeHTTP(0x34012e0?, {0x4a79bb0?, 0xc0007b92d0?}, 0x4?) |  
  |   | 2023-05-23 07:02:20.827 | github.com/grafana/phlare/pkg/util/http.go:368 +0x288 |  
  |   | 2023-05-23 07:02:20.827 | github.com/grafana/phlare/pkg/util.AuthenticateUser.func1.1({0x4a79bb0, 0xc0007b92d0}, 0xc0018be400) |  
  |   | 2023-05-23 07:02:20.827 | github.com/pyroscope-io/[email protected]/pkg/server/ingest.go:60 +0xe3 |  
  |   | 2023-05-23 07:02:20.827 | github.com/pyroscope-io/pyroscope/pkg/server.ingestHandler.ServeHTTP({{0x4a549e0, 0xc000f15e00}, {0x4a570a0, 0xc001338bb0}, 0x44c3c20, {0x4a90f80, 0xc001338bc0}}, {0x4a79bb0, 0xc0007b92d0}, 0xc0018be500) |  
  |   | 2023-05-23 07:02:20.827 | github.com/grafana/phlare/pkg/ingester/pyroscope/ingest_adapter.go:44 +0xad |  
  |   | 2023-05-23 07:02:20.827 | github.com/grafana/phlare/pkg/ingester/pyroscope.(*pyroscopeIngesterAdapter).Ingest(0x4a549e0?, {0x4a8be48?, 0xc001aea240?}, 0xc001338bb0?) |  
  |   | 2023-05-23 07:02:20.827 | github.com/pyroscope-io/[email protected]/pkg/convert/profile/profile.go:56 +0x3aa |  
  |   | 2023-05-23 07:02:20.827 | github.com/pyroscope-io/pyroscope/pkg/convert/profile.(*RawProfile).Parse(0xc001aea360, {0x4a8be48, 0xc001aea240}, {0x4a570e0, 0xc001338bb0}, {0x4a570c0, 0xc001338bb0}, {{0x0, 0xedbfe5bf2, 0x6a75220}, ...}) |  
  |   | 2023-05-23 07:02:20.827 | github.com/grafana/phlare/pkg/ingester/pyroscope/ingest_adapter.go:128 +0xaea |  
  |   | 2023-05-23 07:02:20.827 | github.com/grafana/phlare/pkg/ingester/pyroscope.(*pyroscopeIngesterAdapter).Put(0xc001338bb0, {0x4a8be48, 0xc001aea240}, 0xc000087a80) |  
  |   | 2023-05-23 07:02:20.827 | github.com/grafana/phlare/pkg/distributor/distributor.go:194 +0x17d8 |  
  |   | 2023-05-23 07:02:20.827 | github.com/grafana/phlare/pkg/distributor.(*Distributor).Push(0xc0010ff080, {0x4a8be48?, 0xc001aea240?}, 0xc000f14cd0) |  
  |   | 2023-05-23 07:02:20.827 | github.com/grafana/phlare/pkg/usagestats/stats.go:334 +0x1c6 |  
  |   | 2023-05-23 07:02:20.827 | github.com/grafana/phlare/pkg/usagestats.NewCounter({0xc000f62db0, 0x2d}) |  
  |   | 2023-05-23 07:02:20.827 | expvar/expvar.go:284 +0xf4 |  
  |   | 2023-05-23 07:02:20.827 | expvar.Publish({0xc000fe29b0, 0x47}, {0x4a572e0?, 0xc001aea630}) |  
  |   | 2023-05-23 07:02:20.827 | log/log.go:402 +0x65 |  
  |   | 2023-05-23 07:02:20.827 | log.Panicln({0xc000ca4620?, 0x47?, 0xc0004df140?}) |  
  |   | 2023-05-23 07:02:20.827 | panic({0x34012e0, 0xc0004df160}) |  
  |   | 2023-05-23 07:02:20.827 | github.com/opentracing-contrib/[email protected]/nethttp/server.go:150 +0x126 |  
  |   | 2023-05-23 07:02:20.827 | github.com/opentracing-contrib/go-stdlib/nethttp.MiddlewareFunc.func5.1() |  
  |   | 2023-05-23 07:02:20.827 | panic({0x34012e0, 0xc0004df160}) |  
  |   | 2023-05-23 07:02:20.827 | golang.org/x/[email protected]/http2/server.go:2304 +0x145 |  
  |   | 2023-05-23 07:02:20.827 | golang.org/x/net/http2.(*serverConn).runHandler.func1() |  
  |   | 2023-05-23 07:02:20.827 | goroutine 13499918 [running]: |  
  |   | 2023-05-23 07:02:20.827 |   |  
  |   | 2023-05-23 07:02:20.827 | 2023/05/23 07:02:20 http2: panic serving 10.70.10.24:47264: Reuse of exported var name: github.com/grafana/phlare/distributor_profile_type_process_cpu_received |  

@simonswine simonswine added the kind/bug Something isn't working label May 23, 2023
@simonswine
Copy link
Collaborator Author

Once we are happy, I would contribute it back to loki

@simonswine simonswine marked this pull request as ready for review May 23, 2023 10:34
@simonswine simonswine merged commit ef392e8 into grafana:main May 23, 2023
@simonswine simonswine self-assigned this May 23, 2023
simonswine added a commit to grafana/loki that referenced this pull request May 25, 2023
We discovered a bug in Phlare
grafana/phlare#706. There can be a race occuring
between two different `New<Counter|Int>` calls for the same `name`.
simonswine added a commit to simonswine/pyroscope that referenced this pull request Jun 30, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind/bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants