Ceki, So I gave it a shot. - CloseTBRP.java<https://github.com/trothwell/logback-test/blob/master/src/main/java/org/trothwell/lbtest/CloseTBRP.java> - TestCloseTBRP.java<https://github.com/trothwell/logback-test/blob/master/src/test/java/org/trothwell/lbtest/TestCloseTBRP.java> I'm running into a few problems if you have some time to take a look: - I'm unable to delete files after stopping LoggerContext. - Output file count is off (SiftingAppender or RollingFileAppender doesn't create both output files) - NullPointerException occurs when stopping LoggerContext with CloseTBRP in use I have two scenarios. 1. Create a normal TimeBasedRollingPolicy configured for compression that will create 2 output files. 2. Create the new auto-close CloseTBRP that will do the same. For both configurations this is the steps: (or look at unit test) 1. Create new LoggerContext 2. Configure LoggerContext 3. Submit a log event 4. Set MDC property for discriminator 5. Submit a log event 6. Stop LoggerContext 7. Check log file counts 1. for TBRP, 2 text files 2. for CloseTBRP, 2 zip files Have a great weekend, -- TJ On Fri, Nov 4, 2011 at 9:59 AM, ceki <ceki@qos.ch> wrote:

Hi TJ,

It's a problem which has not been considered previously. Nice problem description btw. One solution would be to subclass TimeBasedRollingPolicy and override the stop() method so that it compresses the current log file. It should not be too hard to do. Ask for help if you run into trouble.

On 04/11/2011 3:32 PM, TJ Rothwell wrote:

...

Hi,

I'm using SiftingAppender on top of an appender that uses compression. When using a variable (like username) that will show up more than once, it works great. But when using a session id which is unique, then the log file will not be triggered for compression leading to larger disk usage. Is there a way to go about triggering the file to be compressed when the SiftingAppender closes the child appender after it's timeout period?

