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

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

Merged
merged 1 commit into from
Feb 1, 2024

Conversation

matejk
Copy link
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 obiltschnig and aleks-f 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
Member

aleks-f commented Jan 25, 2024

Lots of CI failures, something is not right

@matejk
Copy link
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
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
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
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
29 of 30 checks passed
@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