Skip to content
This repository has been archived by the owner on Jan 30, 2024. It is now read-only.

Make logrotate aware of disk space #11671

Merged
merged 1 commit into from May 9, 2022
Merged

Make logrotate aware of disk space #11671

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 10 additions & 0 deletions modules/logrotate/templates/logrotate.conf.erb
View file
Open in desktop
Expand Up @@ -43,6 +43,16 @@
# Remove empty .1 log files. There seems to be an issue where logrotate
# will refuse to rotate the logs if an empty .1 log file exists.
find <%= @matches %>* -wholename '/var/log/*' -name '*.1' -size 0 -delete
# Remove files bigger than $SIZE in chronological order while use of
# disk space on the logging partition is greater than $MAX_USE
SIZE='300M'
MAX_USE=85
ChrisBAshton marked this conversation as resolved.
Show resolved Hide resolved
for file in $(find <%= @matches %>* -type f -size +$SIZE -exec ls -tr {} +); do
USE=$(df /var/log --output=pcent | sed -n '2p' | tr -d %)
if [ $USE -gt $MAX_USE ]; then
rm $file
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand this correctly, we loop over every log file (that is more than 300M), and check if disk space is at >85%. If it is, remove the file. Once we reach <=85%, we won't be removing any more, but we'll still be looping over the rest of the files.

I think it would be logically easier to follow by inverting it like this:

while (not enough disk space)
  remove the oldest file that is >300mb

But this is quite nitpicky 😅 also not sure how slow the find query is, in which case my suggestion would be less efficient. So happy to leave as-is 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes that's the idea, and I agree the logic of the loop is a bit backward.
The reason for that is mainly to keep it simple as find gives us a nice list to go over in a simple way, using the disk space left to control the loop would make more sense but would introduce more control work as we would need some bit of logic to break the loop if we run out of files to delete.
As it is this we just get a list of files we think we can do something about, work and them, and then we've done what we could to alleviate the space issue.

fi
done;
endscript
lastaction
# Clean up files older than maximum rotation. No-op unless "rotate" value is reduced,
Expand Down