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

Example Client: Implementation of a Consumer with reconnection support #40

Closed
andygrunwald opened this issue Jan 21, 2022 · 6 comments · Fixed by #109
Closed

Example Client: Implementation of a Consumer with reconnection support #40

andygrunwald opened this issue Jan 21, 2022 · 6 comments · Fixed by #109

Comments

@andygrunwald
Copy link
Contributor

example_client_test.go shows a great way how to build a Producer with reconnection support.

The Consuming part is not covering the reconnection support:

// Stream will continuously put queue items on the channel.
// It is required to call delivery.Ack when it has been
// successfully processed, or delivery.Nack when it fails.
// Ignoring this will cause data to build up on the server.
func (session *Session) Stream() (<-chan amqp.Delivery, error) {
if !session.isReady {
return nil, errNotConnected
}
return session.channel.Consume(
session.name,
"", // Consumer
false, // Auto-Ack
false, // Exclusive
false, // No-local
false, // No-Wait
nil, // Args
)
}

Even if reconnection support is mentioned as a non-goal for the library itself, I do think showcasing this in the example might be a good thing.

Right now, the implementation is only possible with a hack involved.
Mainly due to these two issues:

These deadlocks prevent the consumer delivery channel to be closed during an unexpected connection loss (e.g., when the RabbitMQ server goes down).

@DanielePalaia
Copy link
Contributor

@andygrunwald Now that we understood what happened in #18 I think the example you propose can be just like an extension to the example we made in the documentation on how to handle channels for NotifyClose to avoid the deadlock for a consumer?

@andygrunwald
Copy link
Contributor Author

Yeah, exactly. I was waiting for #18/#32.
I have it implemented incl. the deadlock in one of my projects.

If you are good with it, I would make a PR in the next few days and playback my implementation into the example client. Might take a few days.

@DanielePalaia
Copy link
Contributor

Sure! good enough! Thanks @andygrunwald

@lukebakken
Copy link
Contributor

This gives an example of auto-reconnection as well:

4ce2c8e

@metalrex100
Copy link

@andygrunwald any updates with your example client?

@andygrunwald
Copy link
Contributor Author

@metalrex100 Still on my list. My client is working in production since quite some time. Will check if I can make some time in the next days to push it back.

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 a pull request may close this issue.

4 participants