Skip to content
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

Add AMQP retry mechanism and example projects #312

Merged
merged 7 commits into from Dec 18, 2020

Conversation

Arkatufus
Copy link
Contributor

@Arkatufus Arkatufus commented Dec 17, 2020

Closes #288

Adds settings variant that's able to manage the Connection and Session states so that it plays nice with disconnection/reconnection; all the user need to do is provide the Address object.

Copy link
Member

@Aaronontheweb Aaronontheweb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to find a way to Connect asynchronously without blocking - it's a bigger issue here due to the backoff.

}

retry--;
if (retry == 0)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@@ -51,10 +69,45 @@ public override void PreStart()
{
base.PreStart();

var consumerCallback = GetAsyncCallback<Message>(HandleDelivery);
_receiver.Start(_amqpSourceSettings.Credit, (_, m) => consumerCallback.Invoke(m));
Connect().Wait();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Connect call is blocking here but there isn't much you can do about that - have to wait for the execution to finish somewhere in order to start the message pump.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, come to think of it - might want to just have the async state machine invoke a callback on the stage and signal failure / success. Given that this retry + backoff mechanism can run for several minutes, you don't want to tie the process up for that long. How hard would it be to do that?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll give it a look see, it shouldn't be that hard

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On second thought, can we separate this into a different PR?
It "works" as it is, I'll work on the thread blocking issue on another PR.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's fine with me

public override void PreStart()
{
base.PreStart();

Connect().Wait();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment as below about blocking on a really long-running call

src/Amqp/Examples/Amqp.V1.Sink/Program.cs Show resolved Hide resolved
@Aaronontheweb Aaronontheweb merged commit 4b62246 into akkadotnet:dev Dec 18, 2020
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

Successfully merging this pull request may close these issues.

Amqp Connector don't work with RestartSource
2 participants