Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Patch for race condition on map access #324

Merged

Conversation

estheruary
Copy link
Contributor

Which problem is this PR solving?

  • When we use refinery in our environment we sometimes encounter a race condition on startup.
refinery_1          | time="2021-09-30T13:28:00Z" level=info msg="Peer list has changed. New peer list: [http://127.0.0
.1:8081]"
refinery_1          | fatal error: concurrent map read and map write
refinery_1          |
refinery_1          | goroutine 61 [running]:
refinery_1          | runtime.throw(0xe386aa, 0x21)
refinery_1          |   /usr/local/go/src/runtime/panic.go:1117 +0x72 fp=0xc000047638 sp=0xc000047608 pc=0x438272
refinery_1          | runtime.mapaccess2_faststr(0xd12000, 0xc0002cbef0, 0xe2fa46, 0x18, 0xc0002e00a0, 0xc000296ba0)
refinery_1          |   /usr/local/go/src/runtime/map_faststr.go:116 +0x4a5 fp=0xc0000476a8 sp=0xc000047638 pc=0x414465
refinery_1          | github.com/honeycombio/refinery/metrics.(*PromMetrics).Histogram(0xc00036ad40, 0xe2fa46, 0x18, 0x
ccd400, 0x13f38c0)
refinery_1          |   /app/metrics/prometheus.go:103 +0x53 fp=0xc0000476e8 sp=0xc0000476a8 pc=0xa52a53
refinery_1          | github.com/honeycombio/refinery/collect.(*InMemCollector).collect(0xc000300280)
refinery_1          |   /app/collect/collect.go:261 +0x14b fp=0xc0000477d8 sp=0xc0000476e8 pc=0xa5bd2b
refinery_1          | runtime.goexit()
refinery_1          |   /usr/local/go/src/runtime/asm_amd64.s:1371 +0x1 fp=0xc0000477e0 sp=0xc0000477d8 pc=0x4719e1
refinery_1          | created by github.com/honeycombio/refinery/collect.(*InMemCollector).Start
refinery_1          |   /app/collect/collect.go:117 +0x588
refinery_1          |
refinery_1          | goroutine 1 [runnable]:
refinery_1          | regexp.onePassCopy(0xc0003584b0, 0x0)
refinery_1          |   /usr/local/go/src/regexp/onepass.go:222 +0x77
refinery_1          | regexp.compileOnePass(0xc0003584b0, 0xc0003584b0)
refinery_1          |   /usr/local/go/src/regexp/onepass.go:497 +0x105
refinery_1          | regexp.compile(0xc00037a108, 0x8, 0xd4, 0xc00037a108, 0x8, 0x0)
refinery_1          |   /usr/local/go/src/regexp/regexp.go:189 +0x111
refinery_1          | regexp.Compile(...)
refinery_1          |   /usr/local/go/src/regexp/regexp.go:133
refinery_1          | github.com/gorilla/mux.newRouteRegexp(0xe2031f, 0x6, 0x0, 0xc000000000, 0x0, 0x0, 0xc00000e068)
refinery_1          |   /go/pkg/mod/github.com/gorilla/mux@v1.6.3-0.20190108142930-08e7f807d38d/regexp.go:121 +0xa97
refinery_1          | github.com/gorilla/mux.(*Route).addRegexpMatcher(0xc000298000, 0xe2031f, 0x6, 0x0, 0x1, 0xc00000e
068)
refinery_1          |   /go/pkg/mod/github.com/gorilla/mux@v1.6.3-0.20190108142930-08e7f807d38d/route.go:186 +0x9f
refinery_1          | github.com/gorilla/mux.(*Route).Path(...)
refinery_1          |   /go/pkg/mod/github.com/gorilla/mux@v1.6.3-0.20190108142930-08e7f807d38d/route.go:354
refinery_1          | github.com/gorilla/mux.(*Router).HandleFunc(0xc0001ce0c0, 0xe2031f, 0x6, 0xc000378140, 0x7)
refinery_1          |   /go/pkg/mod/github.com/gorilla/mux@v1.6.3-0.20190108142930-08e7f807d38d/mux.go:296 +0x176
refinery_1          | github.com/honeycombio/refinery/route.(*Router).LnS(0xc000308020, 0xe22f9c, 0x8)
refinery_1          |   /app/route/route.go:146 +0x8c7
refinery_1          | github.com/honeycombio/refinery/app.(*App).Start(0xc000308000, 0xe2598a, 0xb)
refinery_1          |   /app/app/app.go:35 +0xe7
refinery_1          | github.com/facebookgo/startstop.TryStart(0xc000104120, 0x11, 0x11, 0xf30bd8, 0xc0002e80e0, 0x0, 0
x19, 0xf3dbb0, 0xc0003003c0, 0x0)
refinery_1          |   /go/pkg/mod/github.com/facebookgo/startstop@v0.0.0-20161013234910-bc158412526d/startstop.go:69
+0x224
refinery_1          | github.com/facebookgo/startstop.Start(...)
refinery_1          |   /go/pkg/mod/github.com/facebookgo/startstop@v0.0.0-20161013234910-bc158412526d/startstop.go:82
refinery_1          | main.main()
refinery_1          |   /app/cmd/refinery/main.go:202 +0x1565
refinery_1          |
refinery_1          | goroutine 5 [semacquire]:
refinery_1          | sync.runtime_Semacquire(0xc00037a0a8)
refinery_1          |   /usr/local/go/src/runtime/sema.go:56 +0x45
refinery_1          | sync.(*WaitGroup).Wait(0xc00037a0a0)
refinery_1          |   /usr/local/go/src/sync/waitgroup.go:130 +0x65
refinery_1          | github.com/spf13/viper.(*Viper).WatchConfig.func1(0xc000001080, 0xc00037a050)
refinery_1          |   /go/pkg/mod/github.com/spf13/viper@v1.8.1/viper.go:410 +0x2a7
refinery_1          | created by github.com/spf13/viper.(*Viper).WatchConfig
refinery_1          |   /go/pkg/mod/github.com/spf13/viper@v1.8.1/viper.go:349 +0x79
refinery_1          |
refinery_1          | goroutine 6 [syscall]:
refinery_1          | syscall.Syscall6(0xe8, 0x7, 0xc00038fc2c, 0x7, 0xffffffffffffffff, 0x0, 0x0, 0x0, 0x0, 0x0)
refinery_1          |   /usr/local/go/src/syscall/asm_linux_amd64.s:43 +0x5
refinery_1          | golang.org/x/sys/unix.EpollWait(0x7, 0xc00038fc2c, 0x7, 0x7, 0xffffffffffffffff, 0x0, 0x0, 0x0)
refinery_1          |   /go/pkg/mod/golang.org/x/sys@v0.0.0-20210510120138-977fb7262007/unix/zsyscall_linux_amd64.go:77
 +0x72
