-
Notifications
You must be signed in to change notification settings - Fork 18
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
Comments
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() |
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. |
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. |
How would I check if there are any messages still in-flight? |
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. |
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?
The text was updated successfully, but these errors were encountered: