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

Parent job does not execute when sharing dependency with another job #1021

Open
bilalshaikh42 opened this issue Jan 25, 2022 · 15 comments
Open

Comments

@bilalshaikh42
Copy link
Contributor

bilalshaikh42 commented Jan 25, 2022

We have a series of processing steps linked to particular Id. Some of these steps depends on others, and some dependencies are shared. We have defined a flow as follows:

const runId =SomeSharedID
    
const JobA = {
      name: 'A',
      queueName: QueueA,
      opts: {
        jobId: `A-${runId}`,
      },
      data: {
        runId: runId,
      },
    };

    const JobB = {
      name: 'B',
      queueName: QueueB,
      opts: {
        jobId: `B-${runId}`,
      },
      data: {
        runId: runId,
      },
    };

    const JobC = {
      name: 'C',
      queueName: QueueC,
      data: {
        runId: runId,
      },
      opts: {
        jobId: `C-${runId}`,
      },
// Needs A and B
      children: [JobA, JobB]
    };

    const JobD = {
      name: 'D',
      queueName: QueueD,
      data: {
        runId: runId
      },
      opts: {
        jobId: `D-${job.id}`,
      },
      // Also needs  A and B
      children: [JobA, JobB],
    };
    const FlowC = await flowProducer.add(JobC);
    const FlowD = await flowProducer.add(JobD);

JobA and JobB are manually given ids since they should not be repeated for multiple dependent parents

In this situation, since both JobC and JobD share two jobs that have the same id as children, only the first added one will be triggered.
Is JobD not being triggered an intentional behavior? Or is this a bug?

@roggervalf
Copy link
Collaborator

hi @bilalshaikh42, thanks for submitting this issue, so currently one job only can have 1 parent

@ximus
Copy link

ximus commented Mar 20, 2022

multiple parents would be great!

I have an app flow where I need to generate a PDF to s3, then notify the client via an SMS and an email.

All those steps should ideally remain loosely couple pieces of code, not reference each other directly.

Ideal I would have:

+--> Generate PDF (child job)
      +--> SMS (parent job)
      +--> Email PDF (parent job)

I guess I can link them in series with the current abilities of bullmq:

+--> Generate PDF (child job)
      +--> SMS (child and parent job)
            +--> Email PDF (parent job)

The downside here is
a) some lost efficiency: the latter two jobs could run concurrently
b) reduced resiliency: if sms job fails, then email job doesn't run, which is too bad

or for now I'll couple the email and SMS jobs into one job, losing some granularity and code separation.
or maybe there's a solution using events? listen for PDF complete and queue the two latter jobs then. Will investigate ...

Thanks for BullMQ, a solid contribution to the node world

@ShadowStrikerJ
Copy link

Is there any update regarding this?

From the testing I did, it seems like if you put the full flow in at the same time one of those parent jobs just won't run.

However, if you do it a second time ( after the child jobs have completed ), the parent jobs recognize their dependencies are completed just fine and both parent jobs run.

Is this something that should be happening? Its also worth noting that in the first case it just fails silently, which probably isn't intended behavior.

@manast
Copy link
Contributor

manast commented Sep 9, 2022

@ShadowStrikerJ Unfortunatelly flows do not support multiple parents, and I do not know if and/or when they will be supported.

@imperfect-circuits
Copy link
Contributor

The alternative (which we use) is to create the two new jobs within one parent job, then they can processed indepedently. E.g.

parent: jobParent
children: jobA, jobB

jobParent() {
   create jobC
   create jobD
}

@pbell23
Copy link

pbell23 commented Jul 23, 2023

@imperfect-circuits This solution looks interesting but it adds complexity. If the jobC succeeds but not the jobD it will be necessary to make fail jobParent and then it will be necessary to add code to reexecute jobD and not jobC. Also this assumes that jobC and jobD have no children as they are no longer part of the flow tree as soon as they are manually created by jobParent.

@matpen
Copy link

matpen commented Oct 14, 2023

@ShadowStrikerJ Unfortunatelly flows do not support multiple parents, and I do not know if and/or when they will be supported.

@manast could you expand on whether this limitation is due to specific technical reasons, or simply not implemented yet? Maybe someone in the community might want to pitch in!

@manast
Copy link
Contributor

manast commented Oct 14, 2023

It increases complexity and edge cases maybe an order of magnitude, which use case is so important that will require this functionality?

@matpen
Copy link

matpen commented Oct 14, 2023

which use case is so important that will require this functionality?

Every workflow with multiple dependencies will need this functionality. In addition to the example in the OP, I will expand the example from the documentation.

Currently the example looks like this:

const flow = await flowProducer.add({
  name: 'renovate-interior',
  queueName: 'renovate',
  children: [
    { name: 'paint', data: { place: 'ceiling' }, queueName: 'steps' },
    { name: 'paint', data: { place: 'walls' }, queueName: 'steps' },
    { name: 'fix', data: { place: 'floor' }, queueName: 'steps' },
  ],
});

But suppose that, more realistically, painting the ceiling and the walls can only be approached after the floor is fixed. In this case, both paint jobs will depend on the floor fix, which in turn should have two parents.

The dependency graph now looks as follows:
image

I see no way to implement this situation, apart from:

  • topologically sorting the graph, thereby loosing concurrency (the two paint jobs can no longer be run in parallel)
  • coming up with a custom solution based on the flow pattern

@manast
Copy link
Contributor

manast commented Oct 14, 2023

Yeah, I am aware of the theoretical cases, I mean a real-world case where this is needed and no simple workaround can be found. For instance, in your example above you could just have one queue for fixing the floor, and in the worker for said queue you can add the rest of the flow as the last step before returning.

@matpen
Copy link

matpen commented Oct 14, 2023

@manast The case I described was a simple one just as a basis for discussion. Of course, simple workarounds can be found for this and other cases: the workaround you suggest is basically based on the flow pattern, as mentioned above.

It does have, however, the problem that it does not scale well: with a more involved dependency graph, where there are multiple nodes at every level, one must resort to topological sorting. And this will prevent the flow from maximizing concurrency. To visualize this, please consider the following image:
image

By using the flow pattern here, it is evident that we are loosing efficiency: paint_stairs could run as soon as fix_stairs is finished, but has to wait for fix_floor so that it can be run together with the other two paint tasks.

Please understand that here I am just trying to make the point for a genuine use case, not a "theoretical scenario". Whether such a case emerges often enough to justify the implementation effort, I would prefer to leave to the judgement of the community. I just thought to mention this, because I found many requests for this feature:

That said, thank you for working on a great library!

@zaquas77
Copy link

zaquas77 commented Jan 19, 2024

Hi @manast and @matpen I have same problems.

In my situation I've a tools where the users define jobs and theirs depends. I like run, when possibile, in parallels job.

Thanks again for the great work!

@zaquas77
Copy link

zaquas77 commented Feb 25, 2024

Hi @manast sorry for my insistence, but have you got some idea about this request? Could I help you in some way?

Thanks to @hhopkins95 for his discussion

thanks again.

@x-etienne
Copy link

+1

1 similar comment
@chingiz19
Copy link

+1

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

No branches or pull requests