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

feat: added consumer.WaitForHandlers #150

Closed

Conversation

astromechza
Copy link
Contributor

I'd like to propose adding a WaitForHandlers method to the consumer which waits for any running handlers to complete processing their current messages. This is because Close doesn't wait for the handlers to finish or queue to be drained (this makes sense).

This solves a couple of issues I've been facing:

  1. Some message handling may be performing actions that need to clean up properly: file/database IO, external requests, even when idempotent may cause corruptions if the handlers are interrupted without being allowed to shutdown safely.

  2. Applying a circuit breaking pattern to stop a handler from consuming new messages if they are failing frequently due to overload or dependency failures.

The code in this PR could be added to the handler method itself, but I think it may be a useful pattern that others may take advantage of so I'd like to consider including it in this library.

Happy to discuss further :)

@wagslane
Copy link
Owner

wagslane commented Mar 4, 2024

I like this idea. I actually think .Close should just always wait for handlers though

@eric-chao
Copy link
Contributor

I like this idea. I actually think .Close should just always wait for handlers though

How to finish the handlers gracefully?

@thibleroy
Copy link

Hello @astromechza,
I've made some changes to support handlers completion in consumer.Close:

@astromechza
Copy link
Contributor Author

I'm going to close this PR since #166 seems like a better approach :)

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.

4 participants