SiftingAppender and RollingFileAppender, roll on close.
Hi logback users! After using logback for a few years I've just started trying to use sifting appender. I want to have two messagepricer.${logfileType}.${HOSTNAME}.log (where logfileType is main or workers) (effectivitly statically named) and many msg-123456789.log (dynamically named) logs, one for each message processed. In general this works, apart from two problems: 1) dynamically named logs don't get rolled (they not renamed to dated form and gz into archive dir) 2) the log file is not closed and when I try to manually rename in Windows Explorer I'm told I can't, because of the write lock. Of course I've searched StackOverflow.com, and searched the web, and read the source code, btu nothing has lead me to a solution. So I did a build of trunk (head 3cd6c8ca1142573a2b69e8d2309b31216d764f15) to get logback-1.0.12-SNAPSHOT. I think problem (2) is caused by SiftingAppender waiting 30 mins deciding logs are stale. Maybe this could be configurable. If I make a patch would it be accepted? It has been asked for before, see [1], [2] and [3]. Problem (1) seems to be because when SiftingAppender closes old RollingFileAppenders, the rollover() is not called, and the file just closed as is. I guess this is because during JVM shutdown closing should be fast, and not trigger a rollover? Other people have hit this before, see [4], so it seems like a commonly asked for enhancement. Is there a chance we could add config to RollingFileAppender to enable roll-on-close? You can see my logback.xml at [5]. Apart from my suggested enhancements, is there a way to fix this? Something I've missed Also?: 3) It would be nice if AsyncAppender could fan out to mulitple appenders. Since it only supports one appender I have to have two AsyncAppenders each feeding a different SiftingAppender. References [1] http://logback.10977.n7.nabble.com/how-to-remove-old-log-files-in-this-sifti... [2] http://jira.qos.ch/browse/LOGBACK-244 [3] http://jira.qos.ch/browse/LOGBACK-724 [4] http://mailman.qos.ch/pipermail/logback-user/2011-November/002706.html [5] https://gist.github.com/diroussel/5363736 -- View this message in context: http://logback.10977.n7.nabble.com/SiftingAppender-and-RollingFileAppender-r... Sent from the Users mailing list archive at Nabble.com.
I wrote a patch to make the sifting appender timeout configurable and submitted a pull request. I've since closed the request since I moved the changes to a different branch, and because the maintainer didn't show much interest in accepting it. My patch also contains the ability to specify a hard limit on how many sub-appenders can be open simultaneously, which I intended as a fail-safe to prevent Logback from consuming an unbounded number of file descriptors (since, if you run out your application can fail in many places completely unrelated to logging). You can find the branch here: https://github.com/twbecker/logback/tree/siftingAppender -----Original Message----- From: Logback-user [mailto:logback-user-bounces@qos.ch] On Behalf Of diroussel Sent: Thursday, April 11, 2013 10:42 AM To: logback-user@qos.ch Subject: [logback-user] SiftingAppender and RollingFileAppender, roll on close. Hi logback users! After using logback for a few years I've just started trying to use sifting appender. I want to have two messagepricer.${logfileType}.${HOSTNAME}.log (where logfileType is main or workers) (effectivitly statically named) and many msg-123456789.log (dynamically named) logs, one for each message processed. In general this works, apart from two problems: 1) dynamically named logs don't get rolled (they not renamed to dated form and gz into archive dir) 2) the log file is not closed and when I try to manually rename in Windows Explorer I'm told I can't, because of the write lock. Of course I've searched StackOverflow.com, and searched the web, and read the source code, btu nothing has lead me to a solution. So I did a build of trunk (head 3cd6c8ca1142573a2b69e8d2309b31216d764f15) to get logback-1.0.12-SNAPSHOT. I think problem (2) is caused by SiftingAppender waiting 30 mins deciding logs are stale. Maybe this could be configurable. If I make a patch would it be accepted? It has been asked for before, see [1], [2] and [3]. Problem (1) seems to be because when SiftingAppender closes old RollingFileAppenders, the rollover() is not called, and the file just closed as is. I guess this is because during JVM shutdown closing should be fast, and not trigger a rollover? Other people have hit this before, see [4], so it seems like a commonly asked for enhancement. Is there a chance we could add config to RollingFileAppender to enable roll-on-close? You can see my logback.xml at [5]. Apart from my suggested enhancements, is there a way to fix this? Something I've missed Also?: 3) It would be nice if AsyncAppender could fan out to mulitple appenders. Since it only supports one appender I have to have two AsyncAppenders each feeding a different SiftingAppender. References [1] http://logback.10977.n7.nabble.com/how-to-remove-old-log-files-in-this-sifti... [2] http://jira.qos.ch/browse/LOGBACK-244 [3] http://jira.qos.ch/browse/LOGBACK-724 [4] http://mailman.qos.ch/pipermail/logback-user/2011-November/002706.html [5] https://gist.github.com/diroussel/5363736 -- View this message in context: http://logback.10977.n7.nabble.com/SiftingAppender-and-RollingFileAppender-r... Sent from the Users mailing list archive at Nabble.com. _______________________________________________ Logback-user mailing list Logback-user@qos.ch http://mailman.qos.ch/mailman/listinfo/logback-user
Thomas, That functionality looks good. In order to get accepted, maybe you can make your changes to AppenderTrackerImpl less invasive? In AppenderTrackerImpl, you are subclassing LinkedHashMap instead of using ch.qos.logback.classic.pattern.LRUCache. But really there is no need for a map. Just keep the existing head and tail references, keep a count variablle. If the count gets to high, then remove entries from the tail. I like that you've added unit tests and the code formatting is consistent with the rest of the code base. David -- View this message in context: http://logback.10977.n7.nabble.com/SiftingAppender-and-RollingFileAppender-r... Sent from the Users mailing list archive at Nabble.com.
There was already a Map there. To your point though, that bit of rework to AppenderTrackerImpl was not strictly necessary but I felt the existing code, which essentially re-implemented LinkedHashMap in a clumsy way was pretty low hanging fruit. I'd be willing to make additional changes if the maintainer showed interest in accepting the patch, but my immediate need has passed. My observation has been that the majority of the pull requests to Logback seem to get no comments at all. But I went ahead and reopened this one here: https://github.com/qos-ch/logback/pull/107 -Tommy ________________________________________ From: Logback-user [logback-user-bounces@qos.ch] on behalf of diroussel [nabble@diroussel.xsmail.com] Sent: Friday, April 12, 2013 12:12 PM To: logback-user@qos.ch Subject: Re: [logback-user] SiftingAppender and RollingFileAppender, roll on close. Thomas, That functionality looks good. In order to get accepted, maybe you can make your changes to AppenderTrackerImpl less invasive? In AppenderTrackerImpl, you are subclassing LinkedHashMap instead of using ch.qos.logback.classic.pattern.LRUCache. But really there is no need for a map. Just keep the existing head and tail references, keep a count variablle. If the count gets to high, then remove entries from the tail. I like that you've added unit tests and the code formatting is consistent with the rest of the code base. David -- View this message in context: http://logback.10977.n7.nabble.com/SiftingAppender-and-RollingFileAppender-r... Sent from the Users mailing list archive at Nabble.com. _______________________________________________ Logback-user mailing list Logback-user@qos.ch http://mailman.qos.ch/mailman/listinfo/logback-user
Ceki, Would you be interested in accepting this change? Is there anything else you feel needs to be done? I'd be interested in seeing it land in the next version. David On 13 Apr 2013, at 01:01, "Becker, Thomas" <Thomas.Becker@netapp.com> wrote:
There was already a Map there. To your point though, that bit of rework to AppenderTrackerImpl was not strictly necessary but I felt the existing code, which essentially re-implemented LinkedHashMap in a clumsy way was pretty low hanging fruit. I'd be willing to make additional changes if the maintainer showed interest in accepting the patch, but my immediate need has passed. My observation has been that the majority of the pull requests to Logback seem to get no comments at all. But I went ahead and reopened this one here: https://github.com/qos-ch/logback/pull/107
-Tommy
________________________________________ From: Logback-user [logback-user-bounces@qos.ch] on behalf of diroussel [nabble@diroussel.xsmail.com] Sent: Friday, April 12, 2013 12:12 PM To: logback-user@qos.ch Subject: Re: [logback-user] SiftingAppender and RollingFileAppender, roll on close.
Thomas,
That functionality looks good.
In order to get accepted, maybe you can make your changes to AppenderTrackerImpl less invasive?
In AppenderTrackerImpl, you are subclassing LinkedHashMap instead of using ch.qos.logback.classic.pattern.LRUCache. But really there is no need for a map. Just keep the existing head and tail references, keep a count variablle. If the count gets to high, then remove entries from the tail.
I like that you've added unit tests and the code formatting is consistent with the rest of the code base.
David
-- View this message in context: http://logback.10977.n7.nabble.com/SiftingAppender-and-RollingFileAppender-r... Sent from the Users mailing list archive at Nabble.com. _______________________________________________ Logback-user mailing list Logback-user@qos.ch http://mailman.qos.ch/mailman/listinfo/logback-user _______________________________________________ Logback-user mailing list Logback-user@qos.ch http://mailman.qos.ch/mailman/listinfo/logback-user
I still have a major bug to fix in cal10n. Next, is a review of Carl's contributions in ch.qos.logback.*.net package. Once that is done, I'll look at new bug reports and pull requests. Thus, unfortunately I have not yet had a chance to look at your changes to AppenderTrackerImpl and co. In my defense, my todo list is ever expanding. Even when I spend the whole day working on logback, at the end of the day, there are more logback-related items on my todo list than the day started with. Not to rant but bug reports and pull requests are generated at a rate faster than I can cope with them. As jira.qos.ch states, popular issues, those with most votes and highest activity, get treated first. On 13.04.2013 10:24, David Roussel wrote:
Ceki,
Would you be interested in accepting this change? Is there anything else you feel needs to be done?
I'd be interested in seeing it land in the next version.
David
On 13 Apr 2013, at 01:01, "Becker, Thomas" <Thomas.Becker@netapp.com> wrote:
There was already a Map there. To your point though, that bit of rework to AppenderTrackerImpl was not strictly necessary but I felt the existing code, which essentially re-implemented LinkedHashMap in a clumsy way was pretty low hanging fruit. I'd be willing to make additional changes if the maintainer showed interest in accepting the patch, but my immediate need has passed. My observation has been that the majority of the pull requests to Logback seem to get no comments at all. But I went ahead and reopened this one here: https://github.com/qos-ch/logback/pull/107
-Tommy
________________________________________ From: Logback-user [logback-user-bounces@qos.ch] on behalf of diroussel [nabble@diroussel.xsmail.com] Sent: Friday, April 12, 2013 12:12 PM To: logback-user@qos.ch Subject: Re: [logback-user] SiftingAppender and RollingFileAppender, roll on close.
Thomas,
That functionality looks good.
In order to get accepted, maybe you can make your changes to AppenderTrackerImpl less invasive?
In AppenderTrackerImpl, you are subclassing LinkedHashMap instead of using ch.qos.logback.classic.pattern.LRUCache. But really there is no need for a map. Just keep the existing head and tail references, keep a count variablle. If the count gets to high, then remove entries from the tail.
I like that you've added unit tests and the code formatting is consistent with the rest of the code base.
David
-- Ceki 65% of statistics are made up on the spot
participants (4)
-
Becker, Thomas -
ceki -
David Roussel -
diroussel