-
Notifications
You must be signed in to change notification settings - Fork 1
Description
Describe the bug
With the default configuration of totalLogsSizeKB being 10240 and fileSizeKB being 1024 according to greengrass nucleus component documentation maxHistory is then set to 10 in the PersistenceConfig.
Lines 351 to 352 in 3058bc5
| int maxHistory = Math.toIntExact((totalLogStoreSizeKB * FileSize.KB_COEFFICIENT) | |
| / (fileSizeKB * FileSize.KB_COEFFICIENT)); |
Then, since setCleanLogsByLastModifiedDate(true) is set, which I assume uses similar code to what has been proposed by @tiancishen in qos-ch/logback#763 (though I did not find the source for the logback-core 1.3.14-GG-SNAPSHOT release which is unfortunate) files are then removed based on their modified timestamp and rolling periodicity, which is hours in greengrass case.
Cleanup removes files which are older than the cleanupCutoff, calculated as
Instant cleanupCutoff = rc.getEndOfNextNthPeriod(now, getPeriodOffsetForDeletionTarget());where getPeriodOffsetForDeletionTarget depends solely on maxHistory which is 10240/1024 = 10 in the default case
protected int getPeriodOffsetForDeletionTarget() {
return -maxHistory - 1;
}To Reproduce
Create dummy log files with modified timestamps:
root@test:/greengrass/v2/logs# touch -d "13 hours ago" greengrass_2025_01_01_13_0.log
root@test:/greengrass/v2/logs# touch -d "12 hours ago" greengrass_2025_01_01_12_0.log
root@test:/greengrass/v2/logs# touch -d "11 hours ago" greengrass_2025_01_01_11_0.log
root@test:/greengrass/v2/logs# touch -d "10 hours ago" greengrass_2025_01_01_10_0.log
root@test:/greengrass/v2/logs# ls -la
total 560
drwx------ 2 root root 20480 Nov 7 10:48 .
drwxr-xr-x 12 root root 4096 Nov 7 10:46 ..
-rw-r--r-- 1 root root 0 Nov 15 2024 aws.greengrass.Nucleus.log
-rw-r--r-- 1 root root 0 Nov 7 00:48 greengrass_2025_01_01_10_0.log
-rw-r--r-- 1 root root 0 Nov 6 23:48 greengrass_2025_01_01_11_0.log
-rw-r--r-- 1 root root 0 Nov 6 22:48 greengrass_2025_01_01_12_0.log
-rw-r--r-- 1 root root 0 Nov 6 21:48 greengrass_2025_01_01_13_0.log
-rw-r--r-- 1 root root 65478 Nov 7 10:46 greengrass.log
...
restart greengrass service to trigger log rotation:
root@test:/greengrass/v2/logs# sudo systemctl restart greengrass.service
observe that the files older than 11 hours are gone:
root@test:/greengrass/v2/logs# ls -la
total 576
drwx------ 2 root root 20480 Nov 7 10:50 .
drwxr-xr-x 12 root root 4096 Nov 7 10:50 ..
-rw-r--r-- 1 root root 0 Nov 15 2024 aws.greengrass.Nucleus.log
-rw-r--r-- 1 root root 0 Nov 7 00:48 greengrass_2025_01_01_10_0.log
-rw-r--r-- 1 root root 0 Nov 6 23:48 greengrass_2025_01_01_11_0.log
-rw-r--r-- 1 root root 78719 Nov 7 10:50 greengrass.log
...
Expected behavior
Log rotation and removal should follow contract laid out in the documentation, i.e. the total size of logs should not exceed totalLogsSizeKB and each file should not exceed fileSizeKB.
I would expect all 4 files to remain after the restart in the example above as they do not exceed the default total size of 10MB and even asuming they were 1MB each, they would still fit within the limit of 10 files of 1MB each.
Actual behavior
Files are removed based on their last modified timestamp and rolling periodicity, according to maxHistory formula that assumes fixed size log files.
Additional context
Files should be removed based on their size without unexpected limit on the file age. If greengrass component only logs occasionally, log files should not be removed just because they are old if the total size is still under the limit. If a component needs to be able to limit log age, a separate configuration option should be provided for that.
In case size-based cleanup is not feasible, at least the documentation should be updated to reflect the actual behavior.