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

fix(queue-getters): filter undefined jobs from getJobs #3034

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

Conversation

DanFaudemer
Copy link

Ensure getJobs does not return undefined if a job is deleted between ID retrieval and fetching.

Why

  • getJobs could return undefined if a job was deleted between ID retrieval with a return type Promise<JobBase[]>
  • This caused unexpected behavior when processing job lists.
  • Fixing this ensures getJobs only returns valid job objects.

How

  • Updated getJobs to filter out undefined results.
  • Ensured that the function always returns a valid JobBase[] array.

Ensure getJobs does not return undefined if a job is deleted between ID retrieval and fetching.
@manast
Copy link
Contributor

manast commented Jan 30, 2025

Thanks for the PR. There is a subtle issue here and that is that the number of returned jobs will not match the requested amount based on the start and end arguments, which for an application that maybe is doing pagination it could lead to believe that there are no more pages left as the last call returned less jobs than expected. Maybe the best solution would be to perform the whole operation using MULTI in one transaction.

@DanFaudemer
Copy link
Author

You are right! Just for reference, the Python version is already filtering out None jobs: Python BullMQ - Queue.

I don’t know the codebase in-depth, but I believe that as soon as a job is missing, pagination will break—even without explicitly filtering out missing jobs—because job IDs will shift.

I see three possible solutions:

  • I am not a redis expert but as you mentioned we may use MULTI with WATCH on getRanges and have a retry mechanism in case jobs have been updated
  • Fetch additional pages until the result contains the expected number of jobs
  • Modify the return type of getJobs to include undefined jobs

Which solution do you think is the best approach?

@manast
Copy link
Contributor

manast commented Feb 3, 2025

Yeah, MULTI will not work in this case actually, because you need to have the job ids before you can actually fetch the jobs themselves, so the only way to do this properly would be with a small lua script that both gets the job ids and then fetches the jobs themselves and returns them in one array.

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