Is adding asynchronous non-blocking support possible?
Closed this issue · 9 comments
I noticed that the MessageReceived event is synchronous and blocking. I can't react to new messages while still processing a long-running previous one.
Would it be significant lift to add an asynchronous Task MessageReceived event that would allow immediate reaction to messages as fast as the server can process them?
I should have clarified. This issue is on the Server.
In other words, if I have a single web browser client make a request to the server and then immediate make a second request, the server can't respond to the second one until it finishes handling the first one.
Looked at the Server code for DataReceiver:
private async Task DataReceiver(ClientMetadata md)
{
string header = "[WatsonWsServer " + md.IpPort + "] ";
Logger?.Invoke(header + "starting data receiver");
try
{
while (true)
{
MessageReceivedEventArgs msg = await MessageReadAsync(md);
if (msg != null)
{
_Stats.IncrementReceivedMessages();
_Stats.AddReceivedBytes(msg.Data.Length);
if (msg.Data != null)
{
MessageReceived?.Invoke(this, msg);
...
The DataReceiver is asynchronous but the Invoke() to MessageReceve is not.
Honestly, I never even use Events because I don't really understand them. They seem to have weird gotchas. Is it possible to invoke an Event asynchronously? Is it as simple as changing this one line, or are there other issues to consider?
I just realized I could work around this issue by just spawning a new task in my own MessageReceived handler so that I can return immediately while the task goes on another thread.
No need to alter the library solely on my behalf. :)
Moreover, some people might WANT the blocking behavior.
I suggest we add an example to the README that shows a Task.Run(() => call your own code) example.
To demonstrate how the synchronous model can easily be switched to nonsynchronous.
We could modify the data receiver to do a Task unawaited = Task.Run(() => MessageReceived?.Invoke(this, msg). Under normal circumstances that would work fine, but could create a condition (under heavy load) where messages may be delivered out of order (since we don't control how tasks are scheduled/executed). Thoughts?
So one way to tackle this would be to offer a configuration like server.UseAsyncReception() and include it in documentation examples right up front. Offer two different MessageReceived() events and consumer binds to the correct one.
This makes it obvious to a beginner that he needs to be thinking about which way he wants to receive data. In the further details, you could explain the difference to help facilitate the choice. Its natural education with an elegant solution. I really like the fact that it makes you choose between blocking and non-blocking before you even begin.
I think you're offering ease-of-use as your library's primary selling point, so the performance cost of branching to check a memory variable to determine which event to call probably doesn't even enter into the equation. Obviously, if this was targeted as world's fastest high-throughput library, you wouldn't want to be doing memory fetches or branching just to receive data properly.
If you do what I suggested, the documentation would still need to point out this piece clearly:
If you bind to MessageReceivedAsync(), messages may arrive out-of-order under high loads. If you want to ensure messages are received in order, you need to bind to void MessageReceived() and establish your own queue or other system for receiving messages and handling them asynchronously.
One more thought:
It occurred to me that this problem of deciding how you want to receive data is notorious. LiteNetLib (which I love love love) is an extremely popular UDP library for .NET. You should really check it out if you haven't. It's beautifully crafted. But I digress.... LiteNetLib actually lets you choose multiple different configuration options for how you want to receive data... ordered/unordered are part of the options. That library has seen tremendous success, so its worth examining his approach to this issue.
Thanks @pha3z I will definitely check out that library! v2.2.0.6 provides server async handling of message events (forgot to add to the client), so v2.2.0.7 provides both. Perhaps the next step is to make this configurable (async vs sync message events). Closing for now and adding this configurability to the list! Cheers, Joel