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

Change default retention policy from "default" to "autogen" #53

Open
cslysy opened this issue Dec 12, 2016 · 5 comments
Open

Change default retention policy from "default" to "autogen" #53

cslysy opened this issue Dec 12, 2016 · 5 comments

Comments

@cslysy
Copy link

cslysy commented Dec 12, 2016

New version of influxdb use "autogen" as default retention policy name - please see influxdata/influxdb#3733 for more info.

Plugin configuration need to be adjusted from:

# The retention policy to use
  config :retention_policy, :validate => :string, :default => "default"

to

# The retention policy to use
  config :retention_policy, :validate => :string, :default => "autogen"
@notfol
Copy link

notfol commented Feb 8, 2017

+1

@PauloAugusto-Asos
Copy link

I think I would prefer to have the "default" Retention Policy Name be something that indicates that it should be changed. Ex: "Change-Me".

That way while forgetting so change it, one won't automatically send it to the wrong RP. (as a rule, we should create a new RP and use that one instead)

But I'm a bit divided by that. If that's the case, it should no longer be an "optional" parameter. Not being an optional parameter it makes it harder to accidentally send the data to the "wrong" RP but it also makes it a wee bit harder to make it work out of the box.

As far as I'm concerned, I prefer for this parameter to no longer be optional and have the config template propose the value of "autogen" so that you can just uncomment it.

@Ultimation
Copy link
Contributor

This looks done in 8d2324a ?

@sslupsky
Copy link

I am going to chime in with a discussion about what is meant by the "default" retention policy. Influx defines the "default" retention policy as the policy that is specified as the default when you create the retention policy. That is, a database can have multiple retention policies and one of those retention policies can be set as the "default".

The Influx output plugin defines the "default" retention policy as something that is not specified. This is not consistent with the influx "default" so I propose this plugin be changed to be consistent with the Influx definition. That is, if no retention policy is specified in the logstash configuration then the data is sent to whatever the "default" is set by influx.

This is opposed to the current situation where the retention policy is not specified, then the retention policy is forced to "autogen" and potentially overrides the default setting in Influx.

Philosophically speaking, I want to decision about which retention policy the data goes to to be determined by Influx, not logstash, unless explicitly specified in logstash.

So, I propose the following:

config :retention_policy, :validate => :string, :default => "autogen"

be changed to:

config :retention_policy, :validate => :string, :default => nil

@sslupsky
Copy link

This would be a breaking change for anyone where the retention policy was not specified in logstash and the default setting set by Influx is not "autogen".

If we do not want a breaking change, then I propose we create another parameter boolean "use_default_retention_policy". If true, then the retention_policy is set to nil so that Influx will use the default. If false, the retention_policy is set to what ever is specified by "retention_policy".

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