DO NOT MERGE [SPIKE] Move to async/await

[danielmarbach]

Do not merge, references #83.

This is for demonstration purposes to show that a proper async implementation would essentially mean that you need to maintain two code paths. The async one and the sync one. Each and every bug needs to be fixed in two places.

I worked myself up from NetworkBinaryReader to the connection. Of course the work is nowhere near being finished

and yes it doesn't compile yet ;)

@michaelklishin is this something you would be willing to take as a PR (of course when done)?

[michaelklishin]

Wow, thank you @danielmarbach, this goes well beyond what I expected (and it took you only a few hours).

When I was saying that going all in on async/await isn't currently an option I only meant the IModel and perhaps IConnection APIs. We absolutely can, and would be happy to, replace all the networking I/O and parsing machinery with something asynchronous. In other words, "90% source compatibility" is what I'm after, binary compatibility isn't required for our next release, 3.7.0.

I'd need to set aside an hour or two to review this but at a quick glance, many parts feel perfectly appropriate. I'm very happy to see the old NetworkBinaryWriter, WireFormatting, and related parts to go.

@danielmarbach have you tried building the WinRT version or running tests (which is fairly involved on Windows as it requires using msys2, I know).

[danielmarbach]

@michaelklishin So you'd be fine with having

IAsyncConnection, IAsyncModel, AsyncFrame...

everything duplicated?

We might need to do voice call together. I think we still missunderstand each other. For example

I'm very happy to see the old NetworkBinaryWriter, WireFormatting, and related parts to go.

They would stay in the code for the synchronous version!

You can reach me on twitter, skype under danielmarbach

[michaelklishin]

In my ideal scenario we'd have IModel and IAsyncModel. Same for connections. Their API will different only to the extent async/await differs from normal method returns (there should be few surprises when going from one version to the other).

But all the implementation details — for example, I doubt that Frame is commonly used directly by our users — can build on async/await. We require .NET 4.5 these days, and can even bump that further if .NET 5.0 comes out in time (and makes sense for us). Those not willing to upgrade can use .NET client 3.6.x with RabbitMQ 3.7 and so on.

In other words, we have to have 2 versions of what is obviously publicly facing. Everything else can built on the async version and "synchronised" for the needs of IModel we have today. We obviously want to have as little duplication as practical.

[michaelklishin]

ConnectionFactory then can have two (sets of) methods for creating "regular" and async connections. We'd have to have another set of key tests ported to cover the new API. That's expected and the necessary evil in our case.

Going all in on async/await means most users simply won't upgrade for a year or two.

[danielmarbach]

And here comes the problem

http://blog.stephencleary.com/2012/07/dont-block-on-async-code.html
https://msdn.microsoft.com/en-us/magazine/jj991977.aspx

That means that we are left with the choice of writing everything twice, once for synchronous operations and once for async ops or just living with this issue.

A work around we managed to find is to run the root tasks (which we do control) on our own task scheduler, so they won’t compete with the I/O operations. But that is hardly something that I am thrilled about.

https://ayende.com/blog/166913/mixing-sync-async-calls

I can find you more links if you want ;)

[danielmarbach]

You have two choices:

1. All-in
2. Redundantly implement synchronous and asynchronous code base

There is NO middle-ground. You are going to run really weird scenarios if you do. You can't make assumptions in which environments your lib is going to be used and what the SyncContext will be.

[michaelklishin]

@danielmarbach that's a broad generalisation. Take a look at how continuations work in this client. They are not tied to synchronous I/O. The IModel API can have any implementation but some operations are synchronous semantically in the protocol and currently return objects and not monadic values (some kind of futures) or use async/await.

SocketFrameHandler and such can be replaced entirely.

To convince me to go all in on async you'd need provide some real API examples (e.g. port our tutorials and/or bits of the API guide), not pile up links with generalisations.

[michaelklishin]

@micdenny as an EasyNetQ contributor, what do you think of this discussion? Would it make things easier for EasyNetQ if we moved the client to be 99% based on async/await?

[michaelklishin]

@mikehadlow same question to you :)

[danielmarbach]

@michaelklishin Yes I was talking in general terms, true.

Take a look at how continuations work in this client. They are not tied to synchronous I/O.

Get it. Because you are throwing the frames into a worker queue and hand them off behind the scenes.

