search

bookingcom / carbonapi

High-performance Graphite frontend in Go
Other
81 stars 23 forks source link

Another attempt to fix Consolidate panic #486

Closed deniszh closed 1 year ago

deniszh commented 1 year ago

What issue is this change attempting to solve?

It's rare but still.

http: panic serving 127.0.0.1:44020: runtime error: slice bounds out of range [:-9223372036854775808]
goroutine 1134454440 [running]:
net/http.(*conn).serve.func1()
        /usr/lib64/go/src/net/http/server.go:1850 +0xbf
panic({0xd4bea0, 0xc3c0dc5b00})
        /usr/lib64/go/src/runtime/panic.go:890 +0x262
github.com/bookingcom/carbonapi/pkg/expr/types.(*MetricData).Consolidate(0xc076caf520, 0x8000000000000000)
        /usr/local/git_tree/gopath/src/github.com/bookingcom/carbonapi/pkg/expr/types/types.go:295 +0x6f0
github.com/bookingcom/carbonapi/pkg/expr/types.ConsolidateJSON(0x42b, {0xc1c75fbf00, 0xf, 0x2000100?})
        /usr/local/git_tree/gopath/src/github.com/bookingcom/carbonapi/pkg/expr/types/types.go:130 +0x190
github.com/bookingcom/carbonapi/pkg/app/carbonapi.(*App).renderWriteBody(0xc00025b800?, {0xc1c75fbf00?, 0xf, 0x10}, {{0xc35e47bf20, 0x3, 0x3}, {0xc066986052, 0xa}, {0xc066986063>
        /usr/local/git_tree/gopath/src/github.com/bookingcom/carbonapi/pkg/app/carbonapi/http_handlers.go:708 +0xcb
github.com/bookingcom/carbonapi/pkg/app/carbonapi.(*App).renderHandler(0xc00025b800, {0xee25b8, 0xc41a0dcf80}, 0xc304c82100, 0x1651240?)
        /usr/local/git_tree/gopath/src/github.com/bookingcom/carbonapi/pkg/app/carbonapi/http_handlers.go:313 +0x1933
github.com/bookingcom/carbonapi/pkg/app/carbonapi.(*App).validateRequest.func1({0xee25b8, 0xc41a0dcf80}, 0xc304c82100)

How does this change solve the problem? Why is this the best approach?

I'm not sure that's the best. Crash happening on bookingcom/carbonapi/pkg/expr/types/types.go:295

val, abs := ret.AggregateFunction(v[:valuesPerPoint], absent[:valuesPerPoint])

But valuesPerPoint coming into Consolidate function as int, and according to stacktrace

github.com/bookingcom/carbonapi/pkg/expr/types.(*MetricData).Consolidate(0xc0a86755f0, 0x8000000000000000)

so in https://github.com/bookingcom/carbonapi/blob/master/pkg/expr/types/types.go#L130

            ret[i] = r.Consolidate(int(valuesPerPoint))

Not sure how int(valuesPerPoint) could become 0x8000000000000000 = 9223372036854775808 - it's more then int size, but I just decided to check that valuesPerPoint > 0 just before slice.