Skip to content

fix: queues stop working when there are more than 8k jobs with the same timestamp#550

Merged
compwright merged 1 commit intobee-queue:masterfrom
compwright:fixTooManyValueToUnpack
Nov 23, 2022
Merged

fix: queues stop working when there are more than 8k jobs with the same timestamp#550
compwright merged 1 commit intobee-queue:masterfrom
compwright:fixTooManyValueToUnpack

Conversation

@compwright
Copy link
Copy Markdown
Collaborator

Resubmitting #192 from @Tob0t and @mattbornski to rebase and re-run CI checks:

Since there is no progress on #123 I created another PR.

FYI: The commit from @mattbornski was taken.

Since our queues stopped working if there are more than 8k jobs with the same timestamp, we needed a fix asap. I know that the PR was not merged since there could be a further improvements. However as @mattbornski mentioned, better this fix with blocking redis for short time than processing no jobs at all.

We are already using this code on our production system since we needed a fix asap.

Disclaimer: I'm not a lua programmer, so feel free to adapt the PR ;)

@compwright compwright force-pushed the fixTooManyValueToUnpack branch from c28f586 to c53dcc7 Compare November 22, 2022 18:37
@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage remained the same at 100.0% when pulling c53dcc7 on compwright:fixTooManyValueToUnpack into bed66bf on bee-queue:master.

Copy link
Copy Markdown

@bradvogel bradvogel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving.

It looks like this was approved here but with this concern. However, I agree with the comment in the PR description that it's better to block Redis than to not process at all.

@compwright
Copy link
Copy Markdown
Collaborator Author

@tmeasday care to add a review before we move on this?

@compwright compwright merged commit 28ca743 into bee-queue:master Nov 23, 2022
beequeueci pushed a commit that referenced this pull request Nov 23, 2022
## [1.4.3](v1.4.2...v1.4.3) (2022-11-23)

### Bug Fixes

* queues stop working when there are more than 8k jobs with the same timestamp ([#550](#550)) ([28ca743](28ca743))
@beequeueci
Copy link
Copy Markdown
Collaborator

🎉 This PR is included in version 1.4.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

@tmeasday
Copy link
Copy Markdown
Contributor

Seems reasonable but it would be good to have a test I guess (I'm no expert on how these tests work anymore, or ever really).

@bradvogel
Copy link
Copy Markdown

@tmeasday You're absolutely right. I complete overlooked the fact that there wasn't a test 🤦‍♂️. I think this is OK to skip one for now since it's unlikely the original author is interested in adding one now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants