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

[Bug]: Removed child dependency job still has parent set on next attempt / after delay #2833

Open
1 task done
victorandree opened this issue Oct 16, 2024 · 0 comments
Open
1 task done
Labels
bug Something isn't working

Comments

@victorandree
Copy link

victorandree commented Oct 16, 2024

Version

v5.20.0

Platform

NodeJS

What happened?

If a job calls job.removeChildDependency() and then delays or fails, its parent will be set on the next attempt. This will cause another call to job.removeChildDependency() to throw an exception, if the parent job has finished.

After await job.removeChildDependency(), job.parent is undefined. I'd expect it to be undefined on the next attempt, too.

To work around this, I need to (1) try...catch the call to job.removeChildDependency, or (2) ensure it doesn't get called twice by, for example, changing the job's data.

My use case for removeChildDependency is to let a parent job finish, if the child job has to be delayed for a long time for some reason (in my case, it's due to resource-specific rate limits).

How to reproduce.

The following code will set up a queue where the parent job schedules two child jobs. The second child job (id: c-456) will remove itself from the parent and then delay itself. This allows the parent job to finish.

However, once the (now detached) child job runs again, its job.parent property will still be set to the parent job. Calling job.removeChildDependency would then fail, because the parent will have been removed.

This is demonstrated by the log output in the next section. For the second attempt of c-456, it still prints (p: parent-1), even though the dependency was removed in the previous attempt.

import { DelayedError, Queue, WaitingChildrenError, Worker } from 'bullmq';
import IORedis from 'ioredis';

const connection = new IORedis({ maxRetriesPerRequest: null });

const queue = new Queue('foobar', {
  connection,
  defaultJobOptions: {
    attempts: 10,
    removeOnComplete: true,
    removeOnFail: true,
  },
});

async function processParentJob(job, token) {
  let step = job.data.step;

  while (step !== 'finish') {
    switch (step) {
      case 'init': {
        const opts = { parent: { id: job.id, queue: job.queueQualifiedName } };
        await queue.add('child', {}, { ...opts, jobId: 'c-123' });
        await queue.add('child', { delay: 10000 }, { ...opts, jobId: 'c-456' });

        step = 'children';
        await job.updateData({ step });

        break;
      }

      case 'children': {
        const shouldWait = await job.moveToWaitingChildren(token);
        if (shouldWait) {
          throw new WaitingChildrenError();
        }

        step = 'finish';
        await job.updateData({ step });
        return;
      }
    }
  }
}

async function processChildJob(job, token) {
  await wait(500);

  if (job.data.delay) {
    // Without this, the next attempt will error when it tries to remove itself
    await job.updateData({ delay: 0 });

    await job.removeChildDependency();

    await job.moveToDelayed(Date.now() + job.data.delay, token);
    throw new DelayedError();
  }
}

const worker = new Worker(
  'foobar',
  async (job, token) => {
    switch (job.name) {
      case 'parent':
        await processParentJob(job, token);
        break;

      case 'child':
        await processChildJob(job, token);
        break;
    }
  },
  { connection },
);

worker.on('active', (job) =>
  log('%s active attempt %s', j(job), job.attemptsStarted),
);

worker.on('completed', (job) => log('%s completed', j(job)));
worker.on('failed', (job, err) => log('%s failed: %s', j(job), err));

async function main() {
  await queue.add('parent', { step: 'init' }, { jobId: 'parent-1' });
}

main();

const j = (job) => {
  const s = `${job.name} ${job.id}`;

  if (!job.parent) {
    return s;
  }

  return `${s} (p: ${job.parent.id})`;
};

const log = (s, ...args) => {
  const t = new Date().toISOString();
  console.log(`%s ${s}`, t, ...args);
};

const wait = (ms) => new Promise((r) => setTimeout(r, ms));

Relevant log output

2024-10-16T12:02:58.500Z parent parent-1 active attempt 1
2024-10-16T12:02:58.504Z child c-123 (p: parent-1) active attempt 1
2024-10-16T12:02:59.014Z child c-123 (p: parent-1) completed
2024-10-16T12:02:59.015Z child c-456 (p: parent-1) active attempt 1
2024-10-16T12:02:59.524Z parent parent-1 active attempt 2
2024-10-16T12:02:59.527Z parent parent-1 completed

# In this attempt, c-456 should not have a parent
2024-10-16T12:02:59.528Z child c-456 (p: parent-1) active attempt 2
2024-10-16T12:03:00.033Z child c-456 (p: parent-1) completed

Code of Conduct

  • I agree to follow this project's Code of Conduct
@victorandree victorandree added the bug Something isn't working label Oct 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant