azure-sdk-for-net icon indicating copy to clipboard operation
azure-sdk-for-net copied to clipboard

The `static IQueueClient QueueClient` in the Microsoft.Azure.ServiceBus/BasicSendReceiveUsingQueueClient example is creating bugs and other usage questions

Open StingyJack opened this issue 5 years ago • 9 comments
trafficstars

From here.

I recently saw this in an ASP.net web api controller, copied pretty much verbatim from this example. This not only led to some nasty bugs WRT connection orphaning and premature closures , but also to a few questions. The most important one is "Is this supposed to be treated like HttpClient, where one instance is intended to be used by the whole application?" Its unclear from this console example if that's the correct usage case or not, and documentation is more or less empty. Can someone speak to this, and if its not, can someone update the example to not be so "console app specific"?

StingyJack avatar Apr 26 '20 15:04 StingyJack

ServiceBus connections are expensive to establish. Once a client exists, it should be cached rather than re-created.

SeanFeldman avatar Apr 27 '20 04:04 SeanFeldman

@SeanFeldman - yeah, but is the expectation for usage similar to HttpClient, i.e. once per application (mostly), or should it be treated like SQLServer connections (per user/non-atomic operation)? The example is a console application for a single user, and the necessary implementation could be incredibly different for a Web API controller.

I dont see anything in its source that leads me to believe its like the HttpClient, but I could be missing something and the docs page for this component is just the auto generated stuffs. "Expensive" is also subjective unless there are some limits that we should refer to, but i'd rather take a performance hit than to have two users trying to share a QueueClient at about the same time if the behavior is unknown to be safe.

StingyJack avatar May 02 '20 15:05 StingyJack

Oh, I don't disagree. Documentation is extremely important and I have been pointing out that aspect for a long time.

As far as I know, the connection should not be established each time as it's a super expensive operation performance-wise. There are also quotas. You cannot exceed 5000 connections per namespace. If you do, connections will be refused. Clients (such as QueueClient) are thread-safe. With my usage, I have a single MessageReceiver receiving messages concurrently, and 1 to N (N is between 1 and mostly 2) MessageSenders per destination. The connection object is shared if transactionality is necessary. Otherwise, a connection object sender/receiver.

@axisc do you think it would be possible to provide documentation for something that is not a console application scenario?

SeanFeldman avatar May 04 '20 03:05 SeanFeldman

excuse my ignorance a bit here, is the 5K connections per namespace referring to concurrent connections or lifetime of service?

The use I found was in an API controller thats getting created a-la castle windsor per user request, but that still had the static field for the queue client. I dont imagine the usage is much more than 100 per day, always to the same queue, but it looked like we were getting clients and connections orphaned if two users requests overlapped at about the same time.

StingyJack avatar May 11 '20 04:05 StingyJack

Hi any update on this documentation? It would be very useful.

BhuvanaBellala avatar Feb 26 '21 01:02 BhuvanaBellala

Transferring this to the SDK team, so they can provide information of how to do this in the clients, and optionally see if more documentation needs to be created for this.

EldertGrootenboer avatar Aug 05 '22 21:08 EldertGrootenboer

Hi @StingyJack. Thanks for reaching out and we regret that you're experiencing difficulties. Since you're inquiry is regarding the legacy package (Microsoft.Azure.ServiceBus), we'll need to defer to @shankarsama and @EldertGrootenboer for guidance. That said, since Microsoft.Azure.ServiceBus is deprecated, we strongly advise upgrading to Azure.Messaging.ServiceBus.

For the client types in Azure.Messaging.ServiceBus, the Client lifetime guidance is to cache clients and use them as singletons for the lifetime of the application. In an ASP.NET context, we recommend using the Microsoft.Extensions.Azure package which provides a set of Service Bus extensions to allow for directly registering the clients with DI so that lifetime management takes place implicitly.

jsquire avatar Aug 08 '22 13:08 jsquire

Thanks for the feedback! We are routing this to the appropriate team for follow-up. cc @shankarsama, @DorothySun216, @EldertGrootenboer, @saglodha.

Issue Details

From here.

I recently saw this in an ASP.net web api controller, copied pretty much verbatim from this example. This not only led to some nasty bugs WRT connection orphaning and premature closures , but also to a few questions. The most important one is "Is this supposed to be treated like HttpClient, where one instance is intended to be used by the whole application?" Its unclear from this console example if that's the correct usage case or not, and documentation is more or less empty. Can someone speak to this, and if its not, can someone update the example to not be so "console app specific"?

Author: StingyJack
Assignees: EldertGrootenboer
Labels:

Service Bus, Service Attention, Client, customer-reported, question, needs-team-attention

Milestone: -

ghost avatar Aug 08 '22 13:08 ghost

@jsquire - thanks for returning to this. Others watching this thread may still need the answer but I dont (changed jobs in the 2 years since I opened this)

StingyJack avatar Aug 09 '22 17:08 StingyJack

As this is an issue with the old SDK, and the original poster is not in need of the answer anymore, we will close this issue. We recommend moving to the latest SDK, for which Jesse has thoroughly described the behavior.

EldertGrootenboer avatar Aug 24 '22 16:08 EldertGrootenboer