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.

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.

- 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.

> 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.  I found having a single background thread made it 
easier to implement things like flushing.   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.

> 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.

Joe

Reply via email to