Skip to content

RotatingFileHandler formats the record twice #116267

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

Open
serhiy-storchaka opened this issue Mar 3, 2024 · 1 comment
Open

RotatingFileHandler formats the record twice #116267

serhiy-storchaka opened this issue Mar 3, 2024 · 1 comment
Labels
performance Performance or resource usage

Comments

@serhiy-storchaka
Copy link
Member

serhiy-storchaka commented Mar 3, 2024

RotatingFileHandler calls self.format() twice for every record -- first time to determine if rollover should occur, second time to write it to the stream. It is not optimal. Even if the IO time dominates in common cases, the formatting time may be non-trivial -- this is why formatting is delayed.

It is not easy to fix without duplicating the code and introducing a coupling between several handler classes. The simplest way -- check the current file size in shouldRollover(), without adding the size of the new record. This will make the size of backups slightly larger than maxBytes, but this calculation is already inaccurate (see #64336, #69128). This will automatically solve #116263.

Linked PRs

@Jason-Y-Z
Copy link
Contributor

Thanks for raising it, the idea makes sense to me, but I guess we might need a wider consensus on this.
Nevertheless, I had a go at the implementation in #116330, which might help with the discussion

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Performance or resource usage
Projects
Status: No status
Development

No branches or pull requests

2 participants