nestjs / bull

Bull module for Nest framework (node.js) :cow:
https://nestjs.com
MIT License
604 stars 99 forks source link

Switch from Bull to Bree due to core issues + leaks + user complexity #496

Closed niftylettuce closed 3 years ago

niftylettuce commented 4 years ago

Hello - I'm just sharing a package I created, which was recently featured on Node Weekly https://nodeweekly.com/issues/347 and has 100% code coverage. I built it out of frustration with core bugs in Bull, even after having created my own wrapper https://github.com/ladjs/bull. The author of Bull has blocked me from filing further issues and bugs I have discovered to say the least.

You can read more about it here https://jobscheduler.net/ or directly on GitHub at https://github.com/breejs/bree.

Would love to see Bree used in nestjs!

BrunnerLivio commented 4 years ago

I don’t wanna judge this recommendation, since I don’t feel like I have the expertise. Though I want to extend @niftylettuce issue, since important points we should consider in order to make a decision:

From breejs Readme:

Digging through the Bull repo; @niftylettuce really tried to do an effort (see his PRs) to fix the mentioned issues & push them back upstream, but was unable to due to technical conflict with the Bull mainainter(s).

EDIT: Disclaimer; I tried to to objectively summarize this topic what I found interesting, though better if you make a picture yourself about it.

gaurav- commented 4 years ago

Bree looks very interesting based on a quick read of its documentation. I'd vote for a separate nestjs/bree package so that the community can have alternatives based on the specific needs of different projects

andreialecu commented 4 years ago

I was just looking into Bree but I don't see how it can be really used with NestJS and TypeScript in particular.

It seems that jobs are supposed to go into separate .js files. That's not really idiomatic NestJS as you can't use dependency injection that way, or it would really overcomplicate things.

What Bree does seems to mostly be the same as https://docs.nestjs.com/techniques/task-scheduling

gaurav- commented 4 years ago

It seems that jobs are supposed to go into separate .js files. That's not really idiomatic NestJS as you can't use dependency injection that way, or it would really overcomplicate things.

I guess it could be similar to how the TypeORM module allows specifying path glob for its entities.


export const databaseProviders = [
  {
    provide: 'DATABASE_CONNECTION',
    useFactory: async () => await createConnection({
      ...
      entities: [
          __dirname + '/../**/*.entity{.ts,.js}',
      ],
      ...
    }),
  },
];

Another example is of the GraphQLModule which allows configuring path glob for schema files.

GraphQLModule.forRoot({
  typePaths: ['./**/*.graphql'],
  ...
}),
niftylettuce commented 4 years ago

@andreialecu just note that @nestjs/schedule uses node-cron which I am now a maintainer of. I do not recommend using this. Not only does bree have cron expression validation support, it also has support for seconds format (6 instead of 5 space delimited expressions), and it spawns workers in sandboxed processes. It is extremely bad practice to have a shared process with multiple jobs running on it at once, especially when developers forget to wrap their promises with try/catch blocks as I've seen time and time again.

andreialecu commented 4 years ago

@gaurav- TypeORM entities contain no code other than decorated properties though. So there is no need for dependency injection. Same for *.graphql files.

@niftylettuce I agree that Bree solves some problems. But it's not necessarily for everyone. Seems more of a node-cron replacement than a full fledged job queue.

I personally prefer to run my job runners in a cluster across multiple docker containers, for high availability. Seems like Bree isn't fit for that out of the box. I don't see any built-in distributed job locking support, or any sort of persistence mechanism (users need to write their own). I feel there's a much steeper learning curve to this approach.

The main concern is: I'm not sure how you envision Bree to work with TypeScript in general, or even Babel, considering the need for files to be compiled. Other things like https://nx.dev/ complicate this approach even more. And how about Deno?

As per your description it seems that Bree is more of a node-cron replacement, requiring many things to be coded manually. Note that I do like this approach, and I've been considering using it instead of Bull (probably @nestjs/schedule though) in my future projects, because I feel I get more efficiency and control that way.

I'm sure at some point someone will create a nestjs-bree package as an alternative to @nestjs/schedule. Here's an example for Agenda: https://www.npmjs.com/package/nestjs-agenda

It just feels that Bree is not quite fit yet for the TypeScript world in general, and I think that NestJS is one of the flagship projects of TypeScript, along with Angular.

niftylettuce commented 4 years ago

The core problem that many fail to see nor recognize are that there are core bugs in Bull that are still unresolved. For example, sometimes you will make repeatable jobs and they don't start. I'm not the only one that has reproduced this either. Just look at all the 300+ GitHub issues filed for Bull. People are in general, confused as to how it works, and also quite frequently complain about its slowness or uncertainty as to when jobs will start/repeat. Relying on Redis for a job scheduler is horrible practice, at the very least go back to Agenda for persistent storage, but even I don't recommend that as I'm a former core maintainer of Agenda and financially contributed out of my pocket towards its development. I also am strongly against TypeScript, but if someone wants to maintain TypeScript in Bree (and seriously will maintain it without my help) then I am open to that for sure.

JVMartin commented 4 years ago

"I also am strongly against TypeScript" (@niftylettuce)

Anyone who is strongly against static typing obviously doesn't understand it. Having a type system is the only way to use AST's to statically refactor projects at-scale, otherwise it would take countless hours to perform basic refactors, causing your project to calcify.

kamilmysliwiec commented 3 years ago

@nestjs/bull is a wrapper package that aims to simplify the integration process of NestJS applications with Bull which is widely used and very popular in the community. However, it doesn't mean that other existing packages for creating & managing Queues/Jobs shouldn't be used. Developers can pick & choose whatever they want for their projects, that's one of the most important goals of Nest.

As for switching from bull to bree - as the name suggests, this package is a wrapper for Bull (@nestjs/bull). There are no plans to create an official wrapper for Bree ATM but we encourage anyone in the community to create a wrapper on your own and share with the community if you think that other people may find it valuable.