Enable file compression on file rotation #694
Conversation
WIP Make dlt files compressed on file rotation so the current file is uncompressed but all other are compressed. This is so all files can be copied without issues as the current file otherwise is likely to be truncated.
To enable compilation without gzip support
Fix closing of files in error case Change to Gzip OFF when disabled Fix function parameter gzfile
|
Hello @averater, |
|
Do you want to combine #691 into this PR? or we need to review and merge 691 PR first and then you rebase this PR later |
Done |
I'm not sure how the review process will go for this commit. If there are only minor changes needed then we can abandon 691. But if there are more things needed or if this takes to much time then I think we can get 691 in first as that should be trivial. And then rebase this one on top of 691 once it is merged (that will be an easy rebase). |
|
Hello @averater |
Adds check in dlt_logstorage_sync_to_file that gzlog is NULL if the dlt-daemon is compiled with gzip, otherwise that statement will always return true if using gzip-compression on logs.
dlt_logstorage_log_file_name assumed the suffix always is .dlt which is not be the case if gzip compression is enabled. Changed to assume .dlt.gz when smax is calculated as it prevents buffer overflow if filenames are long and gzip is enabled.
@minminlittleshrimp done |
|
Please commit with sign-off. Thanks. |
| tmp = &config->records; | ||
| while ((*tmp)->next != NULL) { | ||
| int len = strlen((*tmp)->name); | ||
| const char *suffix = ".dlt"; |
There was a problem hiding this comment.
I need more context/info/description about this feature:
- Is it trying to do a dynamic activate compress mode, e.g. logging .dlt and then immediately switch to .dlt.gz using compress function for file/fd? If not, we just be fine with .gz file count from 0 or 1 since .gz and .dlt are already different file types.
- If it does so, why dont we just apply compress function in native logstorage file rotation, adding new func here seems doing a duplicated task?
- Minor suggestion: adding macro name FILE cannot directly inform user/dev that it is for file rotation, any better naming scheme here?
There was a problem hiding this comment.
-
The feature it to log normal dlt and only when a file is "full" it compress it. This way the stored files are compressed but the current file is uncompressed. This is for the use case of copying stored files while logging is in progress.
I think the purpose of removing older files and having a max number of files is to not fill storage. Then it would make sense to bunch dlt and gz.dlt together into one count. -
It is quite different tasks to compress into an open file and to compress an entire file. I thought this way was the easiest and cleanest way.
-
I thought of it but couldn't figure out a better name that is nice and short. Maybe ARCHIVE is better?
Add option to have dlt files compressed on file rotation so the current file stays uncompressed but all other are compressed. This is so all files can be copied without issues as the current file otherwise is likely to be truncated.