Skip to content

Improve size tracking consistency in rotating_file_sink#3628

Closed
sage-mode-hunter wants to merge 6 commits into
gabime:v1.xfrom
sage-mode-hunter:improve-rotating-file-sink-size-tracking
Closed

Improve size tracking consistency in rotating_file_sink#3628
sage-mode-hunter wants to merge 6 commits into
gabime:v1.xfrom
sage-mode-hunter:improve-rotating-file-sink-size-tracking

Conversation

@sage-mode-hunter

Copy link
Copy Markdown
Contributor

rotating_file_sink uses an internal cached size counter to track file growth and determine when log rotation should occur.

In environments where files may be externally modified (log rotation tools, truncation, or unexpected writes), this cached value can become temporarily inconsistent with the actual file size.

This change adds a lightweight resynchronization after rotation to ensure internal state remains consistent with the underlying file.

No change to rotation behavior or public API.

Comment thread include/spdlog/sinks/daily_file_sink.h Outdated
break;
}
if (filenames.empty() && !path_exists(new_filename)) {
break;

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this logic is wrong. and not related to the pr. pls revert it

file_helper_.flush();
if (file_helper_.size() > 0) {
rotate_();
current_size_ = file_helper_.size();

@gabime gabime Jun 26, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need to call twice file_helper_.size(). just store into var the first result and use it. logic is wrong amyway, sinice lime 125 sets again the current size

@sage-mode-hunter sage-mode-hunter Jun 26, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review , I’ve removed the unrelated daily_file_sink change and updated rotating_file_sink to avoid duplicate size calls as suggested.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry this doesnt make any sense to me. Now size() is called in each call. Closing.

@sage-mode-hunter sage-mode-hunter force-pushed the improve-rotating-file-sink-size-tracking branch from dde7152 to 21a555c Compare June 26, 2026 18:08
@gabime gabime closed this Jun 26, 2026
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.

2 participants