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

Add abililty to write to multiple, distinct end points #335

Closed
wants to merge 1 commit into from
Closed

Add abililty to write to multiple, distinct end points #335

wants to merge 1 commit into from

Conversation

sebito91
Copy link
Contributor

@sebito91 sebito91 commented Nov 2, 2015

We should have the ability to write to multiple end points (in this case, starting with just influxdb) without having to spin up multiple instances of the service. For example, if we want to write the same metrics from a host to a Production and a Development version of the backend cluster, we should be able to do that.

This pull request adds a config option called multi_write which effectively overrides the cluster write component by using the url list for end-points instead of parts of a cluster. Instead of randomly choosing a backend for the cluster until we commit to a write, let's just use the list and send to each and every node just sending errors in case it doesn't work.

@wrigtim
Copy link

wrigtim commented Nov 2, 2015

+1!

Spoke to @pauldix regarding similar functionality, which he believed might be possible using multiple outputs definitions. That didn't appear to work though...

[outputs]
[outputs.influxdb]
urls = ["http://devhost:8086"]
database = "telegraf"

[outputs.influxdb]
urls = ["prodhost:8086"]
database = "telegraf"

Results in:

2015/11/02 09:43:20 toml: line 37: key `urls' is in conflict with line 33

@sparrc
Copy link
Contributor

sparrc commented Nov 2, 2015

Thank you for the contribution @sebito91!

I like the feature behind this but I'm not sure I like the implementation being specific to each output sink. I would prefer if we could allow for specifying multiple outputs in a way similar to how @wrigtim described above.

I'll take a look at the config code shortly and see about the feasability of doing that. @gotyaoi was recently in the config code a lot, do you have any insight on how we might be able to do that?

@sparrc
Copy link
Contributor

sparrc commented Nov 2, 2015

@sebito91 it would be nice if you could squash those commits down :-), you should be able to do a git rebase -i HEAD~7 to get them down to a single commit, then force-push to your branch.

@gotyaoi
Copy link
Contributor

gotyaoi commented Nov 2, 2015

According to the section on tables in the toml spec, which I can't link to right now because mobile, "You cannot define any key or table more than once. Doing so is invalid." In @wrigtim's example, the toml implementation we're using appears to be fine with the multiple definitions of outputs.influxdb, but errors when trying to "reassign" urls. Offhand, I'm not sure if this is possible, but I'll give it a think.

fix indentation

updating print statement

fix error statement

refactor error return

updating toml var

remove debug language
@sebito91
Copy link
Contributor Author

sebito91 commented Nov 2, 2015

@Sparcc...squashed! Should have done that before, but was late in the night :)

As @wrigtim + @gotyaoi point out, we should be able to specify multiple backends in the outputs section but AFAICT instead of instantiating a unique instance of InfluxDB struct we try to overwrite/update the existing entry (hence the urls error listed above).

This commit takes things in a slightly different manner, in this case taking the assumption that clustering vs multiple-backends are independent configs. If we're in the former case, we're likely in 'production' mode trying to get replication working well; if we're in the latter, them we're likely in 'dev' mode trying out different versions for the backend (say two different InfluxDB releases) without having two separate telegraf instances altogether.

In this respect, this commit can be viewed another way (though not intended): each element in URLs is guaranteed to write (or failure), so this implements our clustering...poor man's method borrowed from graphite way back when. Again, not intentional...

@gotyaoi
Copy link
Contributor

gotyaoi commented Nov 2, 2015

Actually, I think that the implementation we're using has a bug. It shouldn't even get as far as the urls according to the spec. It should error out on the redefinition of [outputs.influxdb].

The only generic solution that's coming to mind would be to specify multiple outputs of the same type as an array of tables, ie.

[outputs]
[[outputs.influxdb]]
urls = ["http://devhost:8086"]
database = "telegraf"

[[outputs.influxdb]]
urls = ["prodhost:8086"]
database = "telegraf"

This currently is not supported, but wouldn't be too much work in theory.

@sparrc
Copy link
Contributor

sparrc commented Nov 2, 2015

@gotyaoi I was thinking the same thing, this would be a good idea going forward (for outputs, at least)

@sebito91
Copy link
Contributor Author

sebito91 commented Nov 3, 2015

So what do we think is the turnaround for those changes? Should we squash
these commits outright?

@gotyaoi
Copy link
Contributor

gotyaoi commented Nov 3, 2015

I'll try to put something together later today.

@sebito91
Copy link
Contributor Author

sebito91 commented Nov 6, 2015

@gotyaoi, any updates?

@sebito91
Copy link
Contributor Author

@gotyaoi, no dice?

sparrc added a commit that referenced this pull request Nov 13, 2015
This will provide the ability to specify multiple outputs for a single
type of output.

In essence, allowing this:

[outputs]

[[outputs.influxdb]]
  urls = ["udp://localhost:8089"]
  database = "udp-telegraf"

[[outputs.influxdb]]
  urls = ["http://myhost:8086"]
  database = "telegraf"

[[outputs.kafka]]
  brokers = ["192.168.99.100:9092"]
  topic = "telegraf"

closes #335
sparrc added a commit that referenced this pull request Nov 13, 2015
This will provide the ability to specify multiple outputs for a single
type of output.

In essence, allowing this:

[outputs]

[[outputs.influxdb]]
  urls = ["udp://localhost:8089"]
  database = "udp-telegraf"

[[outputs.influxdb]]
  urls = ["http://myhost:8086"]
  database = "telegraf"

[[outputs.kafka]]
  brokers = ["192.168.99.100:9092"]
  topic = "telegraf"

closes #335
sparrc added a commit that referenced this pull request Nov 13, 2015
This will provide the ability to specify multiple outputs for a single
type of output.

In essence, allowing this:

[outputs]

[[outputs.influxdb]]
  urls = ["udp://localhost:8089"]
  database = "udp-telegraf"

[[outputs.influxdb]]
  urls = ["http://myhost:8086"]
  database = "telegraf"

[[outputs.kafka]]
  brokers = ["192.168.99.100:9092"]
  topic = "telegraf"

closes #335
sparrc added a commit that referenced this pull request Nov 16, 2015
This will provide the ability to specify multiple outputs for a single
type of output.

In essence, allowing this:

[outputs]

[[outputs.influxdb]]
  urls = ["udp://localhost:8089"]
  database = "udp-telegraf"

[[outputs.influxdb]]
  urls = ["http://myhost:8086"]
  database = "telegraf"

[[outputs.kafka]]
  brokers = ["192.168.99.100:9092"]
  topic = "telegraf"

closes #335
@sparrc sparrc closed this in #370 Nov 16, 2015
allenj pushed a commit to allenj/telegraf that referenced this pull request Nov 18, 2015
This will provide the ability to specify multiple outputs for a single
type of output.

In essence, allowing this:

[outputs]

[[outputs.influxdb]]
  urls = ["udp://localhost:8089"]
  database = "udp-telegraf"

[[outputs.influxdb]]
  urls = ["http://myhost:8086"]
  database = "telegraf"

[[outputs.kafka]]
  brokers = ["192.168.99.100:9092"]
  topic = "telegraf"

closes influxdata#335
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.

4 participants