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 multiple Graphite inputs #4026

Merged
merged 2 commits into from
Sep 8, 2015
Merged

Support multiple Graphite inputs #4026

merged 2 commits into from
Sep 8, 2015

Conversation

otoolep
Copy link
Contributor

@otoolep otoolep commented Sep 7, 2015

Fixes issue #3636

The fix for this issue is simple. Don't add any config objects for Graphite, let the toml package take care of it. Sensible Graphite defaults are still set when a Graphite service is itself initialized, though a small fix needed to be made to the Graphite config code.

Tested by enabling two Graphite TCP inputs via a config file, one on port 2003, a second on port 2004. Connectivity confirmed to each using telnet, and the logs showed both starting up.

[graphite] 2015/09/06 21:27:08 Starting graphite service, batch size 1000, batch timeout 1s
[graphite] 2015/09/06 21:27:08 Listening on TCP: [::]:2003
[graphite] 2015/09/06 21:27:08 Starting graphite service, batch size 1000, batch timeout 1s
[graphite] 2015/09/06 21:27:08 Listening on TCP: [::]:2004

@otoolep
Copy link
Contributor Author

otoolep commented Sep 7, 2015

@jwilder @corylanou

@otoolep otoolep self-assigned this Sep 7, 2015
@otoolep otoolep added this to the 0.9.4 milestone Sep 7, 2015
@corylanou
Copy link
Contributor

I think we should remove this code then if we aren't going to use it. It is confusing as most of the configs use a NewConfig and I can see a future change where we think we did something but we didn't because it is no longer called.

https://github.com/influxdb/influxdb/blob/multi_graphite/services/graphite/config.go#L51

Otherwise, +1

This function is not helpful for sections of the config that support
multiple instances.
@otoolep
Copy link
Contributor Author

otoolep commented Sep 8, 2015

Unused NewConfig removed, merging on green. Thanks @corylanou

otoolep added a commit that referenced this pull request Sep 8, 2015
Support multiple Graphite inputs
@otoolep otoolep merged commit ca94ad8 into master Sep 8, 2015
@otoolep otoolep deleted the multi_graphite branch September 8, 2015 15:40
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.

None yet

2 participants