refinery_1          | github.com/fsnotify/fsnotify.(*fdPoller).wait(0xc00007cd00, 0x0, 0x0, 0x0)
refinery_1          |   /go/pkg/mod/github.com/fsnotify/fsnotify@v1.4.9/inotify_poller.go:86 +0x91
refinery_1          | github.com/fsnotify/fsnotify.(*Watcher).readEvents(0xc000074910)
refinery_1          |   /go/pkg/mod/github.com/fsnotify/fsnotify@v1.4.9/inotify.go:192 +0x206
refinery_1          | created by github.com/fsnotify/fsnotify.NewWatcher
refinery_1          |   /go/pkg/mod/github.com/fsnotify/fsnotify@v1.4.9/inotify.go:59 +0x1ab
refinery_1          |
refinery_1          | goroutine 7 [select]:
refinery_1          | github.com/spf13/viper.(*Viper).WatchConfig.func1.1(0xc000074910, 0xc00037a0a0, 0xd2abe2, 0x1b, 0
xd2abe2, 0x1b, 0xc000378ca0, 0xc000001080)
refinery_1          |   /go/pkg/mod/github.com/spf13/viper@v1.8.1/viper.go:371 +0xca
refinery_1          | created by github.com/spf13/viper.(*Viper).WatchConfig.func1
refinery_1          |   /go/pkg/mod/github.com/spf13/viper@v1.8.1/viper.go:369 +0x259
refinery_1          |
refinery_1          | goroutine 8 [semacquire]:
refinery_1          | sync.runtime_Semacquire(0xc00037a128)
refinery_1          |   /usr/local/go/src/runtime/sema.go:56 +0x45
refinery_1          | sync.(*WaitGroup).Wait(0xc00037a120)
refinery_1          |   /usr/local/go/src/sync/waitgroup.go:130 +0x65
refinery_1          | github.com/spf13/viper.(*Viper).WatchConfig.func1(0xc000001200, 0xc00037a0d0)
refinery_1          |   /go/pkg/mod/github.com/spf13/viper@v1.8.1/viper.go:410 +0x2a7
refinery_1          | created by github.com/spf13/viper.(*Viper).WatchConfig
refinery_1          |   /go/pkg/mod/github.com/spf13/viper@v1.8.1/viper.go:349 +0x79
refinery_1          |
refinery_1          | goroutine 9 [syscall]:
refinery_1          | syscall.Syscall6(0xe8, 0xb, 0xc000517c2c, 0x7, 0xffffffffffffffff, 0x0, 0x0, 0x0, 0x0, 0x0)
refinery_1          |   /usr/local/go/src/syscall/asm_linux_amd64.s:43 +0x5
refinery_1          | golang.org/x/sys/unix.EpollWait(0xb, 0xc000517c2c, 0x7, 0x7, 0xffffffffffffffff, 0x0, 0x0, 0x0)
refinery_1          |   /go/pkg/mod/golang.org/x/sys@v0.0.0-20210510120138-977fb7262007/unix/zsyscall_linux_amd64.go:77
 +0x72
