Skip to content

Commit

Permalink
store: fix division by zero with query-pushdown (#5185)
Browse files Browse the repository at this point in the history
Do not divide by zero with query-pushdown by checking whether StepMillis
is not zero before using it.

Fixes the following bug:

```
goroutine 6108358 [running]:
runtime/debug.Stack()
	/usr/lib/go-1.17/src/runtime/debug/stack.go:24 +0x65
github.com/thanos-io/thanos/pkg/server/grpc.New.func1({0x1a8e700, 0x32500a0})
	/home/giedrius/dev/thanos/pkg/server/grpc/grpc.go:70 +0xdb
github.com/grpc-ecosystem/go-grpc-middleware/v2/interceptors/recovery.WithRecoveryHandler.func1.1({0x1, 0x10000000040d0e5}, {0x1a8e700, 0x32500a0})
	/home/giedrius/go/pkg/mod/github.com/grpc-ecosystem/go-grpc-middleware/v2@v2.0.0-rc.2.0.20201207153454-9f6bf00c00a7/interceptors/recovery/options.go:33 +0x2d
github.com/grpc-ecosystem/go-grpc-middleware/v2/interceptors/recovery.recoverFrom({0x2160af0, 0xc00119dad0}, {0x1a8e700, 0x32500a0}, 0x8)
	/home/giedrius/go/pkg/mod/github.com/grpc-ecosystem/go-grpc-middleware/v2@v2.0.0-rc.2.0.20201207153454-9f6bf00c00a7/interceptors/recovery/interceptors.go:53 +0x36
github.com/grpc-ecosystem/go-grpc-middleware/v2/interceptors/recovery.StreamServerInterceptor.func1.1()
	/home/giedrius/go/pkg/mod/github.com/grpc-ecosystem/go-grpc-middleware/v2@v2.0.0-rc.2.0.20201207153454-9f6bf00c00a7/interceptors/recovery/interceptors.go:41 +0x7c
panic({0x1a8e700, 0x32500a0})
	/usr/lib/go-1.17/src/runtime/panic.go:1038 +0x215
github.com/thanos-io/thanos/pkg/store.(*PrometheusStore).Series(0xc000fe5720, 0xc0015d1980, {0x2181280, 0xc00025c070})
	/home/giedrius/dev/thanos/pkg/store/prometheus.go:171 +0xfbd
github.com/thanos-io/thanos/pkg/store/storepb._Store_Series_Handler({0x1c87a40, 0xc000fe5720}, {0x21728a8, 0xc00119dc80})
	/home/giedrius/dev/thanos/pkg/store/storepb/rpc.pb.go:1027 +0xd0
github.com/grpc-ecosystem/go-grpc-middleware/v2/interceptors.StreamServerInterceptor.func1({0x1c87a40, 0xc000fe5720}, {0x21728a8, 0xc00119dc50}, 0xc0001172a8, 0x1e00ba0)
	/home/giedrius/go/pkg/mod/github.com/grpc-ecosystem/go-grpc-middleware/v2@v2.0.0-rc.2.0.20201207153454-9f6bf00c00a7/interceptors/server.go:35 +0x2ac
github.com/grpc-ecosystem/go-grpc-middleware/v2.ChainStreamServer.func1.1.1({0x1c87a40, 0xc000fe5720}, {0x21728a8, 0xc00119dc50})
	/home/giedrius/go/pkg/mod/github.com/grpc-ecosystem/go-grpc-middleware/v2@v2.0.0-rc.2.0.20201207153454-9f6bf00c00a7/chain.go:51 +0x3a
github.com/grpc-ecosystem/go-grpc-middleware/v2/interceptors.StreamServerInterceptor.func1({0x1c87a40, 0xc000fe5720}, {0x2172818, 0xc001083ea0}, 0xc0001172a8, 0xc001083e00)
	/home/giedrius/go/pkg/mod/github.com/grpc-ecosystem/go-grpc-middleware/v2@v2.0.0-rc.2.0.20201207153454-9f6bf00c00a7/interceptors/server.go:35 +0x2ac
github.com/thanos-io/thanos/pkg/tracing.StreamServerInterceptor.func1({0x1c87a40, 0xc000fe5720}, {0x21728a8, 0xc00119db90}, 0x1c145a0, 0x2144001)
	/home/giedrius/dev/thanos/pkg/tracing/grpc.go:42 +0x19b
github.com/grpc-ecosystem/go-grpc-middleware/v2.ChainStreamServer.func1.1.1({0x1c87a40, 0xc000fe5720}, {0x21728a8, 0xc00119db90})
	/home/giedrius/go/pkg/mod/github.com/grpc-ecosystem/go-grpc-middleware/v2@v2.0.0-rc.2.0.20201207153454-9f6bf00c00a7/chain.go:51 +0x3a
github.com/grpc-ecosystem/go-grpc-middleware/v2/interceptors.StreamServerInterceptor.func1({0x1c87a40, 0xc000fe5720}, {0x2172938, 0xc0001172c0}, 0xc0001172a8, 0xc001083e20)
	/home/giedrius/go/pkg/mod/github.com/grpc-ecosystem/go-grpc-middleware/v2@v2.0.0-rc.2.0.20201207153454-9f6bf00c00a7/interceptors/server.go:35 +0x2ac
github.com/grpc-ecosystem/go-grpc-middleware/v2.ChainStreamServer.func1.1.1({0x1c87a40, 0xc000fe5720}, {0x2172938, 0xc0001172c0})
	/home/giedrius/go/pkg/mod/github.com/grpc-ecosystem/go-grpc-middleware/v2@v2.0.0-rc.2.0.20201207153454-9f6bf00c00a7/chain.go:51 +0x3a
github.com/grpc-ecosystem/go-grpc-prometheus.(*ServerMetrics).StreamServerInterceptor.func1({0x1c87a40, 0xc000fe5720}, {0x2172db8, 0xc0011be6c0}, 0x40d0b4, 0xc001083e40)
	/home/giedrius/go/pkg/mod/github.com/grpc-ecosystem/go-grpc-prometheus@v1.2.0/server_metrics.go:121 +0x109
github.com/grpc-ecosystem/go-grpc-middleware/v2.ChainStreamServer.func1.1.1({0x1c87a40, 0xc000fe5720}, {0x2172db8, 0xc0011be6c0})
	/home/giedrius/go/pkg/mod/github.com/grpc-ecosystem/go-grpc-middleware/v2@v2.0.0-rc.2.0.20201207153454-9f6bf00c00a7/chain.go:51 +0x3a
github.com/grpc-ecosystem/go-grpc-middleware/v2/interceptors/recovery.StreamServerInterceptor.func1({0x1c87a40, 0xc000fe5720}, {0x2172db8, 0xc0011be6c0}, 0x8, 0xc0001172a8)
	/home/giedrius/go/pkg/mod/github.com/grpc-ecosystem/go-grpc-middleware/v2@v2.0.0-rc.2.0.20201207153454-9f6bf00c00a7/interceptors/recovery/interceptors.go:45 +0xa7
github.com/grpc-ecosystem/go-grpc-middleware/v2.ChainStreamServer.func1.1.1({0x1c87a40, 0xc000fe5720}, {0x2172db8, 0xc0011be6c0})
	/home/giedrius/go/pkg/mod/github.com/grpc-ecosystem/go-grpc-middleware/v2@v2.0.0-rc.2.0.20201207153454-9f6bf00c00a7/chain.go:51 +0x3a
github.com/grpc-ecosystem/go-grpc-middleware/v2.ChainStreamServer.func1({0x1c87a40, 0xc000fe5720}, {0x2172db8, 0xc0011be6c0}, 0x199c180, 0xc001593900)
	/home/giedrius/go/pkg/mod/github.com/grpc-ecosystem/go-grpc-middleware/v2@v2.0.0-rc.2.0.20201207153454-9f6bf00c00a7/chain.go:60 +0xbf
google.golang.org/grpc.(*Server).processStreamingRPC(0xc000143dc0, {0x218a9e8, 0xc0012de300}, 0xc0009e2360, 0xc001046a20, 0x32572c0, 0x0)
	/home/giedrius/go/pkg/mod/google.golang.org/grpc@v1.40.0/server.go:1557 +0xe9a
google.golang.org/grpc.(*Server).handleStream(0xc000143dc0, {0x218a9e8, 0xc0012de300}, 0xc0009e2360, 0x0)
	/home/giedrius/go/pkg/mod/google.golang.org/grpc@v1.40.0/server.go:1630 +0x9e5
google.golang.org/grpc.(*Server).serveStreams.func1.2()
	/home/giedrius/go/pkg/mod/google.golang.org/grpc@v1.40.0/server.go:941 +0x98
created by google.golang.org/grpc.(*Server).serveStreams.func1
	/home/giedrius/go/pkg/mod/google.golang.org/grpc@v1.40.0/server.go:939 +0x294
```

Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>
  • Loading branch information
GiedriusS authored and matej-g committed Feb 25, 2022
1 parent 6ce09b8 commit 498f580
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion pkg/store/prometheus.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ func (p *PrometheusStore) Series(r *storepb.SeriesRequest, s storepb.Store_Serie
// when it gets retrieved by the upper layer's PromQL engine.
// This also is necessary when Sidecar uploads a block and then availableMinTime
// becomes a fixed timestamp.
if r.QueryHints != nil {
if r.QueryHints != nil && r.QueryHints.StepMillis != 0 {
diff := availableMinTime - r.MinTime
r.MinTime += (diff / r.QueryHints.StepMillis) * r.QueryHints.StepMillis
// Add one more to strictly fit within --min-time -> infinity.
Expand Down

0 comments on commit 498f580

Please sign in to comment.