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

Separated params for backup and tidy cronjobs - Fixes #3 #4

Merged
merged 5 commits into from Jan 6, 2022

Conversation

gmwubs
Copy link

@gmwubs gmwubs commented Feb 9, 2021

In manifests/config.pp the module creates a cronjob with the following oneliner:
command => "/usr/bin/find ${influxdb::backup_directory} -mtime +${influxdb::backup_keep} -type f -delete";

If backup_keep is set to 2, this creates something like:
/usr/bin/find /var/lib/influxdb/backups -mtime +2 -type f -delete

This is caused by the cronjobs for backup and tidy being created to run at the exact same time. In certain circumstances, this results in the the backup location running low on disk space, triggering "false alerts".

The solution introduced here separates parameters for both cron jobs, avoiding a race condition between the two cron jobs.

@gmwubs gmwubs changed the title Separated params for backup and tidy cronjobs Separated params for backup and tidy cronjobs - Fixes #3 Feb 9, 2021
@xandm
Copy link
Contributor

xandm commented Feb 9, 2021

Thanks - can you also update data/common.yaml (lines 11 and 12) to have new default settings?

@gmwubs
Copy link
Author

gmwubs commented Feb 9, 2021

Of course! Sorry I didn't think of that!

@xandm
Copy link
Contributor

xandm commented Feb 10, 2021

I don't mind merging it in the current state but wonder if the daily_backup_hour should be later than daily_tidy_hour so tidy happens first?

@gmwubs
Copy link
Author

gmwubs commented Feb 10, 2021

At first, I thought it was perhaps best to create a backup first. On second thought: if the space available is not enough, the backup will fail. I would suggest people make sure that they do off-line backups, for example by creating snapshots or syncing the locally created backups elsewhere :-)

@gmwubs
Copy link
Author

gmwubs commented Nov 18, 2021

Any indication when this PR will be accepted?

@xandm xandm merged commit 22c19b2 into kcl-nmssys:master Jan 6, 2022
@gmwubs gmwubs deleted the issue2_separate_params_cron branch March 8, 2022 12:36
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