Missing async methods for PullSubscriptions

[turowicz]

Why is there no way of making PullSubscriptions via async and no way of Fetching messages from them via async? Seems like a lot of CPU goes to waste.

[scottf]

Because pull is a polling model. I suppose using the lower level pull api you can build this on your own.

[turowicz]

Well, pull is an I/O call and therefore in order to comply with dotnet async guidelines it needs to have an truly async subscrube/pull/fetch methods. Wrapping what you currently have in a Task will only result with wasted CPU time and not a real async workflow.

Please read this official guide:
https://github.com/davidfowl/AspNetCoreDiagnosticScenarios/blob/master/AsyncGuidance.md#asynchrony-is-viral

[turowicz]

Also, NATS is meant for high performance environments. Not having a truly async pull is a waste of CPU and renders this library incompatible with NATS project goals.

[turowicz]

Can I get @ReubenBond and @benjaminpetit from the .NET Orleans team chip in?

[turowicz]

@scottf FYI this is the context: https://github.com/Surveily/Orleans.Streaming.NATS

[scottf]

If you need async, I would switch to push. There are no plans at this time to augment the pull api with async. You are welcome to write up a full issue and the client team can consider it.

[turowicz]

@scottf your pull implementation has a sync block of the CPU thread SubsequentWaits x BatchSize meaning that for a batch of 100 messages it may block the CPU up to 50000ms. This code is definitely a rewrite candidate for async.

[scottf]

The fetch is (my own) bad code that has to be re-written, and is a completely separate issue of having pull be async or having async versions of the pull apis.

You are correct, it technically "may" block. But in reality it doesn't because that timeout only applies if there are not messages from the server already in the internal buffer. Remember the server sends as many as it can up to the batch size, so in reality, subsequent waits will take nanos b/c it's just pulling the message from the buffer, not waiting for the server.

Let me reiterate, adding an async pull api to the client requires approval by the client parity team as a common pattern or as an acceptable per language difference. This has not been fully discussed yet, but is being considered.

Once the fetch is fixed, you may be able to see a way to build your own async pull.

[scottf]

PR with fetch change nats-io/nats.net#558 Also tuned some next message code.

[turowicz]

Dear @scottf

I fully appreciate your efforts to make this work for me. Unfortunately it seems that we are having a problem communicating. I believe that you think I am asking you for a change in NATS.io API. I am not looking to change anything in NATS.io. What I am talking about is a change in the .NET implementation of the NATS.io client. Currently (even after your change) its blocking CPU for loops that wait for messages in a timeout.

The line below should be using async await implementation either via async iterator or just standard Task.

https://github.com/nats-io/nats.net/pull/558/files#diff-ff6694b7b4633830fc7ec2c469488117fb843f35c85a0e723ccb0b94eee3dbd5R81

In .NET it is required for all I/O calls to by fully async. This is a guideline that has existed since year 2012. Everything else will eventually result in wasted CPU resources.

[scottf]

Fetch is a blocking call and will remain a blocking call. The discussion of whether or not to add an async version of pull fetch is on the table. I could argue that technically, fetch is well past the I/O stage. As not a .NET expert, I defer to @sixlettervariables @danielwertheim @ColinSullivan1 regarding the discussion about the underlying implementation, I/O best practices and at what level of the client something like that would need to be implemented.

As a reminder, this is an open source project. You are welcome to make a PR and propose solutions that support consideration of your intent.

[ColinSullivan1]

I believe the entire stack under the callers thread/task would need to use async APIs to properly achieve this. Since we do the lower level socket/network buffer reads in another thread it may not be as much work as it seems. There have been requests to support async event handlers which would require the same type of changes.

The base NATS API here has been around a long time. At some point, we'll want to modernize this to take advantages of newer features. Believe it or not, based on user requests the initial version of this assembly was compilable against .NET 3.5.

[sixlettervariables]

@scottf I believe the entire stack would need to be rewritten to meet .NET Async guidelines, as @ColinSullivan1 notes. Otherwise we're not really Async compatible, just sync-over-async.

[turowicz]

I understand your points. While its nice to have support for a version of .NET from 2007, it certainly is a bummer that such a high-throughput system (NATS) cannot be used to it's full potential within .NET.

I understand that push API has some async but unfortunately I really need the WorkQueue policy here. Perhaps there is a way to achieve something similar with push?

[turowicz]

You might find that kubernetes users will specifically ask for more async as its really important in a shared cpu workflows.

[scottf]

You can do work queue style delivery with push. There is a consumer configuration property DeliverGroup, which is essentially an overload of the queue field on the subscribe api.

[turowicz]

Can you show me some pseudocode / example?

[plaisted]

I'll just second @turowicz here that this would be a great improvement, the sync blocking fetch is making this very inefficient in kubernetes with shared CPUs. The push APIs do not give enough control for what we need to do so we are having to figure if there is a workaround / another library / messaging system that will fit better.