If we stick to that design we might be able to reduce the redundancy, indeed

[michaelklishin]

@danielmarbach to keep this more focused and productive, please come up with an async API. We need it regardless of what happens to the one we have :)

I'm not ruling out going all async/await. Lets see what EasyNetQ maintains say. Later it may make sense to ask on rabbitmq-users and see how much people like the new API.

Thank you again for your time.

[micdenny]

to me would be good to have only async api, because the framework out there start making only async api as .net core is doing, of course only in that points where have sense like every IO parts.

the point is regression, for this @Pliner and me bitterly decided to create two both api sync and async, and more bitterly decided to duplicate code (this to avoid unwanted thread starvation).

we can also say that EasyNetQ is still in a pre-release version (0.9), and we could breaking change everything, but actually has the community growth so much, I would also say that this isn't a pre-release version anymore, is not true, to many people I think rely on this, even if it's clear that is 0.9 version, but I likely to say that a bomb would be dropped on my house if I will break all the ENQ api.

I'm not saying no, I'm just thinking like you now......... 😄

[michaelklishin]

@micdenny on the brighter side, both EasyNetQ and RabbitMQ .NET client versions we have today will continue to function with at least several future RabbitMQ server releases without any changes.

[micdenny]

yes you're right @michaelklishin in fact we had to make the same decision to pass on .net >=4.5 and follow the RabbitMQ decision (that is just fine), and we decided to support only >=4.5 now on, so that's similar to this question, to have only async/await api or not 😄 I think MS decided to have only async for the same reason, why bothering with two implementation when the world has changed and there is only multi-core processors, and apart from multi-core async in IO scale better, full stop! it uses signaling stuff under the wood against the IOCP thread pool. I don't know well the internals of rabbitmq and how it uses the network and if it gain also the scalability of IOCP, but anyway is an IO, and an async api is requested in modern design and force to use only async is a good choice IMHO (at least on >=4.5 projects).

[michaelklishin]

OK, lets sketch out an async/await API and look into replacing what we have with it. 3.6.x will introduce a few small features and the users who cannot upgrade on a short schedule will have to use it for some time.

[danielmarbach]

Guys I love you ;)

Am 08.01.2016 um 19:45 schrieb Michael Denny notifications@github.com:

yes you're right @michaelklishin in fact we had to make the same decision to pass on .net >=4.5 and follow the RabbitMQ decision (that is just fine), and we decided to support only >=4.5 now on, so that's similar to this question, to have only async/await api or not I think MS decided to have only async for the same reason, why bothering with two implementation when the world has changed and there is only multi-core processors, and apart from multi-core async in IO scale better, full stop! it uses signaling stuff under the wood against the IOCP thread pool. I don't know well the internals of rabbitmq and how it uses the network and if it gain also the scalability of IOCP, but anyway is an IO, and an async api is requested in modern design and fo rce to use only async is a good choice IMHO (at least on >=4.5 projects).

—
Reply to this email directly or view it on GitHub.

[phatboyg]

Full asynchronous support and dropping the synchronous API for the next release sounds great. I'd be happy to test with MassTransit as well as contribute back any fixes to bugs found. I've had to build my own synchronization around models and connections already, it would be great to eliminate that need.

I'm sure the channel blocking support for continuations will need to be smart enough to link back to the awaited caller, so there's likely some work to make sure we don't have people using a model from multiple threads at the same time by mistake and fail gracefully.

[danielmarbach]

One thing from my side. I can definitely work with you guys on shapping the APIs and also provide ideas and implementations. But I can only commit few hours per week since I mostly do that in my free time. If you need more commitment I can definitely ask internally if I can contribute more but cannot promise anything. The next two weeks I'll be travelling east and only partially available. But after that I should be more responsive

Am 08.01.2016 um 19:47 schrieb Michael Klishin notifications@github.com:

OK, lets sketch out an async/await API and look into replacing what we have with it. 3.6.x will introduce a few small features and the users who cannot upgrade on a short schedule will have to use it for some time.

—
Reply to this email directly or view it on GitHub.

[michaelklishin]

@danielmarbach we understand. 3.7.0 is at least 6 months away.

[michaelklishin]

