-
Notifications
You must be signed in to change notification settings - Fork 179
Allow duplicate jobs to be discarded #586
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
base: main
Are you sure you want to change the base?
Conversation
Hey @mhenrixon, thanks for this! There was already a PR for it: #523 I need to review what happens here for the bulk enqueue case, which was the tricky part of that other PR. |
657cdab
to
4e3c7f2
Compare
I need to learn to read all the issues. Sorry about the extra PR.
After working on sidekiq-unique-jobs, and with sidekiq. I have learned that some visibility is appreciated. One of the reasons to use a database-backed queue system is to maintain visibility and avoid paying a blackmailer for keeping the data safe. I am happy to close this if you prefer the other one @rosa. I didn't look into the other PR in detail, but the bulk enqueue sounds important. |
Oh, no worries! I think the idea here is better than what we were trying to do in the other PR, so I think combining these two would be the best! I need to review the other one again after some changes so I'll review these two together and will think how to proceed, maybe transplanting some commits over 🤔 |
Feel free to add commits or let me know what to pick or how I can help land this in the main branch. I am at your service! |
I just read this and thought: would it make sense, or be worth it, to add some kind of metadata field to the job to indicate that it has been discarded? |
I thought about that but opted for the simplicity while maintaining visibility. I am open to the idea though. |
4e3c7f2
to
c268ae9
Compare
Should resolve #176
I want to prevent the following:
These jobs can under no circumstances end up in this situation, discarding the new jobs instead of queuing them ensures this won't happen.
I took the simplest possible approach using terminology from sidekiq-unique-jobs. All feedback appreciated.