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

Handle int and int32 types correctly. #2366

Closed
wants to merge 1 commit into from
Closed

Handle int and int32 types correctly. #2366

wants to merge 1 commit into from

Conversation

jtakkala
Copy link
Contributor

Self monitoring enabled results in a panic since #2261 was introduced. I first tried to cast everything to int64 in server.go (eg. GoMaxProcs, PID, NumCPU, etc.), but then realized that in influxql/ast.go that Number types are permitted to be int, int32, and int64; so I added this type switch which I believe solves the problem.

goroutine 238 [running]:
github.com/influxdb/influxdb.(*FieldCodec).EncodeFields(0xc2080cf9c0, 0xc2081a53e0, 0x0, 0x0, 0x0, 0x0, 0x0)
    /Users/jari/go/src/github.com/influxdb/influxdb/database.go:816 +0xb55
github.com/influxdb/influxdb.func·035(0x0, 0x0)
    /Users/jari/go/src/github.com/influxdb/influxdb/server.go:1818 +0x75d
github.com/influxdb/influxdb.(*Server).WriteSeries(0xc20805ea00, 0x9c3bf0, 0x9, 0x9a0dd0, 0x7, 0xc2081c4000, 0x7, 0x8, 0x0, 0x0, ...)
    /Users/jari/go/src/github.com/influxdb/influxdb/server.go:1836 +0xa80
github.com/influxdb/influxdb.func·011()
    /Users/jari/go/src/github.com/influxdb/influxdb/server.go:410 +0x7fe
created by github.com/influxdb/influxdb.(*Server).StartSelfMonitoring
    /Users/jari/go/src/github.com/influxdb/influxdb/server.go:412 +0x241

@otoolep
Copy link
Contributor

otoolep commented Apr 21, 2015

How did you reproduce this crash? When I run monitoring on a 64-bit Ubuntu box, it all works fine.

I am running the latest code on master.

@jtakkala
Copy link
Contributor Author

I have tried both the 0.9.0-rc25 RPM and building from master myself. I'm running it in a container on CentOS (64-bit). Under the [monitoring] section of influxdb.conf I have enabled=true. No other config changes (write-interval is still "1m").

@jtakkala
Copy link
Contributor Author

My original fix is here, https://github.com/influxdb/influxdb/compare/master...jtakkala:monitoring-int64-fix?expand=1 I'm also cross compiling on OS X with "GOOS=linux GOARCH=amd64 go build ./...". Go version 1.4.2.

@otoolep
Copy link
Contributor

otoolep commented Apr 21, 2015

Looks like one of the InfluxDB team has hit this now too.

#2368

@jtakkala -- we'll take a closer look now, thanks. Can you please sign the CLA? http://influxdb.com/community/cla.html

@otoolep
Copy link
Contributor

otoolep commented Apr 21, 2015

OK, I've repro'ed this.

@toddboom
Copy link
Contributor

@jtakkala if you can sign the CLA, we'll get this merged in. thanks!

@otoolep
Copy link
Contributor

otoolep commented Apr 21, 2015

Closed in favour of #2376, which will include an integration test and changelog update.

@otoolep otoolep closed this Apr 21, 2015
mark-rushakoff pushed a commit that referenced this pull request Jan 9, 2019
Wire up Storage Engine to API-layer BucketService
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.

3 participants