You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@logging.apache.org by ma...@apache.org on 2014/04/15 07:26:59 UTC

svn commit: r1587425 - /logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/async/AsyncLogger.java

Author: mattsicker
Date: Tue Apr 15 05:26:59 2014
New Revision: 1587425

URL: http://svn.apache.org/r1587425
Log:
Use Loader.newCheckedInstanceOf.

  - Also noted busy wait usage (static code analysis complains; could use some justification?)

Modified:
    logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/async/AsyncLogger.java

Modified: logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/async/AsyncLogger.java
URL: http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/async/AsyncLogger.java?rev=1587425&r1=1587424&r2=1587425&view=diff
==============================================================================
--- logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/async/AsyncLogger.java (original)
+++ logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/async/AsyncLogger.java Tue Apr 15 05:26:59 2014
@@ -28,6 +28,7 @@ import org.apache.logging.log4j.core.Log
 import org.apache.logging.log4j.core.config.Property;
 import org.apache.logging.log4j.core.helpers.Clock;
 import org.apache.logging.log4j.core.helpers.ClockFactory;
+import org.apache.logging.log4j.core.helpers.Loader;
 import org.apache.logging.log4j.core.impl.Log4jLogEvent;
 import org.apache.logging.log4j.core.jmx.RingBufferAdmin;
 import org.apache.logging.log4j.message.Message;
@@ -189,10 +190,8 @@ public class AsyncLogger extends Logger 
             return null;
         }
         try {
-            @SuppressWarnings("unchecked")
-            final Class<? extends ExceptionHandler> klass = (Class<? extends ExceptionHandler>) Class.forName(cls);
-            final ExceptionHandler result = klass.newInstance();
-            LOGGER.debug("AsyncLogger.ExceptionHandler=" + result);
+            final ExceptionHandler result = Loader.newCheckedInstanceOf(cls, ExceptionHandler.class);
+            LOGGER.debug("AsyncLogger.ExceptionHandler={}", result);
             return result;
         } catch (final Exception ignored) {
             LOGGER.debug("AsyncLogger.ExceptionHandler not set: error creating " + cls + ": ", ignored);
@@ -305,6 +304,7 @@ public class AsyncLogger extends Logger 
             }
             try {
                 // give ringbuffer some time to drain...
+                // TODO: is there a better way to do this than busy-waiting?
                 Thread.sleep(HALF_A_SECOND);
             } catch (final InterruptedException e) {
                 // ignored



Re: svn commit: r1587425 - /logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/async/AsyncLogger.java

Posted by Remko Popma <re...@gmail.com>.
We could, but to be honest I think it's an extremely rare scenario that the buffer has so many events that it takes 10 secs to write them all to disk. 

Remko

Sent from my iPhone

> On 2014/04/17, at 21:11, Gary Gregory <ga...@gmail.com> wrote:
> 
> >Thread.sleep(HALF_A_SECOND);
> 
> Shouldn't this be configurable?
> 
> Gary
> 
> 
>> On Tue, Apr 15, 2014 at 7:43 PM, Remko Popma <re...@gmail.com> wrote:
>> The justification is in the comment: we need to give the async logging thread time to drain the buffer. Scenario where this is needed: app just had a burst of events & has put many logging events in the queue. Then the app was stopped. It is log4j's responsibility to ensure that the enqueued events appear on disk. Hence the wait.
>> 
>> Note that it repeatedly waits small amounts of time & quits as soon as the buffer is empty, and also has a max wait time so it won't stall indefinitely.
>> 
>> Sent from my iPhone
>> 
>> > On 2014/04/15, at 14:26, mattsicker@apache.org wrote:
>> >
>> > Author: mattsicker
>> > Date: Tue Apr 15 05:26:59 2014
>> > New Revision: 1587425
>> >
>> > URL: http://svn.apache.org/r1587425
>> > Log:
>> > Use Loader.newCheckedInstanceOf.
>> >
>> >  - Also noted busy wait usage (static code analysis complains; could use some justification?)
>> >
>> > Modified:
>> >    logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/async/AsyncLogger.java
>> >
>> > Modified: logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/async/AsyncLogger.java
>> > URL: http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/async/AsyncLogger.java?rev=1587425&r1=1587424&r2=1587425&view=diff
>> > ==============================================================================
>> > --- logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/async/AsyncLogger.java (original)
>> > +++ logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/async/AsyncLogger.java Tue Apr 15 05:26:59 2014
>> > @@ -28,6 +28,7 @@ import org.apache.logging.log4j.core.Log
>> > import org.apache.logging.log4j.core.config.Property;
>> > import org.apache.logging.log4j.core.helpers.Clock;
>> > import org.apache.logging.log4j.core.helpers.ClockFactory;
>> > +import org.apache.logging.log4j.core.helpers.Loader;
>> > import org.apache.logging.log4j.core.impl.Log4jLogEvent;
>> > import org.apache.logging.log4j.core.jmx.RingBufferAdmin;
>> > import org.apache.logging.log4j.message.Message;
>> > @@ -189,10 +190,8 @@ public class AsyncLogger extends Logger
>> >             return null;
>> >         }
>> >         try {
>> > -            @SuppressWarnings("unchecked")
>> > -            final Class<? extends ExceptionHandler> klass = (Class<? extends ExceptionHandler>) Class.forName(cls);
>> > -            final ExceptionHandler result = klass.newInstance();
>> > -            LOGGER.debug("AsyncLogger.ExceptionHandler=" + result);
>> > +            final ExceptionHandler result = Loader.newCheckedInstanceOf(cls, ExceptionHandler.class);
>> > +            LOGGER.debug("AsyncLogger.ExceptionHandler={}", result);
>> >             return result;
>> >         } catch (final Exception ignored) {
>> >             LOGGER.debug("AsyncLogger.ExceptionHandler not set: error creating " + cls + ": ", ignored);
>> > @@ -305,6 +304,7 @@ public class AsyncLogger extends Logger
>> >             }
>> >             try {
>> >                 // give ringbuffer some time to drain...
>> > +                // TODO: is there a better way to do this than busy-waiting?
>> >                 Thread.sleep(HALF_A_SECOND);
>> >             } catch (final InterruptedException e) {
>> >                 // ignored
>> >
>> >
>> 
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
>> For additional commands, e-mail: log4j-dev-help@logging.apache.org
> 
> 
> 
> -- 
> E-Mail: garydgregory@gmail.com | ggregory@apache.org 
> Java Persistence with Hibernate, Second Edition
> JUnit in Action, Second Edition
> Spring Batch in Action
> Blog: http://garygregory.wordpress.com 
> Home: http://garygregory.com/
> Tweet! http://twitter.com/GaryGregory

Re: svn commit: r1587425 - /logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/async/AsyncLogger.java

Posted by Gary Gregory <ga...@gmail.com>.
>Thread.sleep(HALF_A_SECOND);

Shouldn't this be configurable?

Gary


On Tue, Apr 15, 2014 at 7:43 PM, Remko Popma <re...@gmail.com> wrote:

> The justification is in the comment: we need to give the async logging
> thread time to drain the buffer. Scenario where this is needed: app just
> had a burst of events & has put many logging events in the queue. Then the
> app was stopped. It is log4j's responsibility to ensure that the enqueued
> events appear on disk. Hence the wait.
>
> Note that it repeatedly waits small amounts of time & quits as soon as the
> buffer is empty, and also has a max wait time so it won't stall
> indefinitely.
>
> Sent from my iPhone
>
> > On 2014/04/15, at 14:26, mattsicker@apache.org wrote:
> >
> > Author: mattsicker
> > Date: Tue Apr 15 05:26:59 2014
> > New Revision: 1587425
> >
> > URL: http://svn.apache.org/r1587425
> > Log:
> > Use Loader.newCheckedInstanceOf.
> >
> >  - Also noted busy wait usage (static code analysis complains; could use
> some justification?)
> >
> > Modified:
> >
>  logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/async/AsyncLogger.java
> >
> > Modified:
> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/async/AsyncLogger.java
> > URL:
> http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/async/AsyncLogger.java?rev=1587425&r1=1587424&r2=1587425&view=diff
> >
> ==============================================================================
> > ---
> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/async/AsyncLogger.java
> (original)
> > +++
> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/async/AsyncLogger.java
> Tue Apr 15 05:26:59 2014
> > @@ -28,6 +28,7 @@ import org.apache.logging.log4j.core.Log
> > import org.apache.logging.log4j.core.config.Property;
> > import org.apache.logging.log4j.core.helpers.Clock;
> > import org.apache.logging.log4j.core.helpers.ClockFactory;
> > +import org.apache.logging.log4j.core.helpers.Loader;
> > import org.apache.logging.log4j.core.impl.Log4jLogEvent;
> > import org.apache.logging.log4j.core.jmx.RingBufferAdmin;
> > import org.apache.logging.log4j.message.Message;
> > @@ -189,10 +190,8 @@ public class AsyncLogger extends Logger
> >             return null;
> >         }
> >         try {
> > -            @SuppressWarnings("unchecked")
> > -            final Class<? extends ExceptionHandler> klass = (Class<?
> extends ExceptionHandler>) Class.forName(cls);
> > -            final ExceptionHandler result = klass.newInstance();
> > -            LOGGER.debug("AsyncLogger.ExceptionHandler=" + result);
> > +            final ExceptionHandler result =
> Loader.newCheckedInstanceOf(cls, ExceptionHandler.class);
> > +            LOGGER.debug("AsyncLogger.ExceptionHandler={}", result);
> >             return result;
> >         } catch (final Exception ignored) {
> >             LOGGER.debug("AsyncLogger.ExceptionHandler not set: error
> creating " + cls + ": ", ignored);
> > @@ -305,6 +304,7 @@ public class AsyncLogger extends Logger
> >             }
> >             try {
> >                 // give ringbuffer some time to drain...
> > +                // TODO: is there a better way to do this than
> busy-waiting?
> >                 Thread.sleep(HALF_A_SECOND);
> >             } catch (final InterruptedException e) {
> >                 // ignored
> >
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
> For additional commands, e-mail: log4j-dev-help@logging.apache.org
>
>


-- 
E-Mail: garydgregory@gmail.com | ggregory@apache.org
Java Persistence with Hibernate, Second Edition<http://www.manning.com/bauer3/>
JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
Spring Batch in Action <http://www.manning.com/templier/>
Blog: http://garygregory.wordpress.com
Home: http://garygregory.com/
Tweet! http://twitter.com/GaryGregory

Re: svn commit: r1587425 - /logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/async/AsyncLogger.java

Posted by Matt Sicker <bo...@gmail.com>.
Sounds good. Thanks for the clarification!


On 15 April 2014 18:43, Remko Popma <re...@gmail.com> wrote:

> The justification is in the comment: we need to give the async logging
> thread time to drain the buffer. Scenario where this is needed: app just
> had a burst of events & has put many logging events in the queue. Then the
> app was stopped. It is log4j's responsibility to ensure that the enqueued
> events appear on disk. Hence the wait.
>
> Note that it repeatedly waits small amounts of time & quits as soon as the
> buffer is empty, and also has a max wait time so it won't stall
> indefinitely.
>
> Sent from my iPhone
>
> > On 2014/04/15, at 14:26, mattsicker@apache.org wrote:
> >
> > Author: mattsicker
> > Date: Tue Apr 15 05:26:59 2014
> > New Revision: 1587425
> >
> > URL: http://svn.apache.org/r1587425
> > Log:
> > Use Loader.newCheckedInstanceOf.
> >
> >  - Also noted busy wait usage (static code analysis complains; could use
> some justification?)
> >
> > Modified:
> >
>  logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/async/AsyncLogger.java
> >
> > Modified:
> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/async/AsyncLogger.java
> > URL:
> http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/async/AsyncLogger.java?rev=1587425&r1=1587424&r2=1587425&view=diff
> >
> ==============================================================================
> > ---
> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/async/AsyncLogger.java
> (original)
> > +++
> logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/async/AsyncLogger.java
> Tue Apr 15 05:26:59 2014
> > @@ -28,6 +28,7 @@ import org.apache.logging.log4j.core.Log
> > import org.apache.logging.log4j.core.config.Property;
> > import org.apache.logging.log4j.core.helpers.Clock;
> > import org.apache.logging.log4j.core.helpers.ClockFactory;
> > +import org.apache.logging.log4j.core.helpers.Loader;
> > import org.apache.logging.log4j.core.impl.Log4jLogEvent;
> > import org.apache.logging.log4j.core.jmx.RingBufferAdmin;
> > import org.apache.logging.log4j.message.Message;
> > @@ -189,10 +190,8 @@ public class AsyncLogger extends Logger
> >             return null;
> >         }
> >         try {
> > -            @SuppressWarnings("unchecked")
> > -            final Class<? extends ExceptionHandler> klass = (Class<?
> extends ExceptionHandler>) Class.forName(cls);
> > -            final ExceptionHandler result = klass.newInstance();
> > -            LOGGER.debug("AsyncLogger.ExceptionHandler=" + result);
> > +            final ExceptionHandler result =
> Loader.newCheckedInstanceOf(cls, ExceptionHandler.class);
> > +            LOGGER.debug("AsyncLogger.ExceptionHandler={}", result);
> >             return result;
> >         } catch (final Exception ignored) {
> >             LOGGER.debug("AsyncLogger.ExceptionHandler not set: error
> creating " + cls + ": ", ignored);
> > @@ -305,6 +304,7 @@ public class AsyncLogger extends Logger
> >             }
> >             try {
> >                 // give ringbuffer some time to drain...
> > +                // TODO: is there a better way to do this than
> busy-waiting?
> >                 Thread.sleep(HALF_A_SECOND);
> >             } catch (final InterruptedException e) {
> >                 // ignored
> >
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
> For additional commands, e-mail: log4j-dev-help@logging.apache.org
>
>


-- 
Matt Sicker <bo...@gmail.com>

Re: svn commit: r1587425 - /logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/async/AsyncLogger.java

Posted by Remko Popma <re...@gmail.com>.
The justification is in the comment: we need to give the async logging thread time to drain the buffer. Scenario where this is needed: app just had a burst of events & has put many logging events in the queue. Then the app was stopped. It is log4j's responsibility to ensure that the enqueued events appear on disk. Hence the wait. 

Note that it repeatedly waits small amounts of time & quits as soon as the buffer is empty, and also has a max wait time so it won't stall indefinitely. 

Sent from my iPhone

> On 2014/04/15, at 14:26, mattsicker@apache.org wrote:
> 
> Author: mattsicker
> Date: Tue Apr 15 05:26:59 2014
> New Revision: 1587425
> 
> URL: http://svn.apache.org/r1587425
> Log:
> Use Loader.newCheckedInstanceOf.
> 
>  - Also noted busy wait usage (static code analysis complains; could use some justification?)
> 
> Modified:
>    logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/async/AsyncLogger.java
> 
> Modified: logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/async/AsyncLogger.java
> URL: http://svn.apache.org/viewvc/logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/async/AsyncLogger.java?rev=1587425&r1=1587424&r2=1587425&view=diff
> ==============================================================================
> --- logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/async/AsyncLogger.java (original)
> +++ logging/log4j/log4j2/trunk/log4j-core/src/main/java/org/apache/logging/log4j/core/async/AsyncLogger.java Tue Apr 15 05:26:59 2014
> @@ -28,6 +28,7 @@ import org.apache.logging.log4j.core.Log
> import org.apache.logging.log4j.core.config.Property;
> import org.apache.logging.log4j.core.helpers.Clock;
> import org.apache.logging.log4j.core.helpers.ClockFactory;
> +import org.apache.logging.log4j.core.helpers.Loader;
> import org.apache.logging.log4j.core.impl.Log4jLogEvent;
> import org.apache.logging.log4j.core.jmx.RingBufferAdmin;
> import org.apache.logging.log4j.message.Message;
> @@ -189,10 +190,8 @@ public class AsyncLogger extends Logger 
>             return null;
>         }
>         try {
> -            @SuppressWarnings("unchecked")
> -            final Class<? extends ExceptionHandler> klass = (Class<? extends ExceptionHandler>) Class.forName(cls);
> -            final ExceptionHandler result = klass.newInstance();
> -            LOGGER.debug("AsyncLogger.ExceptionHandler=" + result);
> +            final ExceptionHandler result = Loader.newCheckedInstanceOf(cls, ExceptionHandler.class);
> +            LOGGER.debug("AsyncLogger.ExceptionHandler={}", result);
>             return result;
>         } catch (final Exception ignored) {
>             LOGGER.debug("AsyncLogger.ExceptionHandler not set: error creating " + cls + ": ", ignored);
> @@ -305,6 +304,7 @@ public class AsyncLogger extends Logger 
>             }
>             try {
>                 // give ringbuffer some time to drain...
> +                // TODO: is there a better way to do this than busy-waiting?
>                 Thread.sleep(HALF_A_SECOND);
>             } catch (final InterruptedException e) {
>                 // ignored
> 
> 

---------------------------------------------------------------------
To unsubscribe, e-mail: log4j-dev-unsubscribe@logging.apache.org
For additional commands, e-mail: log4j-dev-help@logging.apache.org