Definition of my appender: <appender name="SIFT-SESSION_ID" class="ch.qos.logback.classic.**sift.SiftingAppender"> <discriminator class="ch.qos.logback.classic.** sift.MDCBasedDiscriminator"> <key>SESSION_ID</key> <defaultValue>unknown</**defaultValue> </discriminator> <sift> <appender name="FILE-${SESSION_ID}" class="ch.qos.logback.core.**rolling.RollingFileAppender"> <encoder> <pattern>%d{yyyy-MM-dd HH:mm:ss.SSS zz} %-5level [%thread] %logger{35} - %msg%n%xEx{full}</pattern> </encoder> <rollingPolicy class="ch.qos.logback.core.**rolling.** TimeBasedRollingPolicy"> <fileNamePattern>${CONTEXT_**DATA_PATH}/logs/session/** session-${SESSION_ID}.%d{yyyy-**MM-dd}.%i.txt.zip</**fileNamePattern> <**timeBasedFileNamingAndTriggeri**ngPolicy class="ch.qos.logback.core.**rolling.SizeAndTimeBasedFNATP"**> <maxFileSize>100MB</**maxFileSize> </**timeBasedFileNamingAndTriggeri**ngPolicy> </rollingPolicy> </appender> </sift> </appender>

Thanks! -- TJ

-- Ceki http://twitter.com/#!/ceki ______________________________**_________________ Logback-user mailing list Logback-user@qos.ch http://mailman.qos.ch/mailman/**listinfo/logback-user<http://mailman.qos.ch/mailman/listinfo/logback-user>

I've updated my github with an implementation that works when the for my circumstance (compression & no file in appender). * CloseTBRP.java<https://github.com/trothwell/logback-test/blob/master/src/main/java/org/trothwell/lbtest/CloseTBRP.java> * TestCloseTBRP.java<https://github.com/trothwell/logback-test/blob/master/src/test/java/org/trothwell/lbtest/TestCloseTBRP.java> I didn't see a way to close the parent appender's outputstream from my RollingPolicy. I might have to subclass RollingFileAppender to make this solution production ready. I also noted a few other concerns I have in the comments--look for // FIXME's. -- TJ On Fri, Nov 4, 2011 at 7:06 AM, ceki <ceki@qos.ch> wrote:

I don't think rollover() should be invoked within the close() method. You rather want to close the file and than compress it.

The compression goes something like:

if (compressionMode == CompressionMode.NONE) { // nothing to do? } else { if (getParentsRawFileProperty() == null) { asyncCompress(**elapsedPeriodsFileName, elapsedPeriodsFileName, elpasedPeriodStem); } else { renamedRawAndAsyncCompress(**elapsedPeriodsFileName, elpasedPeriodStem); } }

See the rollover() method in TimeBasedRollingPolicy. I'll have another look at all this tomorrow.

Cheers,

-- Ceki http://twitter.com/#!/ceki

On 05/11/2011 12:30 AM, TJ Rothwell wrote:

...

Ceki,

So I gave it a shot.

* CloseTBRP.java <https://github.com/trothwell/**logback-test/blob/master/src/** main/java/org/trothwell/**lbtest/CloseTBRP.java<https://github.com/trothwell/logback-test/blob/master/src/main/java/org/trothwell/lbtest/CloseTBRP.java>

...

* TestCloseTBRP.java <https://github.com/trothwell/**logback-test/blob/master/src/** test/java/org/trothwell/**lbtest/TestCloseTBRP.java<https://github.com/trothwell/logback-test/blob/master/src/test/java/org/trothwell/lbtest/TestCloseTBRP.java>

...

I'm running into a few problems if you have some time to take a look:

* I'm unable to delete files after stopping LoggerContext. * Output file count is off (SiftingAppender or RollingFileAppender

doesn't create both output files) * NullPointerException occurs when stopping LoggerContext with

CloseTBRP in use

I have two scenarios.

1. Create a normal TimeBasedRollingPolicy configured for compression

that will create 2 output files. 2. Create the new auto-close CloseTBRP that will do the same.

For both configurations this is the steps: (or look at unit test)

1. Create new LoggerContext 2. Configure LoggerContext 3. Submit a log event 4. Set MDC property for discriminator 5. Submit a log event 6. Stop LoggerContext 7. Check log file counts 1. for TBRP, 2 text files 2. for CloseTBRP, 2 zip files

Have a great weekend, -- TJ

______________________________**_________________ Logback-user mailing list Logback-user@qos.ch http://mailman.qos.ch/mailman/**listinfo/logback-user<http://mailman.qos.ch/mailman/listinfo/logback-user>

This direction seems a bit clunky, as I'm duplicating code. What if RollingFileAppender knew to close/roll? So a new method in RollingPolicy like 'isTriggeringEvent', but 'isTriggeringOnClose'. The base class would return false so existing impl's aren't affected. Now RollingFileAppender is back in the driver's seat to control the roll at close. If RollingFileAppender doesn't have file set and is using a RollingPolicy using the '%i' in the file pattern, the active file on close would never be compressed. This case could be resolved by returning true for isTriggeringOnClose. -- TJ On Sun, Nov 6, 2011 at 1:01 AM, ceki <ceki@qos.ch> wrote:

I have not looked at your code very carefully but it seems that your life would be easier if the stop() method in RollingFileAppender would be changed from

public void stop() { if(rollingPolicy != null) rollingPolicy.stop(); if(triggeringPolicy != null) triggeringPolicy.stop(); super.stop(); }

to

public void stop() { super.stop(); if(rollingPolicy != null) rollingPolicy.stop(); if(triggeringPolicy != null) triggeringPolicy.stop(); }

or even better to

public void stop() { super.stop(); rollingPolicy.stop(); if(rollingPolicy != triggeringPolicy) triggeringPolicy.stop(); }

Let me know if the above change in RollingFileAppender's stop() method helps and if it does, then let me know and I'll do the necessary changes.

On 11/5/2011 11:46 PM, TJ Rothwell wrote:

...

I've updated my github with an implementation that works when the for my circumstance (compression & no file in appender).

* CloseTBRP.java <https://github.com/trothwell/**logback-test/blob/master/src/** main/java/org/trothwell/**lbtest/CloseTBRP.java<https://github.com/trothwell/logback-test/blob/master/src/main/java/org/trothwell/lbtest/CloseTBRP.java>

...

* TestCloseTBRP.java <https://github.com/trothwell/**logback-test/blob/master/src/** test/java/org/trothwell/**lbtest/TestCloseTBRP.java<https://github.com/trothwell/logback-test/blob/master/src/test/java/org/trothwell/lbtest/TestCloseTBRP.java>

...

I didn't see a way to close the parent appender's outputstream from my RollingPolicy. I might have to subclass RollingFileAppender to make this solution production ready.

I also noted a few other concerns I have in the comments--look for // FIXME's.

-- TJ

On Fri, Nov 4, 2011 at 7:06 AM, ceki <ceki@qos.ch <mailto:ceki@qos.ch>>

wrote:

I don't think rollover() should be invoked within the close() method. You rather want to close the file and than compress it.

The compression goes something like:

if (compressionMode == CompressionMode.NONE) { // nothing to do? } else { if (getParentsRawFileProperty() == null) { asyncCompress(__**elapsedPeriodsFileName, elapsedPeriodsFileName, elpasedPeriodStem); } else { renamedRawAndAsyncCompress(__**elapsedPeriodsFileName,

elpasedPeriodStem); } }

See the rollover() method in TimeBasedRollingPolicy. I'll have another look at all this tomorrow.

Cheers,

-- Ceki http://twitter.com/#!/ceki

On 05/11/2011 12:30 AM, TJ Rothwell wrote:

Ceki,

So I gave it a shot.

* CloseTBRP.java <https://github.com/trothwell/**__logback-test/blob/master/** src/__main/java/org/trothwell/**__lbtest/CloseTBRP.java<https://github.com/trothwell/__logback-test/blob/master/src/__main/java/org/trothwell/__lbtest/CloseTBRP.java> <https://github.com/trothwell/**logback-test/blob/master/src/** main/java/org/trothwell/**lbtest/CloseTBRP.java<https://github.com/trothwell/logback-test/blob/master/src/main/java/org/trothwell/lbtest/CloseTBRP.java>

...

...

* TestCloseTBRP.java <https://github.com/trothwell/**__logback-test/blob/master/** src/__test/java/org/trothwell/**__lbtest/TestCloseTBRP.java<https://github.com/trothwell/__logback-test/blob/master/src/__test/java/org/trothwell/__lbtest/TestCloseTBRP.java>

<https://github.com/trothwell/**logback-test/blob/master/src/** test/java/org/trothwell/**lbtest/TestCloseTBRP.java<https://github.com/trothwell/logback-test/blob/master/src/test/java/org/trothwell/lbtest/TestCloseTBRP.java>

...

...

I'm running into a few problems if you have some time to take a look:

* I'm unable to delete files after stopping LoggerContext. * Output file count is off (SiftingAppender or RollingFileAppender

doesn't create both output files) * NullPointerException occurs when stopping LoggerContext with

CloseTBRP in use

I have two scenarios.

1. Create a normal TimeBasedRollingPolicy configured for compression

that will create 2 output files. 2. Create the new auto-close CloseTBRP that will do the same.

For both configurations this is the steps: (or look at unit test)

1. Create new LoggerContext 2. Configure LoggerContext 3. Submit a log event 4. Set MDC property for discriminator 5. Submit a log event 6. Stop LoggerContext 7. Check log file counts 1. for TBRP, 2 text files 2. for CloseTBRP, 2 zip files

Have a great weekend, -- TJ

-- Ceki http://twitter.com/#!/ceki

______________________________**_________________ Logback-user mailing list Logback-user@qos.ch http://mailman.qos.ch/mailman/**listinfo/logback-user<http://mailman.qos.ch/mailman/listinfo/logback-user>

Ceki, The compression is the goal. However a feature like rollover on close would make it easier to implement. The problem is that it adds complexity by modifying an interface that 3rd parties are using. With the base class, it reduces incompatibilities with those impls. Correct me if I'm wrong, but rollover does processing on the activefile inorder to prepare for the next activefile. The rollover process started from the appender doesn't create the next activefile. Its after rollever that the next activefile is created. So having the appender perform rollover on close instead of the rollingpolicy trying to do the work under its nose seems to be a bit easier to understand. Thanks, - TJ Sent from mobile.

Yes, the change to stop and asyncCompress would get things done. ;) Thanks, -- TJ Sent from mobile. On Nov 6, 2011 2:27 PM, "ceki" <ceki@qos.ch> wrote:

There is a rollover method in RollingFileAppender [1] and another one in the RollingPolicy interface. So the term rollover maybe ambiguous.

I am under the impression that if the stop() method was modified to read:

public void stop() { super.stop(); rollingPolicy.stop(); if(rollingPolicy != triggeringPolicy) triggeringPolicy.stop(); }

and if the access modifier for the asyncCompress method in TimeBasedRollingPolicy was changed to "protected", then the stop() method in your custom rolling policy could simply do the compression and the problem would be solved in just a few lines of code.

Am I misreading an aspect of the problem?

More below. -- Ceki

[1] http://goo.gl/ya7XK

On 11/6/2011 7:20 PM, TJ Rothwell wrote:

...

Ceki,

The compression is the goal. However a feature like rollover on close would make it easier to implement. The problem is that it adds complexity by modifying an interface that 3rd parties are using. With the base class, it reduces incompatibilities with those impls.

Correct me if I'm wrong, but rollover does processing on the activefile inorder to prepare for the next activefile. The rollover process started from the appender doesn't create the next activefile. Its after rollever that the next activefile is created.

It depends on what you by rollover. As mentioned above there are two rollover methods. Were you aware of http://goo.gl/ya7XK ?

______________________________**_________________ Logback-user mailing list Logback-user@qos.ch http://mailman.qos.ch/mailman/**listinfo/logback-user<http://mailman.qos.ch/mailman/listinfo/logback-user>

On 10.11.2011 17:14, TJ Rothwell wrote:

Ceki,

There was one additional problem I ran into so I'm unable to use the stock SizeAndTimeBasedFNATP.

computeCurrentPeriodsHighestCounterValue(String) assumes that it will write to the last index'd value even though it was already compressed.

If I understand correctly, computeCurrentPeriodsHighestCounterValue assumes that the last index points to an uncompressed file, even if it has the .zip extension. This sounds very much like a big. Could you please file a big report for this?

I extended the class to modify this behavior (unfortunately it my class must have your package due to the package-priv visibility of this method).

Why don't you fork the project [1] on github? This makes it much easier to submit patches. [1] https://github.com/ceki/logback

-- TJ

Done. http://jira.qos.ch/browse/LBCORE-236 On Thu, Nov 10, 2011 at 11:24 AM, ceki <ceki@qos.ch> wrote:

On 10.11.2011 17:14, TJ Rothwell wrote:

...

Ceki,

There was one additional problem I ran into so I'm unable to use the stock SizeAndTimeBasedFNATP.

computeCurrentPeriodsHighestCo**unterValue(String) assumes that it will write to the last index'd value even though it was already compressed.

I just modified computeCurrentPeriodsHighestCo**unterValue so that it takes into account compression of the latest file

Please test it. A new bug report regarding this issue is still warranted. Please create it. -- Ceki

______________________________**_________________ Logback-user mailing list Logback-user@qos.ch http://mailman.qos.ch/mailman/**listinfo/logback-user<http://mailman.qos.ch/mailman/listinfo/logback-user>

Ceki, I haven't tested against HEAD yet. But I was tweaking with <maxHistory>, and it doesn't appear to obey when the filename pattern changes on the folder. This might be an unsupported configuration, but thought it might work since I saw it as an example on the website. "/wombat/%d{yyyy/MM}/foo.txt" [1] This is the config I'm testing with. <appender name="FILE-${KEY}" class="ch.qos.logback.core.rolling.RollingFileAppender"> <encoder class="ch.qos.logback.classic.encoder.PatternLayoutEncoder"> <pattern>%d{HH:mm:ss.SSS zz} %-5level [%thread] %logger{35} [%-3X{KEY}] %msg%n</pattern> </encoder> <rollingPolicy class="com.touchnet.twp.contextconfig.lbclassic.CloseTBRP"> <fileNamePattern>${TEMP_FOLDER}/%d{HHmmss'/maxhist-key/key.'HHmmss }.${KEY}.%i.txt.zip</fileNamePattern> <maxHistory>1</maxHistory> <!-- date periods to keep in addition to current --> <timeBasedFileNamingAndTriggeringPolicy class="ch.qos.logback.core.rolling.CloseSTBFNATP"> <maxFileSize>1</maxFileSize> <!-- testing if index values were counted in history --> </timeBasedFileNamingAndTriggeringPolicy> </rollingPolicy> </appender> [1] http://logback.qos.ch/manual/appenders.html Thanks, -- TJ On Fri, Nov 11, 2011 at 2:15 AM, ceki <ceki@qos.ch> wrote:

So does compression with SiftingAppender work now? -- Ceki http://twitter.com/#!/ceki

On 10.11.2011 18:35, TJ Rothwell wrote:

...

Done.

http://jira.qos.ch/browse/**LBCORE-236<http://jira.qos.ch/browse/LBCORE-236>

On Thu, Nov 10, 2011 at 11:24 AM, ceki <ceki@qos.ch <mailto:ceki@qos.ch>> wrote:

On 10.11.2011 17:14, TJ Rothwell wrote:

Ceki,

There was one additional problem I ran into so I'm unable to use the stock SizeAndTimeBasedFNATP.

computeCurrentPeriodsHighestCo**__unterValue(String) assumes that

it will write to the last index'd value even though it was already compressed.

I just modified computeCurrentPeriodsHighestCo**__unterValue so that

it takes into account compression of the latest file

Please test it. A new bug report regarding this issue is still warranted. Please create it. -- Ceki

______________________________**_________________ Logback-user mailing list Logback-user@qos.ch http://mailman.qos.ch/mailman/**listinfo/logback-user<http://mailman.qos.ch/mailman/listinfo/logback-user>