rabbitmq-dotnet-client
rabbitmq-dotnet-client copied to clipboard
rabbitmq
AMQP 0-9-1 Channel (IModel) API with async methods
I asked for some actionable feedback: https://news.ycombinator.com/item?id=23263536
If the library were being designed from scratch today, pretty much every method on the model would be Async. After all, if it leads to any network I/O of any kind, that can block.
Definitely on the 7.0 roadmap.
Working with the current public API, Trying to implement a publish wrapper that never blocks, and returns a task that either completes when the publisher confirm is received, or faults after some provided timeout, is a lot trickier than it might sound.
Recovery from network interruptions is complicated, and auto-recovery features are limited, and in some use cases actually dangerous. For example, if you are manually acknowledging messages to ensure end-to-end at-least-once delivery, then you cannot safely use the auto-recovery, since the delivery numbers would reset when the connection does, and you can accidentally aknowlodge the wrong message with delivery tag 5. (Acknowledge the new one, when you were trying to ack the old one).
In my implementation of that included my own recovery, I ended up needing to pass around the IModel itself with the delivery tags, so I can check if the channel I am about to acknowledge on is really the same one I received the message on. (There is no unique identifier of a channel instance, since even the channel number is likely to get re-used).
Interestingly, I have a pretty big changelist getting close to being ready (still have a few quirks to work out) that does exactly this, proper async for the entire API. It is a very BIG change in all public interfaces though, as almost every method becomes async and I don't think it serves any purpose at that point to create non-async versions as it'll require A LOT of extra code to do properly (since there is no easy way to make an async method synchronous).
Unfortunately there is no easy way to turn the current client into an asynchronous one without making some sacrifices.
I'll see if I can post that PR soon, and I'm hoping I can convince @danielmarbach to go over the details with me. It's as close to a rewrite you can get without actually rewriting the entire thing i.m.o.
Currently I though it made sense for it to target netstandard2.1 (.NET Core 3.0 upwards, will be .NET 5.0 compatible, but drops .NET Framework support entirely). It should also spark some interesting design discussions and what features makes sense to keep (event-based notifications and handlers vs. async interface-based ones which I'm leaning more towards).
I wouldn't expect that PR to be the big 7.0 PR, but hopefully it'll make a good base to start on.
Working with the current public API, Trying to implement a publish wrapper that never blocks, and returns a task that either completes when the publisher confirm is received, or faults after some provided timeout, is a lot trickier than it might sound.
This is something that I feel the library itself could do automatically. For example, when putting the channel in ConfirmSelect mode, calling BasicPublish wouldn't return until the publisher confirm message was received, or a given timeout would be hit, or at least provide an overloaded method that does the same?
Recovery from network interruptions is complicated, and auto-recovery features are limited, and in some use cases actually dangerous. For example, if you are manually acknowledging messages to ensure end-to-end at-least-once delivery, then you cannot safely use the auto-recovery, since the delivery numbers would reset when the connection does, and you can accidentally aknowlodge the wrong message with delivery tag 5. (Acknowledge the new one, when you were trying to ack the old one).
Has any testing/research been done on if AutoRecovery is creating more problems than it solves? On that note, diagnostics/telemetry and logging is severely in need of an upgrade. That's on my radar for a potential PR as well.
In my implementation of that included my own recovery, I ended up needing to pass around the IModel itself with the delivery tags, so I can check if the channel I am about to acknowledge on is really the same one I received the message on. (There is no unique identifier of a channel instance, since even the channel number is likely to get re-used).
This should be easy to solve.
This is something that I feel the library itself could do automatically. For example, when putting the channel in ConfirmSelect mode, calling BasicPublish wouldn't return until the publisher confirm message was received, or a given timeout would be hit, or at least provide an overloaded method that does the same?
I agree that this is just something the library can do. That's definitely how I made it work in my client.