@jeremymeng sorry to drag you into this issue but you've looked into a spike of a CoreCLR port and (according to your GitHub page) work for MSFT. Do you have an opinion on what's discussed above?

[danielmarbach]

@michaelklishin It seems that https://github.com/dotnet/corefxlab/tree/master/src/System.Threading.Tasks.Channels would be quite interesting for RabbitMQ. Large parts of that can be rebuilt and included into the library directly without the need for CoreCLR. Or would you like to build it for CoreCLR as a first class citizen?

[michaelklishin]

@danielmarbach I'm happy to replace our consumer work service with something standard. It can be a separate library at first. The purpose of it is this: avoid blocking of I/O reads (and incoming protocol methods dispatch) even with slow/blocking consumers, while maintaining per-channel method dispatch ordering guarantee.

The channels library seems very relevant. Will cause some confusion in the terminology but oh well.

[danielmarbach]

Have to dig deeper first but I think in a pure IO async world we can achieve that with semaphoreslim and potentially a concurrentqueue

Am 09.01.2016 um 15:02 schrieb Michael Klishin notifications@github.com:

@danielmarbach I'm happy to replace our consumer work service with something standard. It can be a separate library at first. The purpose of it is this: avoid blocking of I/O reads (and incoming protocol methods dispatch) even with slow/blocking consumers, while maintaining per-channel method dispatch ordering guarantee.

The channels library seems very relevant. Will cause some confusion in the terminology but oh well.

—
Reply to this email directly or view it on GitHub.

[danielmarbach]

But of course I'm still learning the internal design. Is there any doco about the build and release process and the generator part?

Am 09.01.2016 um 15:02 schrieb Michael Klishin notifications@github.com:

@danielmarbach I'm happy to replace our consumer work service with something standard. It can be a separate library at first. The purpose of it is this: avoid blocking of I/O reads (and incoming protocol methods dispatch) even with slow/blocking consumers, while maintaining per-channel method dispatch ordering guarantee.

The channels library seems very relevant. Will cause some confusion in the terminology but oh well.

—
Reply to this email directly or view it on GitHub.

[michaelklishin]

The docs on release process are not public, sorry. I don't think there's anything on the code generator, fortunately it is a fairly small tool. It takes in JSON from rabbitmq-codegen as an input.

[phatboyg]

Is the code generator even still relevant? Wouldn't it be easier to just have the code converted and used? The only thing people use the client for is talking to RMQ. Any other ideas seem to have disappeared. The AMQP for RMQ is just that, period.

[danielmarbach]

Less work is always good ;)

Am 09.01.2016 um 17:29 schrieb Chris Patterson notifications@github.com:

Is the code generator even still relevant? Wouldn't it be easier to just have the code converted and used? The only thing people use the client for is talking to RMQ. Any other ideas seem to have disappeared. The AMQP for RMQ is just that, period.

—
Reply to this email directly or view it on GitHub.

[michaelklishin]

New protocol methods are introduced every once in a while. I'm not sure what "other ideas" you're referring to. This issue is not about code generation. Lets not try to reinvent everything at once.

[hammett]

I solved a lot of throughput issues in our infrastructure by embracing an async rabbitmq library. I had to code my own, though: https://github.com/clearctvm/RabbitMqNext

Previously the old client IO was the bottleneck, now rabbitmq is the bottleneck and we got over an order of magnitude improvements in throughput.

Of course, not at all api compatible. And async/await is more tricky than it looks from distance.

[phatboyg]

@hammett rocking it. Definitely going to check out your custom client. +1

[danielmarbach]

@hammet will have a look as well :)

Am 13.02.2016 um 03:19 schrieb Chris Patterson notifications@github.com:

@hammett rocking it. Definitely going to check out your custom client. +1

—
Reply to this email directly or view it on GitHub.

[hammett]

Thanks. Feedback is welcomed. Mind you, not ready for production use. It's going thru a lot of stress testing at this moment, and while it seems stable, I'm experimenting with read barriers to reduce even more buffer allocations. Try it, but dont change production code to use it YET :-)

[michaelklishin]

I've recently reviewed this with someone and will close this and post some thoughts about our plan to #83. Thank you very much for providing this spike PR, it's influenced our thinking on the subject.

[annymsMthd]

This part would benefit greatly from async / await as the thread is waiting to receive from the network serially