Add size based log rotation to butil/logging - #41
Closed
cw20050111-prog wants to merge 1 commit into
Closed
Conversation
Add --log_rotate_size_mb (0, disabled by default) and --log_rotate_max_backups (10). When the current log file is about to grow beyond the limit it is renamed to "<log_file>.1", the older backups are shifted down and the ones beyond the limit are removed, so ".1" is always the most recent backup. The check lives in Log2File(), the single funnel of both DefaultLogSink and AsyncLogger::DoLog, so rotation covers synchronous and asynchronous logging alike and stays orthogonal to any later LogStream refactor. The file is opened in append mode, so its initial size is read with fstat()/GetFileSizeEx() instead of being counted from zero. A log larger than the limit is written as a whole rather than producing empty backups. The size that triggered a failed rotation does not change by itself, so retrying on the next log would close, reopen and rename the file for every single log. Instead the size at which rotation failed is remembered and rotation is retried once the file has grown by another --log_rotate_size_mb, with the error printed once rather than per log. A file smaller than it was at that point has been replaced from the outside, which lets rotation resume immediately. Sharing one log file between several processes is not supported together with rotation and is documented as such: on POSIX LoggingLock only serializes the threads of a single process, and after the rename the other processes would keep writing to the renamed inode. Running an external rotator such as logrotate on the same file is unsupported for the same reason.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addresses the log rotation part of apache#2696.
What
butil/logginghas no rotation today, which is one of the reasons users fall back to glog. This adds size based rotation, disabled by default.--log_rotate_size_mb--log_rotate_max_backupsBoth are registered with
BUTIL_VALIDATE_GFLAG, so they are modifiable at run time through/flags. The names deliberately avoid glog's--max_log_sizeso that there is no collision underBRPC_WITH_GLOG.When the limit would be exceeded,
<log_file>is renamed to<log_file>.1, the existing backups are shifted down (.1->.2-> ...) and the ones beyond--log_rotate_max_backupsare removed, so.1is always the most recent backup.Where the check lives
In
Log2File(). It is the single funnel for both the synchronous path (DefaultLogSink) and the asynchronous one (AsyncLogger::DoLog), so one check covers both, and it sits in the file writing layer rather than in a sink, which keeps it orthogonal to a possible futureLogStream/LogStreamFactoryrefactor.Details worth calling out:
"a", so its initial size is read withfstat()(GetFileSizeEx()on Windows) rather than being counted from zero after a restart.When the rename fails
The size that triggered a failed rotation does not change by itself, so retrying on the next log would close, reopen and rename the file for every single log. Instead the size at which rotation failed is remembered and rotation is retried only once the file has grown by another
--log_rotate_size_mb. The error is printed once rather than per log, and logs keep being appended to the current file in the meantime.If the file turns out to be smaller than it was when rotation failed, it has been replaced or truncated from the outside and rotation resumes immediately. This is what happens when an external tool renames the log file away: the next write recreates it, and rotation recovers on its own instead of staying disabled for the lifetime of the process.
Running an external rotator such as logrotate on the same file is documented as unsupported, since the two sets of rules interfere and the number of files kept is no longer bounded by
--log_rotate_max_backups.Multiple processes
Sharing one log file between several processes is documented as unsupported together with rotation, as agreed in apache#2696: on POSIX
LoggingLockonly serializes the threads of a single process, and after the rename the other processes would keep writing to the renamed inode. Addingflockwas considered and deliberately left out.Tests
LogRotationTestintest/logging_unittest.cc, 8 cases: disabled by default, rotation and pruning of old backups,--log_rotate_max_backups=1, a log larger than the limit, the size of an existing file being picked up after a restart, the asynchronous path, backing off when the rename keeps failing, and recovering after the log file is replaced from the outside. The assertions do not depend on how much the rest of the process logs into the same file. The wholetest_butilsuite passes on x86_64 in CI and on aarch64 (openEuler 24.03, gcc 12.3).Besides the unit tests, the change was exercised on a 308 core aarch64 machine (openEuler 24.03, gcc 12.3) with a multi threaded load generator whose every log carries a thread id and a sequence number:
--log_rotate_size_mbflipped through/flagswhile running--log_rotate_max_backupslowered from 10 to 2 through/flagsThe Windows half of the change has no CI anywhere in the project, so it was cross compiled with mingw-w64 (
-DUNICODE -Wall, clean) as a compile time check. It has not been run on Windows.Left out on purpose
Time based rotation, compression of old files and a "current" symlink, so that this first change stays reviewable.