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

rrdcreate: support scale suffix for heartbeat/steps/rows #468

Merged
merged 2 commits into from May 16, 2014

Conversation

Projects
None yet
2 participants
@pabigot
Contributor

pabigot commented May 15, 2014

This feature allows this opaque create specification:

rrdtool create power1.rrd \
  --start 0 --step 1 \
  DS:watts:GAUGE:300:0:24000 \
  RRA:AVERAGE:0.5:1:864000 \
  RRA:AVERAGE:0.5:60:129600 \
  RRA:AVERAGE:0.5:3600:13392 \
  RRA:AVERAGE:0.5:86400:3660

to be expressed with more understandable durations:

rrdtool create power2.rrd \
  --start 0 --step 1s \
  DS:watts:GAUGE:5m:0:24000 \
  RRA:AVERAGE:0.5:1s:10d \
  RRA:AVERAGE:0.5:1m:90d \
  RRA:AVERAGE:0.5:1h:18M \
  RRA:AVERAGE:0.5:1d:10y
rrdcreate: support scale suffix for heartbeat/steps/rows
This feature allows this opaque create specification:

  rrdtool create power1.rrd \
    --start 0 --step 1 \
    DS:watts:GAUGE:300:0:24000 \
    RRA:AVERAGE:0.5:1:864000 \
    RRA:AVERAGE:0.5:60:129600 \
    RRA:AVERAGE:0.5:3600:13392 \
    RRA:AVERAGE:0.5:86400:3660

to be expressed with more understandable durations:

  rrdtool create power2.rrd \
    --start 0 --step 1s \
    DS:watts:GAUGE:5m:0:24000 \
    RRA:AVERAGE:0.5:1s:10d \
    RRA:AVERAGE:0.5:1m:90d \
    RRA:AVERAGE:0.5:1h:18M \
    RRA:AVERAGE:0.5:1d:10y

Signed-off-by: Peter A. Bigot <pab@pabigot.com>
@oetiker

This comment has been minimized.

Show comment
Hide comment
@oetiker

oetiker May 16, 2014

Owner

neat ...
looking through the patch it seems that you are not raising any flags when someone tries to create an impossible RRA but just let rounding do its job ... Or did I miss this ?
I would like to see rrd complain and refuse to proceed.

Owner

oetiker commented May 16, 2014

neat ...
looking through the patch it seems that you are not raising any flags when someone tries to create an impossible RRA but just let rounding do its job ... Or did I miss this ?
I would like to see rrd complain and refuse to proceed.

@pabigot

This comment has been minimized.

Show comment
Hide comment
@pabigot

pabigot May 16, 2014

Contributor

Do you want a warning, or an error, or a new flag to allow the user to select the behavior?

Contributor

pabigot commented May 16, 2014

Do you want a warning, or an error, or a new flag to allow the user to select the behavior?

@pabigot

This comment has been minimized.

Show comment
Hide comment
@pabigot

pabigot May 16, 2014

Contributor

Let me rephrase, since in fact you were clear and I'm not waking up fast enough: do you want the ability to make it be a warning instead of an error? I think there may be a value in having rrdcreate do the rounding.

Contributor

pabigot commented May 16, 2014

Let me rephrase, since in fact you were clear and I'm not waking up fast enough: do you want the ability to make it be a warning instead of an error? I think there may be a value in having rrdcreate do the rounding.

rrdcreate: improve scale support
- Fix missed diagnostic for negative values accepted by strtoul.
- Add detailed diagnostics for parsing failures.
- Add diagnostic when duration-based RRA steps (rows) are not multiples of
  PDP interval (RRA span).
- Fix row count when PDP interval not one second.
- Extend documentation to special-function RRA example.
@pabigot

This comment has been minimized.

Show comment
Hide comment
@pabigot

pabigot May 16, 2014

Contributor

I've added a patch that I think does what you want and agree it's an improvement. Now this:

rrdtool create foo.rrd \
  --start now \
  --step 10s \
  DS:v:GAUGE:5:U:U \
  RRA:AVERAGE:0.5:15s:1h

produces:

ERROR: Invalid step 15s: value would truncate when scaled

and

rrdtool create foo.rrd \
  --start now \
  --step 5s \
  DS:v:GAUGE:5:U:U \
  RRA:AVERAGE:0.5:15s:10s

produces:

ERROR: Invalid row count 10s: value would truncate when scaled

Diagnostics for other invalid values are also enhanced:

rrdtool create foo.rrd \
  --start now \
  --step -1 \
  DS:v:GAUGE:xff:U:U \
  RRA:AVERAGE:0.5:1:100

produces:

ERROR: step size: value must be (suffixed) positive number

The patch also fixes a few other issues and applies the new feature to the HWPREDICT example.

Let me know if you have other concerns you'd like addressed. I had been writing really ugly shell scripts to do the math for me, but seeing how cleanly it was done in carbon's storage-schemas.conf inspired me to add direct support in rrdcreate instead.

Contributor

pabigot commented May 16, 2014

I've added a patch that I think does what you want and agree it's an improvement. Now this:

rrdtool create foo.rrd \
  --start now \
  --step 10s \
  DS:v:GAUGE:5:U:U \
  RRA:AVERAGE:0.5:15s:1h

produces:

ERROR: Invalid step 15s: value would truncate when scaled

and

rrdtool create foo.rrd \
  --start now \
  --step 5s \
  DS:v:GAUGE:5:U:U \
  RRA:AVERAGE:0.5:15s:10s

produces:

ERROR: Invalid row count 10s: value would truncate when scaled

Diagnostics for other invalid values are also enhanced:

rrdtool create foo.rrd \
  --start now \
  --step -1 \
  DS:v:GAUGE:xff:U:U \
  RRA:AVERAGE:0.5:1:100

produces:

ERROR: step size: value must be (suffixed) positive number

The patch also fixes a few other issues and applies the new feature to the HWPREDICT example.

Let me know if you have other concerns you'd like addressed. I had been writing really ugly shell scripts to do the math for me, but seeing how cleanly it was done in carbon's storage-schemas.conf inspired me to add direct support in rrdcreate instead.

@oetiker

This comment has been minimized.

Show comment
Hide comment
@oetiker

oetiker May 16, 2014

Owner

I like it ! thanks!

Owner

oetiker commented May 16, 2014

I like it ! thanks!

oetiker added a commit that referenced this pull request May 16, 2014

Merge pull request #468 from pabigot/enh-create-with-dur
rrdcreate: support scale suffix for heartbeat/steps/rows

@oetiker oetiker merged commit 223201b into oetiker:master May 16, 2014

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment