Skip to content

feat(RedisBroker): poll for unacked events #11004

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

didinele
Copy link
Member

@didinele didinele commented Jul 18, 2025

Initially, as the branch name implies, I wanted to handle cases where a service is downscaled, potentially leaving some events orphaned (if Redis had already deemed they were owned by that consumer). As I wrote the logic though, I realized we effectively end up reading "dead events" (i.e. caused by a crash mid processing) that belong to the current consumer too!

As such, this actually supersedes #11003

One thing that's a bit of a TODO (probably not for this PR though, I'll keep iterating some more first) is to add a section in the README that warns users about dangerous ack usage (e.g. if you default on always acking at the very end of your logic, you risk duplicating certain actions, like database entries maybe; while acking at the very beginning is an opt-out of recovery logic). There's use cases to acking in all sorts of contexts, but I think we should describe them and recommend that users exercise caution.

Funnily, this could also serve as a workaround for #11001, but that one is still arguably important.

@didinele didinele requested a review from a team as a code owner July 18, 2025 16:00
@didinele didinele requested a review from iCrawl July 18, 2025 16:00
Copy link

vercel bot commented Jul 18, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

2 Skipped Deployments
Name Status Preview Comments Updated (UTC)
discord-js ⬜️ Ignored (Inspect) Jul 18, 2025 4:00pm
discord-js-guide ⬜️ Ignored (Inspect) Jul 18, 2025 4:00pm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Todo
Development

Successfully merging this pull request may close these issues.

2 participants