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

fix: panic on slice bounds out of range when preparing data stream #599

Merged

Conversation

dowster
Copy link
Contributor

@dowster dowster commented Jul 9, 2024

We've been seeing fatals from go-carbon when queries pull back more than the max metrics limit. Unsure which of these values is the one being read but it seemed like guarding all of them would be the best approach.

fatal log:

2024-07-09 14:58:57.474	panic: runtime error: slice bounds out of range [:40000] with capacity 0
2024-07-09 14:58:57.474	
2024-07-09 14:58:57.474	goroutine 4140 [running]:
2024-07-09 14:58:57.474	github.com/go-graphite/go-carbon/carbonserver.(*CarbonserverListener).prepareDataStream(0xc00014e000, {0xf71cf8, 0xc03ca6c780}, 0x2, 0xc03ca3dcb0?, 0xc03ca3dec0?, 0xc03ca7c600)
2024-07-09 14:58:57.474		/usr/local/src/go-carbon/carbonserver/render.go:425 +0xf4d
2024-07-09 14:58:57.474	github.com/go-graphite/go-carbon/carbonserver.(*CarbonserverListener).prepareDataProto.func1()
2024-07-09 14:58:57.474		/usr/local/src/go-carbon/carbonserver/render.go:495 +0xb3
2024-07-09 14:58:57.474	created by github.com/go-graphite/go-carbon/carbonserver.(*CarbonserverListener).prepareDataProto
2024-07-09 14:58:57.474		/usr/local/src/go-carbon/carbonserver/render.go:493 +0x4d5

dowster and others added 2 commits June 17, 2024 11:25
Other slices may be 0 length, resulting in a panic
when running `slice[:listener.maxMetricsReturned`

```
{"level":"ERROR","timestamp":"2024-06-17T15:29:14.474Z","logger":"access","message":"rendering too many metrics","limit":10000,"target":11272}
panic: runtime error: slice bounds out of range [:10000] with capacity 0

goroutine 2919 [running]:
github.com/go-graphite/go-carbon/carbonserver.(*CarbonserverListener).prepareDataStream(0xc0000c8000, {0xf71cf8, 0xc0015b3860}, 0x2, 0xc0015b3b90?, 0xc0015b3ef0?, 0xc00c88b7a0)
	/usr/local/src/go-carbon/carbonserver/render.go:425 +0xf4d
github.com/go-graphite/go-carbon/carbonserver.(*CarbonserverListener).prepareDataProto.func1()
	/usr/local/src/go-carbon/carbonserver/render.go:495 +0xb3
created by github.com/go-graphite/go-carbon/carbonserver.(*CarbonserverListener).prepareDataProto
	/usr/local/src/go-carbon/carbonserver/render.go:493 +0x4d5
```
@deniszh
Copy link
Member

deniszh commented Jul 10, 2024

LGTM, merging. Thanks a lot!

@deniszh deniszh merged commit 330b27b into go-graphite:master Jul 10, 2024
8 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants