fix: infer log rotation start index from oldest file on startup#18789
Open
glyh wants to merge 7 commits intocompatiblefrom
Open
fix: infer log rotation start index from oldest file on startup#18789glyh wants to merge 7 commits intocompatiblefrom
glyh wants to merge 7 commits intocompatiblefrom
Conversation
Instead of always starting curr_index at 0, scan existing rotated log files and pick the oldest (by mtime) or first empty slot. This ensures round-robin overwrites the oldest log rather than blindly starting from index 0 on each restart. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
glyh
force-pushed
the
lyh/proper-infer-of-log-rotation-index
branch
from
7ac008e to
70adba0
Compare
Member
Author
|
!ci-build-me |
Replace access+stat with a single Option.try_with wrapping stat, eliminating the race window where a file could disappear between the existence check and the mtime read. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace imperative for loop and usage of special float constants with List.init to build mtime option list, List.findi to locate the first empty slot, and List.min_elt to find the oldest existing file. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Use Fun.protect instead of try/with for guaranteed temp dir removal - Extract with_temp_dir helper to avoid duplicating cleanup code Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
georgeee
approved these changes
Apr 21, 2026
Member
|
!ci-build-me |
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.
This PR fixes log rotation resume behavior for
Dumb_logrotate.Before this change, after daemon restart rotation always resumed at index
.0, which could overwrite newer rotated logs first.Now startup infers the next rotation index by scanning existing rotated files (
<log>.0..N) and selecting the oldest file (or the first missing slot). Rotation remains round-robin; we only changed the starting point after restart.Also included:
changes/18789.mdsrc/lib/logger/test/logger_test.ml(resumes rotation from oldest log)Validation:
dune test src/lib/logger/passes (2 tests).