search

cjbhaines / Log4Net.Async

Asynchronous Log4Net appenders and forwarder
http://www.nuget.org/packages/Log4Net.Async/
MIT License
121 stars 37 forks source link

Amended parallel appender to immediately proceed with shutdown once all messages have flushed #28

Closed stas-a closed 8 years ago

stas-a commented 8 years ago

Amended ParallelForwardingAppender to immediately proceed with shutdown once all messages have flushed. It will sleep and check every 100ms until flush timeout is expired or all events have flushed. Otherwise it will behave same as before and wait for th specified time. Moved timeout assignemnt out of ActivateOptions so that it can be set from unit tests. Added appropriate unit tests.

rcollette commented 8 years ago

I'm assuming this is to obtain quicker shutdown. Seems to make sense but I am curious about the use case that motivated looking into this.

stas-a commented 8 years ago

The use case is simple: I was testing the parallel appender using a console app and noticed that messages could be lost if an appender was relatively slow and the app exited quickly. Added LogManager.Shutdown() to ensure messages are flushed. Then the test app was getting stuck for extra 5 seconds during every run, regardless of number of messages to flush.

So, to recap, console app or any other app that runs and terminates quickly may incur message loss unless it calls LogManager.Shutdown(). And if it does, it will be getting stuck for extra X seconds waiting for timeout.

I guess in most cases one can do without calling LogManager.Shutdown() and never encounter message loss. But if there is a slow appender, all bets are off.

02 авг. 2016 г. 3:01 пользователь "Richard Collette" < notifications@github.com> написал:

I'm assuming this is to obtain quicker shutdown. Seems to make sense but I am curious about the use case that motivated looking into this.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/cjbhaines/Log4Net.Async/pull/28#issuecomment-236747839, or mute the thread https://github.com/notifications/unsubscribe-auth/ALSxlzX_BJcHF4nSXpTj0-ZxF4b5WTxTks5qbojdgaJpZM4JZEd2 .

rcollette commented 8 years ago

@cjbhaines - This looks good. Historically it looks like you merge and then bump the nuspec version number. Same process here or should the nuspec version be bumped as part of the merge?

stas-a commented 8 years ago

Awesome! Thanks!

cjbhaines commented 8 years ago

Thanks guys! packages pushed to NuGet.