azservice bus can over-issue receiving link credit

[jhendrixMSFT]

Over-issuing of credit can cause a receiver to hang in some circumstances. This has been fixed in later versions of go-amqp to return an error, so SB will need to update to handle this (unsure about EH).

[richardpark-msft]

@jhendrixMSFT

Our basic issue here is that we don't know (at start) what the credit limit should be.
Are there downsides to having an arbitrarily high credit limit when doing manual credit management?

[jhendrixMSFT]

It's not the arbitrarily high value, it's issuing more credit than the link has been configured to handle.

In the current design, when the receiver is constructed, you specify a max credit its link can handle. This max is used to create the buffered messages channel. The current version of go-amqp in use by EH/SB erroneously allows one to issue more credit than the preconfigured max; this can lead to the peer sending transfers that exceed the size of the messages channel which causes the receiver's mux to become blocked. This bug has been fixed in [email protected] so you can no longer issue more credit than the preconfigured max.

[richardpark-msft]

Sounds like the channel size is really the only thing that grows, so I think I'll do two things:

1. Check the credit limit, make sure you can't issue more credits than the ceiling. We already take into account any already issued credits, so this should be fine so long as the max requestd never exceeds the link max.
2. Increase the ceiling since it's a hard ceiling. I don't think we'd ever hit it, but 2048 was a rather arbitrarily chosen number anyways.

[jhendrixMSFT]

I'll also add that if this current design is too restrictive, we can consider changing it so there's no max.

[richardpark-msft]

I was thinking that too, but it doesn't seem too bad really.

SB has manual credits to avoid over-caching messages and having them expire while caught in some internal buffer. As part of this there's also a cap to the amount of messages that are returned in one batch. I'll set it high just to avoid having too much trouble, no issues.

For EH we just use the go-amqp auto-flow stuff so we can take advantage of the prefetching.

So it won't be so bad. The other stack I've used (rhea, js) has a similar issue with an internal circular buffer so it's not unheard of.