-
Notifications
You must be signed in to change notification settings - Fork 882
Description
This library exposes IAsyncSerializer<T> and yet restricts the contexts in which you can use it. If configured for a producer, that producer will only support task-based producing (i.e. ProduceAsync(...)).
Contrary to appearances, Produce(...) is not a synchronous (i.e. blocking) alternative to this method. Rather, it is more like a "fire and forget" - using an optional callback to offload delivery handling to the background ("polling") thread.
As indicated in the README, callback-based producing is better for high throughput scenarios since it avoids the task overhead. It's also just a natural fit for scenarios where the action you take to handle failures (e.g. logging) does not impact the unit of work involved in the produce.
With this context, the exception when attempting to use callback producing is surprising:
| : throw new InvalidOperationException("Produce called with an IAsyncSerializer key serializer configured but an ISerializer is required."); |
The workaround for this is to use SyncOverAsync(). However, this introduces blocking for both ways of producing and makes configuring a producer unnecessarily confusing (requires knowing which methods I am going to call). Since there's no expectation on callback producing to block or return a result, there's no reason we can't go "full async" here and schedule a continuation (using async void - or something less sugary). It's an easy change that would make this library more async-friendly and prevent issues like #1448.
Let me know if there's interest in a PR.