Added queue full notification

[accelerated]

Added queue full notification with various triggering options. This is needed so that the application can be notified when the rdkafka queue is backed up and can be allowed to reduce upstream pressure on the producers.

[mfontanini]

This is always resetting queue_full_notify, so if you actually use QueueFullNotification::EachOccurence you'll still get only once per produce_message call, right? Not sure if the issue is here or if you wanted "each occurrence" to still mean "every time but only once per message"/

[accelerated]

Yes that's the idea...EachOccurence is once per message, otherwise you can get spammed by too many callbacks.

[mfontanini]

Right but what if you keep hitting the limit forever for a single message? There's currently no way of actually knowing you get stuck. "Each occurrence" makes me think it's actually every occurrence of a full queue rather than "only one occurrence per message".

[accelerated]

Yes, there's no way of knowing if you're stuck or even which message got stuck. It's better than being silently loopinh though. Another alternative would be to also add one enum for "every single time" which would essentially be for each error multiple times per message, if it so happens. And instead of using the error callback we can have a separate callback just for this, with an argument taking a MessageBuilder reference, so the application can also see what got stuck (topic,partition). This would be better than the generic error callback. Otherwise, if you have other suggestions i'd be glad to implement them.

So then:
EdgeTriggered -> fire once with manual reset
OncePerMessage -> fire once per message.
EachOccurence -> every time it happens.

[accelerated]

For the importance of having this notification, you can read the response in the rdkafka issue

[accelerated]

Hi @mfontanini, i am ready to resume this. Can you please lmk what changes you want? Thanks!

[mfontanini]

Hi! So, looking at these changes and your comments, I think:

• It's probably better to introduce a new callback as you mention. Even though this is an error from rdkafka's perspective, it's not necessarily fatal and can be cleared by just polling so I think it makes sense to move it to another one that takes the message builder for more insights into what happened.
• I think adding another enum value for "once per message" makes sense.
• Looking back, I'm not so sure adding "EdgeTriggered" is a good idea. This seems like something that can (and should) be done by users by using either "each occurrence" or "once per message" and adding that logic on top of it. This makes this class more complicated for something that doesn't really belong here.

Cheers!

[accelerated]

Ok that makes sense. Will make changes as you requested. Stay tuned.

[mfontanini]

Maybe the setter for the notifiaction enum and the callback should be done in just one? You'll always call either both or none of them so I think that would make sense.

[accelerated]

You mean the setter of the notification type? I think there might be cases when the app wants to set the notifocation to some enum and then it might want to turn it off (None) for a period and re-enable it later. In that case it would have to set the callback each time...prob not desirable.

[mfontanini]

Alright, fair enough. Merging this one, thanks!