Repository navigation
count only the current date's backups when RollingFileAppender starts - #334
Merged
FreeAndNil merged 4 commits intoOct 10, 2026
Merged
Conversation
On start-up the appender counts the size backups already on disk and continues from the highest. It read the number after the last "." of any matching file, which went wrong in three ways: - a date pattern without separators (.yyyyMMdd) was read as a backup number: counting up the next backup was named .20261004, counting down every size roll walked twenty million names - with staticLogFileName false, a date pattern that supplies the extension (.yyyy-MM-dd'.log') and preserveLogFileNameExtension, the current date's backups were skipped as "from a different date period", so the count restarted at 0 and the next roll deleted the existing backups - with a static file name, backups already rolled to an earlier date raised the current date's count GetBackupIndex now builds the name the current date's backups must have, as the appender names them, and counts only files of that name plus .N. With a static file name, a suffix that parses as a date in DatePattern is a dated file, not a backup. A date pattern that formats as a short number (.dd, .MM, .HH, .yyyy) names the dated file log.txt.10 exactly like backup 10, so one overwrites the other. For such configurations no numbered backups are made: size rolling is switched off, the existing file is appended to at start-up when appendToFile is false, and the reason is written as a FATAL line at the top of each file and as an internal warning. The new IgnoreDateWarnings property keeps the configured behaviour for those who accept the risk. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
FreeAndNil
added a commit
that referenced
this pull request
Oct 4, 2026
…334 InitializeRollBackups took dated files and other dates' backups for backups of the current roll point: * A date without separators (.yyyyMMdd) parsed as a backup index, with a static or a non-static name, so the count jumped to 20261004. * With a static name, log.txt.20261003.1 counted as backup 1. * A date pattern supplying the extension (.yyyy-MM-dd'.log' with PreserveLogFileNameExtension) hid the current date's backups. Dates of up to four characters (.dd) still count as indexes, they cannot be told apart. Tests by @Snotface.
InitializeRollBackups took dated files and other dates' backups for backups of the current roll point: * A date without separators (.yyyyMMdd) parsed as a backup index, with a static or a non-static name, so the count jumped to 20261004. * With a static name, log.txt.20261003.1 counted as backup 1. * A date pattern supplying the extension (.yyyy-MM-dd'.log' with PreserveLogFileNameExtension) hid the current date's backups. Dates of up to four characters (.dd) still count as indexes, they cannot be told apart. Tests by @Snotface.
Contributor
|
Thanks @Snotface, the backup counting bugs you found are real and your The PR is more than we want in
Here is a smaller fix for the same bugs on top of your branch, with your tests and a changelog entry: ba511c5 Its diff shows what changed against your version. Could you merge or cherry-pick it into your branch and push? It applies without conflicts, and we squash when merging the PR. The short date collision ( One more request: please keep PR descriptions and changelog entries short. A few lines on what is broken and how it is fixed are much easier to review than a long write-up. |
Contributor
|
@Snotface are you still interested in merging this PR? |
Contributor
|
@Snotface I will close this PR on 2026-10-19 when we get no feedback from you. |
Contributor
Author
|
Sorry, i was away on a conference. Will go through your points and see what i can do. I will provide feedback. |
Contributor
Author
|
Thanks, merged and pushed. Builds clean, tests pass. |
FreeAndNil
approved these changes
Oct 10, 2026
FreeAndNil
left a comment
FreeAndNil
left a comment
Contributor
There was a problem hiding this comment.
Thanks for your contribution.
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
2 participants
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.
RollingFileAppender miscounted backups at start-up: a
.yyyyMMdddate was read as a backup index, other dates' backups raised the count, and a date pattern supplying the extension hid the current date's backups. It now counts only the current date's backups.