serilog / serilog-sinks-periodicbatching

Infrastructure for Serilog sinks that process events in batches.
Apache License 2.0
73 stars 29 forks source link

Port to `System.Threading.Channels` #66

Closed nblumhardt closed 10 months ago

nblumhardt commented 10 months ago

This is effectively a complete rewrite; it's intended to be more efficient, robust, and modern, but it's going to need some bake time.

The sink now uses System.Threading.Channels to implement an async queue, so there's no longer a blocked worker thread waiting to process batches.

The sink also now begins processing batches as soon as they're full, so if the batch size is 100, and 100 events are written, they'll be processed regardless of whether the buffering delay (PeriodicBatchingSinkOptions.Period) has elapsed.

The sink now properly avoids overlapping batches (the old Monitor-based logic was/is broken, as far as I can tell). Shut-down/flush logic is also cleaner: the queue will always be fully-flushed on shutdown, except when an error occurs.

Finally:

I've dropped out the old performance tests, which only effectively covered the custom blocking collection type that the sink used. I'll explore performance using downstream sinks and try to formulate a better perf plan.

Breaking changes:

nblumhardt commented 10 months ago

Thanks @bartelink! Currently just roughing it out - will loop back and make sure I've sorted those things out once I've decided on how shutdown should work 👍

nblumhardt commented 10 months ago

Tests are failing on .NET Framework (yay 🙄), will sort that out shortly, and then with some luck this will be ready for a look! :-)

nblumhardt commented 10 months ago

I think this is ready for a closer look now :-)

nblumhardt commented 10 months ago

@skomis-mm hope all is well with you! Just flagging this as one that would benefit from your eyes, in case you're able to take a look at any point.

Planning to merge fairly soon, to keep pushing ahead, but expecting to run through a few iterations before calling it "stable".

nblumhardt commented 10 months ago

Thanks for the awesome review @bartelink 👍 should all be done, now.

nblumhardt commented 10 months ago

Published to NuGet now in 4.0.0-dev-*