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.

Sign up for GitHub

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

Jump to bottom

[feature request] allow for mixed units when specifying retention policy durations #3634

Closed
olivierHa opened this issue Aug 12, 2015 · 9 comments
Closed

[feature request] allow for mixed units when specifying retention policy durations #3634

olivierHa opened this issue Aug 12, 2015 · 9 comments
Assignees
@jsternberg
Labels
RFC
Milestone
1.1.0

Comments

@olivierHa
Copy link

Hello,

On Influxdb version 0.9.2, when creating a policy retention like :

create RETENTION POLICY "1w" ON test DURATION 1w REPLICATION 1 DEFAULT

It creates :

/opt/influxdb/influx -execute 'show  RETENTION POLICIEs ON test'
name        duration    replicaN    default
default     0       1       false
1w      168h0m0s    1       true

So 1w has been converted to 168h0m0s. Ok this seems legit :)

But we can't create policy retention with this value :(

/opt/influxdb/influx -execute 'create RETENTION POLICY "1w" ON test DURATION 168h0m0s REPLICATION 1 DEFAULT'
ERR: error parsing query: found 0m, expected REPLICATION at line 1, char 52
Warning: It is possible this error is due to not setting a database.
Please set a database with the command "use <database>".

/opt/influxdb/influx -execute 'create RETENTION POLICY "1w" ON test DURATION "168h0m0s" REPLICATION 1 DEFAULT'
ERR: error parsing query: found 168h0m0s, expected duration at line 1, char 48
Warning: It is possible this error is due to not setting a database.
Please set a database with the command "use <database>".

"168h" works, but we should be able to give the same exact "duration" as shown in "show retention policy"

A side effect (I guess) is that you can't give '24h30m' as a duration

/opt/influxdb/influx -execute 'create RETENTION POLICY "test2" ON test DURATION "168h45m" REPLICATION 1 DEFAULT'
ERR: error parsing query: found 168h45m, expected duration at line 1, char 47
Warning: It is possible this error is due to not setting a database.
Please set a database with the command "use <database>".

/opt/influxdb/influx -execute 'create RETENTION POLICY "test2" ON test DURATION 168h45m REPLICATION 1 DEFAULT'
ERR: error parsing query: found 45m, expected REPLICATION at line 1, char 51
Warning: It is possible this error is due to not setting a database.
Please set a database with the command "use <database>".

Regards

Olivier
@beckettsean beckettsean changed the title Retention duration issue [feature request] allow for mixed units when specifying retention policy durations Aug 12, 2015
@beckettsean beckettsean added this to the Longer term milestone Aug 12, 2015
@beckettsean
Copy link
Contributor

The fairly easy workaround is to provide the retention policy duration in a single unit rather than mixed units.

@olivierHa
Copy link
Author

It is easy to workaround, but we should be able to give the same 'value' given by the "show" command when creating a retention policy :

/opt/influxdb/influx -execute 'create RETENTION POLICY "testmix" ON test DURATION 10125m REPLICATION 1 DEFAULT'

gives :

/opt/influxdb/influx -execute 'show retention policies on test'
name    duration    replicaN    default
default 0        1      false
testmix     168h45m0s   1   true

But we can't create a policy with "168h45m0s" as value for duration ....

Olivier

@beckettsean
Copy link
Contributor

@olivierHa I do understand your point. I'm not disputing whether it's useful, I'm merely saying it's not a bug, just slightly annoying behavior with a trivial workaround. Please notice I left the issue open and changed it into a feature request to deliver the functionality you requested.

@olivierHa
Copy link
Author

Thanks.
I am developing a puppet module to install influx, that's why i was looking for "reusable" value for idempotency. As a workaround on my side, i am converting everything to seconds.

@lmx1989219
Copy link

why me set my custom retetion policy default then i can not cross cli query series?So i set default to default and the series i can query too.

@beckettsean
Copy link
Contributor

@lmx1989219 your question is unclear, and I do not believe it relates in any way to specifying retention policy durations in mixed units. Please send a note to the mailing list influxdb@googlegroups.com if oyu have questions.

@lmx1989219
Copy link

@beckettsean
thanks for your suggest,and now the reason is that i write data with influxdbAPI without specify the retention name.

@jsternberg jsternberg mentioned this issue May 11, 2016
Merged
@jsternberg jsternberg modified the milestones: 1.0.0, Longer term May 11, 2016
@jsternberg jsternberg self-assigned this May 11, 2016
@jsternberg jsternberg modified the milestones: Future Point Release, 1.0.0 May 11, 2016
jsternberg added a commit that referenced this issue May 11, 2016
@jsternberg
Support mixed duration units
b0bc3ef 
It is now possible to use a mixed duration unit like `1h30m`. The
duration units can be in whatever order as long as they are connected to
each other.

There is a change to the scanner. A token such as `10x` will be scanned
as a duration literal, but will then fail to parse as an invalid
duration. This should not be a breaking change as there is no situation
where `10m10` was a valid order of tokens for the parser.

Fixes #3634.
jsternberg added a commit that referenced this issue May 13, 2016
@jsternberg
Support mixed duration units
7835a14 
It is now possible to use a mixed duration unit like `1h30m`. The
duration units can be in whatever order as long as they are connected to
each other.

There is a change to the scanner. A token such as `10x` will be scanned
as a duration literal, but will then fail to parse as an invalid
duration. This should not be a breaking change as there is no situation
where `10m10` was a valid order of tokens for the parser.

Fixes #3634.
jsternberg added a commit that referenced this issue May 16, 2016
@jsternberg
Support mixed duration units
7afa6af 
It is now possible to use a mixed duration unit like `1h30m`. The
duration units can be in whatever order as long as they are connected to
each other.

There is a change to the scanner. A token such as `10x` will be scanned
as a duration literal, but will then fail to parse as an invalid
duration. This should not be a breaking change as there is no situation
where `10m10` was a valid order of tokens for the parser.

Fixes #3634.
jsternberg added a commit that referenced this issue Jun 24, 2016
@jsternberg
Support mixed duration units
e9693be 
It is now possible to use a mixed duration unit like `1h30m`. The
duration units can be in whatever order as long as they are connected to
each other.

There is a change to the scanner. A token such as `10x` will be scanned
as a duration literal, but will then fail to parse as an invalid
duration. This should not be a breaking change as there is no situation
where `10m10` was a valid order of tokens for the parser.

Fixes #3634.
jsternberg added a commit that referenced this issue Aug 10, 2016
@jsternberg
Support mixed duration units
ab049d7 
It is now possible to use a mixed duration unit like `1h30m`. The
duration units can be in whatever order as long as they are connected to
each other.

There is a change to the scanner. A token such as `10x` will be scanned
as a duration literal, but will then fail to parse as an invalid
duration. This should not be a breaking change as there is no situation
where `10m10` was a valid order of tokens for the parser.

Fixes #3634.
@beckettsean
Copy link
Contributor

@jsternberg this is fantastic! I assume that the change is universal within InfluxQL, and not just for retention policy durations?

@jsternberg
Copy link
Contributor

It should be universal.

@beckettsean beckettsean mentioned this issue Sep 26, 2016
Closed
@timhallinflux timhallinflux added this to the 1.1.0 milestone Dec 19, 2016
@timhallinflux timhallinflux removed this from the Future Point Release milestone Dec 19, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jsternberg jsternberg

Labels
RFC
Projects
None yet
Milestone
1.1.0
Development

No branches or pull requests
5 participants
@olivierHa @jsternberg @beckettsean @lmx1989219 @timhallinflux