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]: Dragonfly wont add children jobs #2440

Open
1 task done
ivnnv opened this issue Feb 22, 2024 · 8 comments
Open
1 task done

[Bug]: Dragonfly wont add children jobs #2440

ivnnv opened this issue Feb 22, 2024 · 8 comments
Labels
bug Something isn't working

Comments

@ivnnv
Copy link

ivnnv commented Feb 22, 2024

Version

5.3.0

Platform

NodeJS

What happened?

It seems Dragonfly wont add child jobs on a FlowProcess if --default_lua_flags=allow-undeclared-keys flag is NOT set.

How to reproduce.

Run two Dragonfly instances, both with recommended --cluster_mode=emulated --lock_on_hashtags.

Instance A) runs with --default_lua_flags=allow-undeclared-keys
Instance B) runs with just the bullmq recommended

Create a flowProducer with a parent job with childrens (in my case, the parent job belongs to a different queue than the childs).

On instance A), the child jobs are added and processed.
On instance B) Just the parent is created, no childs are created.

Relevant log output

// THIS IS A SIMPLIFIED VERSION OF THE CODE JUST TO ILLUSTRATE THE FLOW CREATION
  // It DOES WORK on actual Redis and Dragonfly (A)

  const parentJobId = `${A}_${B}-parent`;

  const childJobs = REDACTED_ARRAY.map((ITEM) => {
    const jobId = `${A}_${}}-${ITEM.key}`;

    const { opts, data } = createJobData(ITEM);

    const childJob = new Job(childQueue, jobId, data, opts);
    childJob.parent = {
      id: parentJobId,
      queueKey: `{flows}`,
    };
    childJob.parentKey = `bull:{flows}:${parentJobId}`;

    return childJob;
  });

  const { opts, data } = Q.createJobData(ITEM);

  const parentJob = new Job(parentQueue, parentJobId, data, opts);

  const flowJob: T.bullmq.FlowJob = {
    ...parentJob,
    prefix: "bull",
    queueName: "{flows}",
    children: childJobs,
  };

  const flowProducer = new FlowProducer({ connection: C.REDIS_OPTIONS });
  await flowProducer.add(flowJob);

Code of Conduct

  • I agree to follow this project's Code of Conduct
@ivnnv ivnnv added the bug Something isn't working label Feb 22, 2024
@ivnnv
Copy link
Author

ivnnv commented Feb 22, 2024

Sorry I dont know if this would be a bug in BullMQ's Flow implementation that should be addressed here or Dragonfly repo.
Friendly tagging @chakaz if rings a bell on what could be the reason behind this weird behaviour.

@ivnnv ivnnv changed the title [Bug]: Dragonfly wont add children [Bug]: Dragonfly wont add children jobs Feb 22, 2024
@manast
Copy link
Contributor

manast commented Feb 22, 2024

@ivnnv we can look into the issue. The first thing I would like to ask is if you could clean up the code, you are using some patterns that are not what we recommend when using flows, such as creating job instances first and then using them in the flow producer. You could just follow the example here: https://docs.bullmq.io/guide/flows
Also, we have tests that covers this simple case and they all pass with Dragonfly with the recommended settings. It would also be useful if you check the dragonfly logs to see if some error has been produced.

@chakaz
Copy link

chakaz commented Feb 22, 2024

I know little (read: nothing) about BullMQ, so without actually knowing which operations are misbehaving in Dragonfly I can be of little help..

@ivnnv
Copy link
Author

ivnnv commented Feb 22, 2024

@ivnnv we can look into the issue.

Thanks!

Yes @manast, Ive tried several versions of this code to discard it was a code thing.
I started doing regular object for creating the jobs and then the flow, then injecting everything at once as a big object (doing exactly as you have in tests/test_flow.ts) and ended up with this last version. All versions of the code can reproduce the above scenario, I really wanted not to now open the issue until I was 100% sure about this.

@ivnnv
Copy link
Author

ivnnv commented Feb 28, 2024

hey @manast just a friendly ping wondering if you were able to have a look at this (Redis feels super slow after trying Dragonfly 😄)

@tomkuehl
Copy link

tomkuehl commented Mar 1, 2024

Hi, we are heavily using parent-child relations and running on Dragonfly without any issues. Dragonfly's docs states that you have to set --default_lua_flags=allow-undeclared-keys and that the hashtag used for queues have to be the same if they have a parent-child relation. So I would say that's a non-issue, or am I missing something?

https://www.dragonflydb.io/docs/integrations/bullmq

@ivnnv
Copy link
Author

ivnnv commented Mar 1, 2024

hey @tomkuehl, thanks for joining the convo.

I had a second look at that and yeah it seems they say that this is required (thought it was optional).
But @chakaz advised this will cause performance drawbacks and also it is stated in that same page that these other flags need to be turned on (--cluster_mode=emulated --lock_on_hashtags)

So Im hesitant now if ALL OF THESE FLAGS are required to run BullMQ with Dragonfly, and if so, I think this should be state more clearly in every doc, is this true @manast and if so, is there any plans to support parent-child relations to work without this flag?

And then the question is: if we need to put all of those flags to run Dragonfly, would we get any performance benefit after all?

@chakaz
Copy link

chakaz commented Mar 4, 2024

Dragonfly's docs states that you have to set --default_lua_flags=allow-undeclared-keys and that the hashtag used for queues have to be the same if they have a parent-child relation.

That's actually not what we (try to?) say :)
If you could rename your queues (or prefix) to have hashtags, and use the same hash tags for parents and children (but different hash tags for unrelated queues), you should definitely not use --default_lua_flags=allow-undeclared-keys, but instead use --cluster_mode=emulated --lock_on_hashtags
I'll ask internally to modify it since it seems like a source of confusion.

So I would say that's a non-issue, or am I missing something?

Running with --default_lua_flags=allow-undeclared-keys slows things down considerably, but there's no correctness issue with it.

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

4 participants