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

fix mt-store-cat nilpointer #758

Merged
merged 1 commit into from
Nov 13, 2017
Merged

fix mt-store-cat nilpointer #758

merged 1 commit into from
Nov 13, 2017

Conversation

Dieterbe
Copy link
Contributor

@Dieterbe Dieterbe commented Nov 8, 2017

the provided context must contain a span

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x28 pc=0x7d3c2d]

goroutine 1 [running]:
github.com/raintank/metrictank/tracing.NewSpan(0xc94ec0, 0xc4200d6008, 0xc94680, 0xce80f0, 0xa0e4c5, 0x1a, 0x4bd0d3, 0x989da0, 0xc420fa2220, 0x99)
/home/ubuntu/.go_workspace/src/github.com/raintank/metrictank/tracing/tracing.go:19 +0x6d
github.com/raintank/metrictank/mdata.(*CassandraStore).SearchTable(0xc42046e1e0, 0xc94ec0, 0xc4200d6008, 0xc420a0fb90, 0x26, 0xc4205b82a0, 0x9, 0x5a0349e65a01f866, 0x0, 0x0, ...)
/home/ubuntu/.go_workspace/src/github.com/raintank/metrictank/mdata/store_cassandra.go:416 +0xce
main.points(0xc42046e1e0, 0xc4201e4380, 0x5, 0x8, 0xc4209a9780, 0x1, 0x1, 0x5a0349e65a01f866, 0x0)
/home/ubuntu/.go_workspace/src/github.com/raintank/metrictank/cmd/mt-store-cat/series.go:25 +0x3c6
main.main()
/home/ubuntu/.go_workspace/src/github.com/raintank/metrictank/cmd/mt-store-cat/main.go:251 +0x15b7

the provided context must contain a span

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x28 pc=0x7d3c2d]

goroutine 1 [running]:
github.com/raintank/metrictank/tracing.NewSpan(0xc94ec0, 0xc4200d6008, 0xc94680, 0xce80f0, 0xa0e4c5, 0x1a, 0x4bd0d3, 0x989da0, 0xc420fa2220, 0x99)
	/home/ubuntu/.go_workspace/src/github.com/raintank/metrictank/tracing/tracing.go:19 +0x6d
github.com/raintank/metrictank/mdata.(*CassandraStore).SearchTable(0xc42046e1e0, 0xc94ec0, 0xc4200d6008, 0xc420a0fb90, 0x26, 0xc4205b82a0, 0x9, 0x5a0349e65a01f866, 0x0, 0x0, ...)
	/home/ubuntu/.go_workspace/src/github.com/raintank/metrictank/mdata/store_cassandra.go:416 +0xce
main.points(0xc42046e1e0, 0xc4201e4380, 0x5, 0x8, 0xc4209a9780, 0x1, 0x1, 0x5a0349e65a01f866, 0x0)
	/home/ubuntu/.go_workspace/src/github.com/raintank/metrictank/cmd/mt-store-cat/series.go:25 +0x3c6
main.main()
	/home/ubuntu/.go_workspace/src/github.com/raintank/metrictank/cmd/mt-store-cat/main.go:251 +0x15b7
@@ -9,7 +10,7 @@ import (
"github.com/grafana/metrictank/mdata"
)

func chunkSummary(store *mdata.CassandraStore, tables []string, metrics []Metric, keyspace, groupTTL string) error {
func chunkSummary(ctx context.Context, store *mdata.CassandraStore, tables []string, metrics []Metric, keyspace, groupTTL string) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

that ctx doesn't seem to be used

Copy link
Contributor Author

Choose a reason for hiding this comment

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

for consistency, all these functions that go out and collect and print data get a context arg

@@ -247,12 +256,15 @@ func main() {

fmt.Printf("# Keyspace %q:\n", *cassandraKeyspace)

span := tracer.StartSpan("mt-store-cat " + format)
Copy link
Contributor

Choose a reason for hiding this comment

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

isn't the only purpose of initializing the tracer just to prevent stuff from calling methods on a nil value? why bother starting a span then?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the span needs to be non-nil otherwise we get a panic.
plus also i envision at some point we'll actually start collecting traces from this tool.

@replay
Copy link
Contributor

replay commented Nov 10, 2017

ok, that looks good to me. shall i merge?

@Dieterbe
Copy link
Contributor Author

Yes please or approve it thanks

@Dieterbe Dieterbe merged commit 2f305e0 into master Nov 13, 2017
@Dieterbe Dieterbe deleted the fix-mt-store-cat branch September 18, 2018 08:59
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants