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

Support for Graphite limited to 1 input #3636

Closed
otoolep opened this issue Aug 12, 2015 · 8 comments
Closed

Support for Graphite limited to 1 input #3636

otoolep opened this issue Aug 12, 2015 · 8 comments
Assignees
Milestone

Comments

@otoolep
Copy link
Contributor

otoolep commented Aug 12, 2015

While the Graphite configuration indicates that multiple Graphite inputs are supported, this isn't actually the case. Attempting to enable more than 1 results in a panic:

using configuration at: /home/philip/config.sample.toml
panic: reflect: slice index out of range

goroutine 1 [running]:
reflect.Value.Index(0x8e1280, 0xc20806c0d8, 0xd7, 0x1, 0x0, 0x0, 0x0)
        /usr/local/go/src/reflect/value.go:818 +0x149
github.com/BurntSushi/toml.(*MetaData).unifySliceArray(0xc208050910, 0x8e2060, 0xc20801ffa0, 0x57, 0x8e1280, 0xc20806c0d8, 0xd7, 0x0, 0x0)
        /home/philip/repos/influxdb/src/github.com/BurntSushi/toml/decode.go:321 +0x137
github.com/BurntSushi/toml.(*MetaData).unifySlice(0xc208050910, 0x8e2060, 0xc20801ffa0, 0x8e1280, 0xc20806c0d8, 0xd7, 0x0, 0x0)
        /home/philip/repos/influxdb/src/github.com/BurntSushi/toml/decode.go:314 +0x25f
github.com/BurntSushi/toml.(*MetaData).unify(0xc208050910, 0x8e2060, 0xc20801ffa0, 0x8e1280, 0xc20806c0d8, 0xd7, 0x0, 0x0)
        /home/philip/repos/influxdb/src/github.com/BurntSushi/toml/decode.go:206 +0xd52
github.com/BurntSushi/toml.(*MetaData).unifyStruct(0xc208050910, 0x8f5b00, 0xc20800af00, 0xaa69e0, 0xc20806c000, 0xd9, 0x0, 0x0)
        /home/philip/repos/influxdb/src/github.com/BurntSushi/toml/decode.go:252 +0x7c9
github.com/BurntSushi/toml.(*MetaData).unify(0xc208050910, 0x8f5b00, 0xc20800af00, 0xaa69e0, 0xc20806c000, 0xd9, 0x0, 0x0)
        /home/philip/repos/influxdb/src/github.com/BurntSushi/toml/decode.go:200 +0xdea
github.com/BurntSushi/toml.Decode(0xc208076800, 0x1734, 0x8bdac0, 0xc20804e020, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
        /home/philip/repos/influxdb/src/github.com/BurntSushi/toml/decode.go:114 +0x225
github.com/BurntSushi/toml.DecodeFile(0x7fffe4e9adc1, 0x1f, 0x8bdac0, 0xc20804e020, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
        /home/philip/repos/influxdb/src/github.com/BurntSushi/toml/decode.go:124 +0x132
github.com/influxdb/influxdb/cmd/influxd/run.(*Command).ParseConfig(0xc2080a0300, 0x7fffe4e9adc1, 0x1f, 0x0, 0x0, 0x0)
        /home/philip/repos/influxdb/src/github.com/influxdb/influxdb/cmd/influxd/run/command.go:196 +0x328
github.com/influxdb/influxdb/cmd/influxd/run.(*Command).Run(0xc2080a0300, 0xc20800a010, 0x2, 0x2, 0x0, 0x0)
        /home/philip/repos/influxdb/src/github.com/influxdb/influxdb/cmd/influxd/run/command.go:77 +0x32f
main.(*Main).Run(0xc208037b40, 0xc20800a010, 0x2, 0x2, 0x0, 0x0)
        /home/philip/repos/influxdb/src/github.com/influxdb/influxdb/cmd/influxd/main.go:81 +0x3f8
main.main()
        /home/philip/repos/influxdb/src/github.com/influxdb/influxdb/cmd/influxd/main.go:42 +0xdc

This is because of this line:

c.Graphites = append(c.Graphites, graphite.NewConfig())

but removing this line means the Graphite input is not configured with the defaults. To support multiple Graphite inputs, defaults are going to have to be set by the Graphite input code itself and not by `NewConfig'.

@noroute
Copy link

noroute commented Aug 24, 2015

Does this have a target release?

And: would more than one graphite instance be able to share a database (e.g. tcp+udp)?

@otoolep
Copy link
Contributor Author

otoolep commented Aug 24, 2015

@noroute -- there is no target yet for this release. We don't know of an important use case for this functionality. Do you have one that you can share with us?

To answer your second question, a TCP and UDP input could write to the same database when this feature is implemented. There will be no limit to the number that can write to a single database.

@beckettsean beckettsean added this to the 0.9.4 milestone Aug 25, 2015
@pauldix
Copy link
Member

pauldix commented Aug 25, 2015

Pretty sure this is already done. see https://github.com/influxdb/influxdb/blob/master/etc/config.sample.toml#L121-L127

Although it does highlight an important problem with that sample config. Where's the specification of the database?

@otoolep
Copy link
Contributor Author

otoolep commented Aug 25, 2015

@pauldix -- it's done, but doesn't work. if one adds more than 1 input, the system panics.

@noroute
Copy link

noroute commented Aug 29, 2015

@otoolep Our use case is a large bunch of applications writing to influxdb some of which we'd like to migrate to UDP for performance reasons. Supporting TCP and UDP would make for a much simpler migration path.

@otoolep
Copy link
Contributor Author

otoolep commented Aug 31, 2015

OK, thanks for the use case @noroute

@aheusingfeld
Copy link

We don't know of an important use case for this functionality. Do you have one that you can share with us?

My "important use case" is that I have multiple applications sending their metrics via UDP and I need to

  1. send these metrics to different databases
  2. add different tags to these metrics
  3. apply a different template to these metrics

Unfortunately most 3rd-party libraries still didn't adopt the API changes you introduced with 0.9 so the Graphite API is the only working fallback there is at the moment. Alternatives welcome!

@otoolep
Copy link
Contributor Author

otoolep commented Sep 7, 2015

Fixed by #4026

otoolep added a commit that referenced this issue Sep 7, 2015
@otoolep otoolep closed this as completed Sep 8, 2015
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

No branches or pull requests

5 participants