PeriodicBatchingSink Duplicates Messages on Failure

[alanedwardes]

The current implementation uses Serilog's PeriodicBatchingSink. It is set up to receive a message batch, loop the collection, sending each message as a separate HTTP request (a Slack limitation, the API only allows one message).

This isn't a good use case for PeriodicBatchingSink, because if we have a batch of 10 messages, and HTTP request 9 out of 10 is throttled by Slack and fails, the batching sink will retry the entire batch. This will duplicate messages 1 - 8 until all requests for the batch succeed.

What the sink really needs to do is post one at a time, in a way Serilog can retry. Needs some investigation - maybe we can just set the batch size to 1?

[mgibas]

We are using PeriodicBatchSink only to work with rate limits (https://api.slack.com/docs/rate-limits) - maybe its time to queue messages on our own (we just need to stick to 1msg/s rate limit) ?

[alanedwardes]

Possibly, but the beauty of using PeriodicBatchingSink is that it handles all of the async/await + queueing for you. I am leaning towards just using a batch size of 1 so retry can't possibly duplicate messages, but it needs some testing :)

[mgibas]

yeah, you are 100% right - what the heck I was thinking 🤔😉

[alanedwardes]

Ha ha! Better to re-use :)

So looking at the code again ( https://github.com/serilog/serilog-sinks-periodicbatching/blob/dev/src/Serilog.Sinks.PeriodicBatching/Sinks/PeriodicBatching/PeriodicBatchingSink.cs#L53-L54 ) I think this will work... setting the batch size to 1 and the period to 1 second. No duplication on request errors and doesn't exceed Slack's limits.

Need to test it though, if a lot is being logged to Slack it's going to back up. My use case for this Sink is to log anything above a certain level. I doubt anyone will use it to log at the Verbose level to Slack, so volume-wise it might work out OK.

It's a bit of an abuse of the PeriodicBatchSink because there is no batch, but I am not sure if the regular Serilog Sink class does any retrying at all. Will check the docs when I get a moment!

[mgibas]

So I was wondering - maybe its a https://github.com/serilog/serilog-sinks-periodicbatching issue (the way it tries to retry) ?

[alanedwardes]

It's working as intended, if there is an exception thrown from the EmitBatch method Serilog will retry. It's useful when you are using it to submit a batch in one shot, which it is designed for, but won't work if you are making multiple requests in a tight loop: https://github.com/serilog/serilog/wiki/Reliability#asynchronousbatched-network-operations

[mgibas]

Yeah, but they are retrying whole batch even though only one message failed. I'm good with previous solution though :)

[alanedwardes]

It's designed to make only one HTTP request per batch though, not many 😄 So this is a little outside of the use case for the periodic batching sink, but I think we can make it work.

[mgibas]

Still exploring possibilities here: https://github.com/serilog/serilog-sinks-periodicbatching/issues/15