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

Confirmation buffer on shutdown #60

Open
Crevil opened this issue Jan 12, 2021 · 5 comments
Open

Confirmation buffer on shutdown #60

Crevil opened this issue Jan 12, 2021 · 5 comments

Comments

@Crevil
Copy link

Crevil commented Jan 12, 2021

Thank you for adding support for publish confirmations in #43 and #44.
I have a question regarding how the buffer is handled during shutdown.

Say there are 10 messages in-flight waiting for confirmation in the confirmation buffer and we call cancel the context passed to the dialer in amqpextra.NewDialer(amqpextra.WithContext(ctx)).

Will the close call block until confirmations has been received or timed out or will we abort right away?

I wasn't able to find some documentation around this and I couldn't see any tests that tests that?

@makasim
Copy link
Owner

makasim commented Jan 12, 2021

I suppose It should send amqp.ErrClosed to all pending messages, wait till it is done and exit.

Ideally you should stop publishing, wait till all pending messages are published and confirmed and only after call publisher.Close()

@makasim
Copy link
Owner

makasim commented Jan 12, 2021

I gave it another though and It looks like there is room for improvement. Here https://github.com/makasim/amqpextra/blob/master/publisher/publisher.go#L365 we can catch graceful stop and give some time for messages to catch up.

@Crevil
Copy link
Author

Crevil commented Jan 12, 2021

Yes, that would make good sense. It would be nice with a timeout on the last catch up part, as to be able to indicate how long one is willing to wait for the issue to resolve.

@nilathedragon
Copy link

Ideally you should stop publishing, wait till all pending messages are published and confirmed and only after call publisher.Close()

How would I check if there are any messages still in-flight?

@makasim
Copy link
Owner

makasim commented Nov 3, 2021

By closing contexts in proper order. You have some inputs and outputs in your app, and a pipeline in between. So, you have to start stopping the app from inputs and go to outputs.

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

No branches or pull requests

3 participants