Skip to content

lack of async code / sync over async #947

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
JanEggers opened this issue Oct 6, 2020 · 8 comments
Closed

lack of async code / sync over async #947

JanEggers opened this issue Oct 6, 2020 · 8 comments

Comments

@JanEggers
Copy link
Contributor

Im having some deadlocks in my Unittests during shutdown. it is certainly related to #874.
Im not using a sync task completion source but something else that leads to the test executing in the context of receiving a message.

I know that it would be a big impact breaking change but are there any considerations to make more operations async?

there seem to be some async code paths in the implementation but as long as they use sync over async they are prone to cause deadlocks / use more resources (threads) than actually needed...

https://github.com/rabbitmq/rabbitmq-dotnet-client/search?q=getawaiter%28%29.getresult

I really like the project bedrock stuff @davidfowl came up with but the rabbitmq bits in there are early prototype / id rather like to use the official package..

https://github.com/davidfowl/BedrockFramework/tree/master/src/Bedrock.Framework.Experimental/Protocols/RabbitMQ

@michaelklishin
Copy link
Contributor

See #83. This client has over a decade of history and making it entirely async is not going to work, although to some extent this has been done (e.g. there's an async consumer operation dispatcher). We have concluded that an "everything async" client should be a separate library.

@asbjornu
Copy link
Contributor

asbjornu commented Oct 9, 2020

@michaelklishin, I got the clear impression from #869 that 7.0 is going to be full async. What am I misunderstanding?

@sungam3r
Copy link
Contributor

sungam3r commented Oct 9, 2020

I got the clear impression from #869 that 7.0 is going to be full async. What am I misunderstanding?

I asked the same question.

@JanEggers
Copy link
Contributor Author

@asbjornu thx for pointing me to that pr looks promising

@michaelklishin
Copy link
Contributor

The comments in #869 make it pretty clear that there are many open-ended questions and we don't want to change the API of the client entirely as very few would upgrade.

@JanEggers
Copy link
Contributor Author

JanEggers commented Oct 10, 2020

@michaelklishin in #869 you are talking about a call. Did that happpen / what was the conclusion?

We have concluded that an "everything async" client should be a separate library.

is this the conclusion? If so is this project started already? Can I start it?

@stebet are you going to rebase #706 any time soon? I would be willing to help if that is ok for you.

@stebet
Copy link
Contributor

stebet commented Oct 10, 2020

There is a lot of stuff that could be reused between the sync/async libraries (most of the (de)serialization among other things). Let's see if I can take some of the async work I've made and start something this week. I'll keep y'all in the loop for feedback/ideas :)

Sound good?

@asbjornu
Copy link
Contributor

The comments in #869 make it pretty clear that there are many open-ended questions

@michaelklishin, true. But the pull request is both merged and assigned to the milestone 7.0.0, giving the impression that not only is async incoming, it will arrive in version 7 of RabbitMQ.Client.

and we don't want to change the API of the client entirely as very few would upgrade.

I beg to differ. As a consumer of RabbitMQ.Client I'm bothered by the lack of full async on an almost daily basis and would love to change it out with a new official one supporting async, even though it would mean rewriting all of my RabbitMQ-interfacing code (which isn't much anyway; most of the important stuff happens both before and after de/serialization, which means it is going to be completely reusable).

There is a lot of stuff that could be reused between the sync/async libraries (most of the (de)serialization among other things). Let's see if I can take some of the async work I've made and start something this week. I'll keep y'all in the loop for feedback/ideas :)

That sounds great, @stebet! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants