Skip to content

fix(logs): synchronise log file rotation and compression.#4417

Merged
matejk merged 1 commit intodevelfrom
2439-log-rotation-and-compression
Feb 1, 2024
Merged

fix(logs): synchronise log file rotation and compression.#4417
matejk merged 1 commit intodevelfrom
2439-log-rotation-and-compression

Conversation

@matejk
Copy link
Copy Markdown
Contributor

@matejk matejk commented Jan 25, 2024

This PR solves log file naming when compression is enabled by locking log file name rotation and compression with a mutex.

The file being compressed is renamed by prefixing to be skipped by purge. The prefix is removed at the end of compression procedure.

Purge strategy also interferes with this process and might have to be included in this locking process.

@matejk matejk requested review from aleks-f and obiltschnig January 25, 2024 17:23
@matejk matejk linked an issue Jan 25, 2024 that may be closed by this pull request
@aleks-f
Copy link
Copy Markdown
Member

aleks-f commented Jan 25, 2024

Lots of CI failures, something is not right

@matejk
Copy link
Copy Markdown
Contributor Author

matejk commented Jan 25, 2024

Unit test that I wrote is still timing sensitive. Log rotation, purge and compression has some design flaws. I wanted to check with you if the approach in draft PR is good enough as temporary improvement.

@matejk matejk force-pushed the 2439-log-rotation-and-compression branch from 77fbb1a to 90138c7 Compare January 26, 2024 09:13
@matejk
Copy link
Copy Markdown
Contributor Author

matejk commented Jan 26, 2024

The PR was updated: file being compressed is prefixed with ".~", which is removed when compressions is done.

Purge procedure was updated to sort log files names in ascending order before comparing last modification times. File with alphabetically larger file name is therefore taken in cases when multiple files have identical modification time.

It looks like the combination of mutex, renaming the compressed file and ordering files in purge process solves the issue. I was not able to reproduce anymore.

@matejk matejk force-pushed the 2439-log-rotation-and-compression branch 2 times, most recently from 727a62f to 322a649 Compare January 26, 2024 10:42
@matejk matejk marked this pull request as ready for review January 26, 2024 10:42
@matejk matejk marked this pull request as draft January 26, 2024 11:50
@matejk matejk force-pushed the 2439-log-rotation-and-compression branch 7 times, most recently from cd9dc00 to 72496f9 Compare January 29, 2024 13:10
@matejk matejk marked this pull request as ready for review January 29, 2024 18:02
@matejk matejk added this to the Release 1.13.1 milestone Jan 30, 2024
@matejk
Copy link
Copy Markdown
Contributor Author

matejk commented Jan 30, 2024

PR is ready to be reviewed.

The reason for failed task on Windows is missing compile flags for C++17 in VS files and that was resolved on branch "4368-oracle-odbc-tests".

@aleks-f
Copy link
Copy Markdown
Member

aleks-f commented Jan 30, 2024

moved this to 1.14 (interface changes and still some CI errors)

@matejk matejk force-pushed the 2439-log-rotation-and-compression branch from 72496f9 to 51f8b0b Compare January 31, 2024 21:28
@matejk matejk merged commit 9aec797 into devel Feb 1, 2024
@matejk matejk deleted the 2439-log-rotation-and-compression branch February 1, 2024 12:42
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.

Issue with log purging when FileChannel compression is enabled

2 participants