Cannot create multiple sessions per connection

[princjef]

As far as I can tell, creating a client using the current amqp10 implementation creates a single session, which becomes tied to the client/connection and is used for all links created on that connection.

While this works well for most cases, I chatted with the Azure Service Bus about some problems that we've been seeing related to #245 and it turns out that Service Bus expects almost every link to exist within its own session (though there are some specialized cases where two links should share a session). This has significant implications around connection/disconnection of links (and general communication - Service Bus doesn't design/test for single session per connection so we can't be sure that it won't break in the future). It turns out that for many cases where Service Bus sends a detach on a link (such as when a client is idle for too long), they invalidate the entire session, rather than just the individual link. As a result, reconnecting the link doesn't always work as expected and other links that still seem to be attached can start misbehaving or simply stop responding. While all of this is going on, the actual connection is still sending heartbeats back and forth just fine.

I'm going to devote a lot more time to this tomorrow to try to think through a reasonable design to this which can support this need without making the usage pattern for other clients more complicated. My initial thought is to add something like createSession() to the AMQPClient class, which would create a new session and then expose createSender() and createReceiver() off of whatever object gets returned. It wouldn't be as simple as that (that wouldn't handle reattach for the link vs. the session, for instance), but it seems like a reasonable starting point. Anyway, I'd be happy to hear any thoughts/concerns about this and will update with more once I've had more of a chance to think through the implications of such a change.

(Side note: the AMQP Protocol Guide for Service Bus states that there should only be one session per connection, but I have confirmation from the Service Bus team that this is not the case and that the documentation is simply out of date/wrong)

[princjef]

Alright I've poked around the code some more and have a slightly better idea as to what this would entail. Here's my current thinking:

1. Add a createSession() method to the AMQPClient class. I'm not passing any parameters for now, but it could potentially take a policyOverrides for the session portion of the client's policy. Just like when creating links, the client must first be connected before a session can be created. The method returns a promise which resolves with a Session object once the session is mapped.
2. Add an optional third parameter to createSender(), createSenderStream(), createReceiver(), and createReceiverStream() to provide a session on which the link should be created. If no session is provided, the link gets created on the session that gets associated with the client (this._session in AMQPClient). This preserves the existing interface/behavior while allowing new behavior.
3. Lazily construct this._session within AMQPClient. Since this change would open up the possibility of all links on a client being associated with user-created sessions, alter the connect() functionality to not include the creation and initialization of the link. Instead, create the client's session the first time the user tries to create a link without providing the optional session parameter. The reconnect logic would still pick up the session if it already exists and reassociate the new connection with it.
4. Add a policy option within the session to re-establish the session any time a link encounters an error (including on disconnect). Due to Service Bus invalidating the session for many link issues, it would be easier if the library automatically took care of killing and recreating the session and any associated links any time a link failed. This would be turned off by default and would only be enabled in the Service Bus policy for now.
5. Add the reattach policy option for sessions like we have for links. Since the session can now be closed without the connection closing due to item 4, it should also be able to perform reconnects just like the link in case something goes wrong in the process of reestablishing the session. This would also be turned off by default and only enabled for the Service Bus policy

Pros

• Keeps the existing interface the same for anyone who doesn't need this functionality
• Provides flexibility in pairing links up with sessions so that users can associate whichever links they need/want with whichever sessions they have to fit the needs of their scenario
• New APIs are extensions of existing ones. Besides the addition of createSession(), there are no new APIs that people need to understand, just new ways to associate things in the existing APIs

Cons

• Requires Service Bus users to do three steps (connect(), createSession(), createSender()/createReceiver()) instead of two (connect(), createSender()/createReceiver()) to get their link. This also means Service Bus users have to understand more about how Service Bus session/link relationships work than they should really need to. In 90% of cases, they'll want to just create a single link per session.
  • We could address that by adding methods like createStandaloneSender() and createStandaloneReceiver() which create a new session under the hood and associate it with the link for you, but I didn't want to go down that road unless people think it's worthwhile for simplifying the Service Bus use case.

I haven't yet dug into all of the code/state implications of 4 and 5 above, but implementing 1-3 seems to be more straightforward than I anticipated. If there are no objections, I'll start working up a PR with what I've described above in it so people can take a look and give their thoughts on the change and the approach.