refinery_1          | github.com/fsnotify/fsnotify.(*fdPoller).wait(0xc00007cd60, 0x0, 0x0, 0x0)
refinery_1          |   /go/pkg/mod/github.com/fsnotify/fsnotify@v1.4.9/inotify_poller.go:86 +0x91
refinery_1          | github.com/fsnotify/fsnotify.(*Watcher).readEvents(0xc000074960)
refinery_1          |   /go/pkg/mod/github.com/fsnotify/fsnotify@v1.4.9/inotify.go:192 +0x206
refinery_1          | created by github.com/fsnotify/fsnotify.NewWatcher
refinery_1          |   /go/pkg/mod/github.com/fsnotify/fsnotify@v1.4.9/inotify.go:59 +0x1ab
refinery_1          |
refinery_1          | goroutine 10 [select]:
refinery_1          | github.com/spf13/viper.(*Viper).WatchConfig.func1.1(0xc000074960, 0xc00037a120, 0xd2f3cf, 0x18, 0
xd2f3cf, 0x18, 0xc000378cc0, 0xc000001200)
refinery_1          |   /go/pkg/mod/github.com/spf13/viper@v1.8.1/viper.go:371 +0xca
refinery_1          | created by github.com/spf13/viper.(*Viper).WatchConfig.func1
refinery_1          |   /go/pkg/mod/github.com/spf13/viper@v1.8.1/viper.go:369 +0x259
refinery_1          |
refinery_1          | goroutine 12 [select]:
refinery_1          | github.com/facebookgo/muster.(*Client).worker(0xc0002e03c0)
refinery_1          |   /go/pkg/mod/github.com/facebookgo/muster@v0.0.0-20150708232844-fd3d7953fd52/muster.go:165 +0x23
1
refinery_1          | created by github.com/facebookgo/muster.(*Client).Start
refinery_1          |   /go/pkg/mod/github.com/facebookgo/muster@v0.0.0-20150708232844-fd3d7953fd52/muster.go:113 +0x12
f
refinery_1          |
refinery_1          | goroutine 13 [select]:
refinery_1          | github.com/facebookgo/muster.(*Client).worker(0xc0002e0000)
refinery_1          |   /go/pkg/mod/github.com/facebookgo/muster@v0.0.0-20150708232844-fd3d7953fd52/muster.go:165 +0x23
1
refinery_1          | created by github.com/facebookgo/muster.(*Client).Start
refinery_1          |   /go/pkg/mod/github.com/facebookgo/muster@v0.0.0-20150708232844-fd3d7953fd52/muster.go:113 +0x12
f
refinery_1          |
refinery_1          | goroutine 16 [IO wait]:
refinery_1          | internal/poll.runtime_pollWait(0x7f1f787736d8, 0x72, 0x0)
refinery_1          |   /usr/local/go/src/runtime/netpoll.go:222 +0x55
refinery_1          | internal/poll.(*pollDesc).wait(0xc00007a118, 0x72, 0x0, 0x0, 0xe21a27)
refinery_1          |   /usr/local/go/src/internal/poll/fd_poll_runtime.go:87 +0x45
refinery_1          | internal/poll.(*pollDesc).waitRead(...)
refinery_1          |   /usr/local/go/src/internal/poll/fd_poll_runtime.go:92
refinery_1          | internal/poll.(*FD).Accept(0xc00007a100, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0)
refinery_1          |   /usr/local/go/src/internal/poll/fd_unix.go:401 +0x212
refinery_1          | net.(*netFD).accept(0xc00007a100, 0x30, 0x30, 0x7f1f9f3d25b8)
refinery_1          |   /usr/local/go/src/net/fd_unix.go:172 +0x45
refinery_1          | net.(*TCPListener).accept(0xc00000c060, 0xc000014dd0, 0x40e2f8, 0x30)
refinery_1          |   /usr/local/go/src/net/tcpsock_posix.go:139 +0x32
refinery_1          | net.(*TCPListener).Accept(0xc00000c060, 0xda5fc0, 0xc0003581e0, 0xd048a0, 0x1423f30)
refinery_1          |   /usr/local/go/src/net/tcpsock.go:261 +0x65
refinery_1          | net/http.(*Server).Serve(0xc0000f0620, 0xf37e50, 0xc00000c060, 0x0, 0x0)
refinery_1          |   /usr/local/go/src/net/http/server.go:2961 +0x285
refinery_1          | net/http.(*Server).ListenAndServe(0xc0000f0620, 0xc0000f0620, 0xc000044fb8)
refinery_1          |   /usr/local/go/src/net/http/server.go:2890 +0xba
refinery_1          | net/http.ListenAndServe(0xc0000362a0, 0xc, 0xf281c0, 0xc0001ce000, 0xc00056fb20, 0x7bcc45)
refinery_1          |   /usr/local/go/src/net/http/server.go:3144 +0x74
refinery_1          | created by github.com/honeycombio/refinery/metrics.(*PromMetrics).Start
refinery_1          |   /app/metrics/prometheus.go:40 +0x1cc
refinery_1          |
refinery_1          | goroutine 42 [select]:
refinery_1          | github.com/honeycombio/refinery/transmit.(*DefaultTransmission).processResponses(0xc0002e8000, 0x
f39f70, 0xc0002592c0, 0xc000370d20)
refinery_1          |   /app/transmit/transmit.go:149 +0xe9
refinery_1          | created by github.com/honeycombio/refinery/transmit.(*DefaultTransmission).Start
refinery_1          |   /app/transmit/transmit.go:78 +0x448
refinery_1          |
refinery_1          | goroutine 46 [select]:
refinery_1          | github.com/honeycombio/refinery/transmit.(*DefaultTransmission).processResponses(0xc0002e8070, 0x
f39f70, 0xc0002593c0, 0xc000370ea0)
refinery_1          |   /app/transmit/transmit.go:149 +0xe9
refinery_1          | created by github.com/honeycombio/refinery/transmit.(*DefaultTransmission).Start
refinery_1          |   /app/transmit/transmit.go:78 +0x448
refinery_1          |
refinery_1          | goroutine 62 [chan receive]:
refinery_1          | github.com/klauspost/compress/zstd.(*blockDec).startDecoder(0xc00007e1a0)
refinery_1          |   /go/pkg/mod/github.com/klauspost/compress@v1.13.3/zstd/blockdec.go:212 +0x149
refinery_1          | created by github.com/klauspost/compress/zstd.newBlockDec
refinery_1          |   /go/pkg/mod/github.com/klauspost/compress@v1.13.3/zstd/blockdec.go:118 +0x173
refinery_1          |
refinery_1          | goroutine 63 [runnable]:
refinery_1          | github.com/klauspost/compress/zstd.(*blockDec).startDecoder(0xc00007e270)
refinery_1          |   /go/pkg/mod/github.com/klauspost/compress@v1.13.3/zstd/blockdec.go:210
refinery_1          | created by github.com/klauspost/compress/zstd.newBlockDec
refinery_1          |   /go/pkg/mod/github.com/klauspost/compress@v1.13.3/zstd/blockdec.go:118 +0x173
refinery_1          |
refinery_1          | goroutine 64 [runnable]:
refinery_1          | github.com/klauspost/compress/zstd.(*blockDec).startDecoder(0xc00007e410)
refinery_1          |   /go/pkg/mod/github.com/klauspost/compress@v1.13.3/zstd/blockdec.go:210
refinery_1          | created by github.com/klauspost/compress/zstd.newBlockDec
refinery_1          |   /go/pkg/mod/github.com/klauspost/compress@v1.13.3/zstd/blockdec.go:118 +0x173
refinery_1          |
refinery_1          | goroutine 65 [runnable]:
refinery_1          | github.com/klauspost/compress/zstd.(*blockDec).startDecoder(0xc00007e4e0)
refinery_1          |   /go/pkg/mod/github.com/klauspost/compress@v1.13.3/zstd/blockdec.go:210
refinery_1          | created by github.com/klauspost/compress/zstd.newBlockDec
refinery_1          |   /go/pkg/mod/github.com/klauspost/compress@v1.13.3/zstd/blockdec.go:118 +0x173

Short description of the changes

  • Moar locks.

@estheruary estheruary requested a review from a team October 1, 2021 12:32
@robbkidd robbkidd self-assigned this Oct 1, 2021
@robbkidd robbkidd added status: review needed Changes need review. type: bug Something isn't working labels Oct 1, 2021
Copy link
Contributor

@MikeGoldsmith MikeGoldsmith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @estheruary - thank you for the PR to fix this race issue 👍🏻

@MikeGoldsmith MikeGoldsmith merged commit 491e49a into honeycombio:main Oct 4, 2021
@MikeGoldsmith MikeGoldsmith added the version: bump patch A PR with release-worthy changes and is backwards-compatible. label Oct 4, 2021
MikeGoldsmith added a commit that referenced this pull request Oct 6, 2021
A mutex is used in the prometheus metrics implementation and in #324 was used the lock when using the different metrics types. This PR switches the mutex type to a RWLock to allow concurrent retrieving of metrics from the map.

Co-authored-by: Robb Kidd <robbkidd@honeycomb.io>
@robbkidd robbkidd removed the status: review needed Changes need review. label Oct 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug Something isn't working version: bump patch A PR with release-worthy changes and is backwards-compatible.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants