Added cleanup for old RRD files to daily.sh #3907

Merged
merged 6 commits into from Aug 1, 2016

Projects

None yet

5 participants

@laf
Member
laf commented Jul 30, 2016

Fix #2388

laf added some commits Jul 30, 2016
@laf laf Added cleanup for old RRD files 318c9cd
@laf laf Updated docs for rrd_purge option
1741231
@laf laf added the Core label Jul 30, 2016
@f0o
Member
f0o commented Jul 31, 2016

👍

@paulgear
Member

My thoughts:

  1. If it's set to 0, it should skip the check. I think if it is run with the setting at 0 at the moment, it will delete all RRD files.
  2. Once the above is fixed, the config.php.default setting should be 0. It's easy to clean up data you don't care about if you run out of space, but it's impossible to recreate data you do care about but has already been deleted.
  3. The comment should be consistent with the others in config.php.default (before the check, and # style).
@laf laf Updated as per pr comments
5a7a606
@laf
Member
laf commented Jul 31, 2016

Done

@paulgear paulgear and 1 other commented on an outdated diff Aug 1, 2016
@@ -31,6 +31,14 @@
exit(0);
}
+if ($options['f'] === 'rrd_purge') {
+ if (is_numeric($config['rrd_purge']) && $config['rrd_purge'] > 0) {
+ $cmd = "find ".$config['rrd_dir']." -mtime +".$config['rrd_purge']." -exec rm -Rf {} \;";
+ $purge = `$cmd`;
+ echo 'purged old rrd files';
@paulgear
paulgear Aug 1, 2016 edited Member

Might be nice to echo $purge here so people can know what was removed.

@laf
laf Aug 1, 2016 Member

Done

@laf laf Updated rrd_purge to echo removed files/folders
f733859
@murrant murrant and 1 other commented on an outdated diff Aug 1, 2016
@@ -31,6 +31,16 @@
exit(0);
}
+if ($options['f'] === 'rrd_purge') {
+ if (is_numeric($config['rrd_purge']) && $config['rrd_purge'] > 0) {
+ $cmd = "find ".$config['rrd_dir']." -mtime +".$config['rrd_purge']." -exec rm -Rf {} \;";
@murrant
murrant Aug 1, 2016 edited Contributor

This doesn't print output, you'll need to add -print in the find command.
You should also print why you are listing those files...

@laf
laf Aug 1, 2016 Member

Done.

@laf laf Added -print to find + description of why we have deleted files
b0d05ad
@murrant
Contributor
murrant commented Aug 1, 2016

👍 from me, but do you think we need to mention it in the docs, or is the line in config.php.default enough?

@laf laf Updated docs to be clearer
1d2c8f5
@laf
Member
laf commented Aug 1, 2016

I've added a bit more to try and be clearer but yes we should mention as much as possible in docs to save queries.

@murrant murrant merged commit f4fb470 into librenms:master Aug 1, 2016

1 check was pending

Auto-Deploy Building
Details
@laf laf deleted the laf:issue-2388 branch Aug 1, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment