logging-log4net-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Dominik Psenner <dpsen...@apache.org>
Subject Re: AsyncAppenderSkeleton
Date Mon, 31 Oct 2016 14:30:35 GMT
See inlines.

On 2016-10-31 11:30, Joe wrote:
> Hi Dominik,
> Thanks for the feedback
> > Please note also that MSMQ sounds like a MS only service and that would in turn
mean that the .net core 
> variant would no longer be able to benefit from the 
> AsyncAppenderSkeleton. To me this outlines a path that we would not 
> like to walk on
> I don’t see the problem here.
> I’m proposing that we could implement the queue as a separate class 
> implementing a suitable interface (ILoggingEventQueue, IAppenderQueue 
> or whatever – I’m with Philip Karlton on naming).    The default 
> implementation would be an in-memory queue, would not depend on MSMQ 
> and would be available for .net core etc.

Sorry, my fault, the sentence was TL;DR it's entirety. I had it read it 
as "The default implementation could be MSMQ". ;-) Thanks for the 

> Then there could be an alternate implementation with a persistent 
> queue using MSMQ, or users could provide their own custom 
> implementations using some other persistent queueing technology if 
> they wish.
> The alternative of a persistent queue is useful to reduce the risk of 
> (probably the last and therefore most important) logging events being 
> lost when an application crashes: with a persistent queue they could 
> be picked up again from the queue when the application restarts, or by 
> an independent utility.
> > This sounds mostly like the BufferingAppenderSkeleton, which only 
> misses the background worker thread to send the buffer.
> I’m not convinced that BufferingAppenderSkeleton is a good candidate. 
>  For example:
> - Locking is problematic.  The appender would need to lock(this) while 
> it is forwarding logging events to the sink 
> (BufferingAppenderSkeleton.SendBuffer).  This could hold the lock for 
> an extended period (e.g. due to a communication timeout).  Therefore 
> DoAppend should not lock(this) while enqueueing logging events or 
> we’ll be unnecessarily blocking the calling application.  This is one 
> of the main reasons I want to implement my own DoEvents rather than 
> deriving from AppenderSkeleton.

If the implementation requires lock(this) to work, the implementation is 
broken. The queue itself has to be thread safe. Hence, a true async 
appender should block the calling application only to fix a few logging 
event properties that would otherwise be lost (i.e. stacktrace or thread 

> - I see the buffering and triggering logic being implemented in a 
> pluggable ILoggingEventQueue.   By default, there would be no 
> buffering, except what is implicit in the fact that events may be 
> enqueued faster than they can be dequeued.  I.e. whenever the 
> background thread detects events in the queue, by default it processes 
> all available events immediately, in blocks whose maximum size is a 
> configurable SendBufferSize.    A custom queue implementation could 
> implement triggering logic akin to BufferingAppenderSkeleton, e.g. 
> wait for a timeout defined by TimeEvaluator if there are fewer than 
> SendBufferSize events in the queue.

A async appender working in async mode always buffers, by definition. If 
it wouldn't buffer, there would be nothing that a background thread 
could work on and it would block the calling application.

> > System.Threading.Task.Run().
> The TPL could be one way of implementing the queue, though I’m not 
> convinced that it adds much value.  The custom implementation I did 
> recently didn’t use TPL, and that would be my starting point.  This 
> also means it would be compatible with .NET 3.5.

If .net 3.5 is a target for async logging, then the implementation 
cannot use the System.Threading.Tasks namespace. Otherwise I would build 
upon the default task scheduler implementation or provide a custom task 
scheduler implementation that derives from 
System.Threading.Tasks.TaskScheduler and let all logging tasks run on 
that task scheduler.

>   I found having a single background thread made it easier to 
> implement things like flushing.

Mileage may vary but to me, this is not the case.

>    Flush was implemented to:
> - return true immediately if the queue is empty and all events have 
> been successfully sent to the sink.
> - return false immediately if the last attempt to send events to the 
> sink failed.
> - else wait for the background thread to set a ManualResetEvent when 
> it’s finished processing all events in the queue.

That could read as:

bool Flush() {
return await Task.Run(() => {
   return doFlush();


bool Flush() {
  Task<bool> task = Task.Run() => {
   return doFlush();
  return task.Result;

or even:

Task<bool> FlushAsync() {
return Task.Run() => {
   return doFlush();

> > The default implementation should probably be able to operate 
> asynchronously or synchronously and change mode of operation based on 
> a configuration flag "Asynchronous=True"
> That’s exactly what I had in mind.


View raw message