Data dropped without error report in Async mode

[bboreham]

I see a lot of this in my logs:

[2020-03-26T13:23:11Z] Unable to send logs to fluentd, reconnecting...
[2020-03-26T13:23:11Z] Unable to send logs to fluentd, reconnecting...
[2020-03-26T13:23:11Z] Unable to send logs to fluentd, reconnecting...
[2020-03-26T13:23:11Z] Unable to send logs to fluentd, reconnecting...
[2020-03-26T13:23:11Z] Unable to send logs to fluentd, reconnecting...
[2020-03-26T13:23:11Z] Unable to send logs to fluentd, reconnecting...

no records are sent, no errors are returned.

I think it would be better to leave the data in the pending queue until reconnection is achieved.

[tagomoris]

Currently, we have two modes to:

• Sync(default): send a message to Fluentd immediately, and returns an error if it failed
• Async: push a message into a queue, then returns (it doesn't take care of success/failure)

And those two modes are used on different expectations on whether it blocks main code processing (or not), and on whether it consumes memory (or not).
The request to contain messages which met errors in Async mode should be the different expectation from those two.

-	it blocks main	it may consume memory	data will remain during failures
Sync(default)	yes	no	yes (with appropriate error handling)
Something new	no	yes	yes (if the destination back to operational eventually)
Async	no	no	no

This new mode has a different expectation, so I think we should NOT update Async mode, but should add a different mode to work as described above.
Any other opinions?

[bboreham]

I think you should add a column whether the caller is notified of data drops - this was what I found most surprising.

My proposal does not consume any more memory than the queue limit already allows, e.g. if consumer is working but much slower than producer.

[tagomoris]

IIUC, we can't unshift objects into golang channels.
So, if we use the channel for retried messages too, we need to push the unsent messages into the channel, in the same manner with the producer. Consumer will try sending messages in the HEAD of the channel, so messages will be sent to the destination in random order if there are failures.

@bboreham Is this what you proposed accurately?

[tagomoris]

And, even if we have retries for message transferring failures in Async mode, we can't return errors to the caller because pushing messages into the channel succeeded. Message transferring will be processed asynchronously, so caller can't know the result of message transferring.
If the caller needs to know the result of message transferring, Sync mode (avoiding Async) is the only way to get it.

[bboreham]

You can have one popped item which you retry sending and a queue of items in the chan to follow.

[ivan-valkov]

I have a similar use case. Here is a PR that attempts to give the users of this library the flexibility to resend messages/handle errors when using async - #97

It does not break the expectations you mentioned above @tagomoris if you don't want to use the callback.

[tagomoris]

@ivan-valkov That looks like a smart solution! I'll review the change a little later.