Azure / azure-amqp

AMQP C# library
Other
94 stars 72 forks source link

Enable Link Recovery #208

Closed bainian12345 closed 1 year ago

bainian12345 commented 3 years ago

Enable link recovery for this AMQP client. Below are the major changes made to enable this feature:

To properly test the changes mentioned above, new tests and mock broker changes were added:

Note:

JoshLove-msft commented 2 years ago

Should this PR be targeting hotfix rather than main?

JoshLove-msft commented 2 years ago
  • Added mechanism to skip processing aborted and already processed deliveries sent to a ReceivingAmqpLink.

I assume that the implication here is that non-aborted/processed deliveries will be redelivered to receivers? Is there anything we need to do from the SB SDK perspective here or will it be automatic when reconnecting to the AmqpLinkTerminus?

JoshLove-msft commented 2 years ago

How is message locking going to fit in here? Are there going to be service side changes to disregard lock expiration when this feature is enabled?

bainian12345 commented 2 years ago

There will of course be a major change from the service side as well. Lock will still expire (it's not practical to hold the message in memory forever), we will still give it a hard cap from the service side. Not sure if this answer your question?


In reply to: 981945704

bainian12345 commented 2 years ago

From the SDK perspective, it would still need to ensure a reconnect attempt is made to the cloud service, as this library on its own has no reconnect capabilities. This PR just handles the AMQP details on negotiating which messages should be redelivered between the service and client.


In reply to: 981338624

bainian12345 commented 2 years ago

Why is that?


In reply to: 977570107

JoshLove-msft commented 2 years ago

Why is that?

In reply to: 977570107

all of the latest versions are shipped out of hotfix. Main contains a bunch of breaking changes that have not shipped.

JoshLove-msft commented 2 years ago

@xinchen10 are you able to review this PR?

xinchen10 commented 2 years ago

General comments on high level design.

  1. Recovery capability should be at link level. A connection may have multiple links with different roles and they may require different capabilities.
  2. The link endpoint state required by link recovery should not live in connection. Even if a connection is closed, one should still be able to resume a link.
  3. The library by default provides an optional volatile link endpoint state store, which can be used for resuming a link under transient errors. The user should be able to plug in their own store to handle advanced recovery scenarios. We don't have to support it right now but the design should allow for it in future.
xinchen10 commented 1 year ago

Replaced by #230