[Twisted-Python] Log Rotation Blocks w/ Large Number of Log Files

Hello, Twistd blocks for a long period of time when the number of log files built up is large, thereby causing connections to freeze and (depending on the length of the rotation) time out. Given I've only used Twisted in the past and not contributed to it, I was curious if someone could take a look at my changes to twisted.python.logfile.LogFile's rotate() method and see if there's any reason the changes would be a bad idea. What I've attempted to do is defer the rotation of the old log files to a thread, and only rotate the current log file in the foreground. Original twisted.python.logfile.LogFile.rotate(): http://gist.github.com/334538 My version with deferral of old log rotation: http://gist.github.com/334545 (Changes are against Twisted 10.0). Thank you in advance. -J

On Tuesday 16 March 2010, Jason J. W. Williams wrote:
Twistd blocks for a long period of time when the number of log files built up is large, thereby causing connections to freeze and (depending on the length of the rotation) time out.
Given I've only used Twisted in the past and not contributed to it, I was curious if someone could take a look at my changes to twisted.python.logfile.LogFile's rotate() method and see if there's any reason the changes would be a bad idea.
What I've attempted to do is defer the rotation of the old log files to a thread, and only rotate the current log file in the foreground.
An alternative approach could be to name the log files differently. For example, use the timestamp of the rotation as a filename suffix: twistd.log.2010-03-16.23-56 This is not the traditional way logging is done, but it would get good performance with simple code. (It would still need resolution of duplicate file names, in case the log rotates very quickly or the clock is moved backwards.) Bye, Maarten

True. Technically, you could also just kick it out to syslog and let logrotate.d do the rotation for you. The code I put up had a major "concurrency" error in that multiple rotation threads could be launched and conflict with each other (try to remove the same file). Updated the Gist to deal with this: http://gist.github.com/334545 -J On Tue, Mar 16, 2010 at 5:01 PM, Maarten ter Huurne <maarten@treewalker.org> wrote:
On Tuesday 16 March 2010, Jason J. W. Williams wrote:
Twistd blocks for a long period of time when the number of log files built up is large, thereby causing connections to freeze and (depending on the length of the rotation) time out.
Given I've only used Twisted in the past and not contributed to it, I was curious if someone could take a look at my changes to twisted.python.logfile.LogFile's rotate() method and see if there's any reason the changes would be a bad idea.
What I've attempted to do is defer the rotation of the old log files to a thread, and only rotate the current log file in the foreground.
An alternative approach could be to name the log files differently. For example, use the timestamp of the rotation as a filename suffix: twistd.log.2010-03-16.23-56
This is not the traditional way logging is done, but it would get good performance with simple code. (It would still need resolution of duplicate file names, in case the log rotates very quickly or the clock is moved backwards.)
Bye, Maarten
_______________________________________________ Twisted-Python mailing list Twisted-Python@twistedmatrix.com http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python

On Mar 16, 2010, at 5:34 PM, Jason J. W. Williams wrote:
Given I've only used Twisted in the past and not contributed to it, I was curious if someone could take a look at my changes to twisted.python.logfile.LogFile's rotate() method and see if there's any reason the changes would be a bad idea.
The right way to ask this question is to open a ticket on twistedmatrix.com and put it into review (attach the 'review' keyword). More information is available here: <http://twistedmatrix.com/trac/wiki/ContributingToTwistedLabs>. Otherwise, unless someone has a chance to look at it right away (which is always unlikely) it will languish in the mailing list archive forever. Be sure to check back on the ticket when you receive feedback, as approximately 0% of suggested patches are actually accepted as-is without some cleanup :). Thanks!

As requested: http://twistedmatrix.com/trac/ticket/4372 Thank you for taking a look at it. -J On Wed, Mar 17, 2010 at 10:12 AM, Glyph Lefkowitz <glyph@twistedmatrix.com> wrote:
On Mar 16, 2010, at 5:34 PM, Jason J. W. Williams wrote:
Given I've only used Twisted in the past and not contributed to it, I was curious if someone could take a look at my changes to twisted.python.logfile.LogFile's rotate() method and see if there's any reason the changes would be a bad idea.
The right way to ask this question is to open a ticket on twistedmatrix.com and put it into review (attach the 'review' keyword). More information is available here: <http://twistedmatrix.com/trac/wiki/ContributingToTwistedLabs>. Otherwise, unless someone has a chance to look at it right away (which is always unlikely) it will languish in the mailing list archive forever. Be sure to check back on the ticket when you receive feedback, as approximately 0% of suggested patches are actually accepted as-is without some cleanup :). Thanks!
_______________________________________________ Twisted-Python mailing list Twisted-Python@twistedmatrix.com http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
participants (3)
-
Glyph Lefkowitz
-
Jason J. W. Williams
-
Maarten ter Huurne