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(redis): add "pending" task to re-queue unhandled messages #20

Merged
merged 1 commit into from
Nov 17, 2023

Conversation

svix-onelson
Copy link
Contributor

@svix-onelson svix-onelson commented Nov 2, 2023

In svix-server's redis queue implementation, there's a spawned task to look for messages that have been "claimed" (i.e. pulled off the queue) but not ack'd or nack'd within some deadline period.

This was missing from the omniqueue version, added in this diff.

This refactors the prior background task spawned for delayed messages so it and the new one for the pending sweeper are bundled together in a JoinSet.
Since dropping a JoinSet means the spawned tasks under it will abort, the handle is now held by any consumers/producers that share the same config.

@svix-onelson svix-onelson force-pushed the onelson/redis-pending-task branch from 7b762c3 to 7cb5316 Compare November 2, 2023 05:35
@svix-onelson svix-onelson changed the title add "pending" task to re-queue unack'd messages add "pending" task to re-queue unhandled messages Nov 2, 2023
@svix-onelson
Copy link
Contributor Author

Leaving this as a draft until I add some tests to show this actually works, as well as resolve some of the FIXMEs sprinkled throughout. Most of the open questions relate to exposing things via config and similar.

Base automatically changed from onelson/memory-queue-refactor to main November 6, 2023 20:48
@svix-onelson svix-onelson force-pushed the onelson/redis-pending-task branch 2 times, most recently from 8a8b0c6 to f432b32 Compare November 16, 2023 03:45
In svix-server's redis queue implementation, there's a spawned task to
look for messages that have been "claimed" (i.e. pulled off the queue)
but not ack'd or nack'd within some deadline period.

This was missing from the omniqueue version, added in this diff.

This refactors the prior background task spawned for delayed messages so
it and the new one for the pending sweeper are bundled together in a
`JoinSet`. Since dropping a `JoinSet` means the spawned tasks under it
will abort, the handle is now held by any consumers/producers that share
the same config.
@svix-onelson svix-onelson force-pushed the onelson/redis-pending-task branch from f432b32 to 232f1e8 Compare November 16, 2023 20:29
@svix-onelson svix-onelson marked this pull request as ready for review November 16, 2023 20:32
@svix-onelson svix-onelson requested a review from a team November 16, 2023 20:32
@svix-onelson svix-onelson changed the title add "pending" task to re-queue unhandled messages feat(redis): add "pending" task to re-queue unhandled messages Nov 16, 2023
@svix-onelson svix-onelson merged commit a95ce9a into main Nov 17, 2023
2 checks passed
@svix-onelson svix-onelson deleted the onelson/redis-pending-task branch November 17, 2023 19:13
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.

2 participants