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

Allow running crons in worker threads #289

Open
franky47 opened this issue Dec 7, 2021 · 7 comments
Open

Allow running crons in worker threads #289

franky47 opened this issue Dec 7, 2021 · 7 comments
Labels
enhancement New feature or request

Comments

@franky47
Copy link
Member

franky47 commented Dec 7, 2021

This could make it tricky to implement such cron tasks, as worker threads have a different execution context and may not do as much as main-threaded tasks, but it would be a nice option for CPU-intensive tasks.

Documentation

https://nodejs.org/api/worker_threads.html

Packages & Third-Party Solutions

@franky47 franky47 added the enhancement New feature or request label Dec 7, 2021
@phr3nzy
Copy link

phr3nzy commented Aug 3, 2022

I can work on this

@franky47
Copy link
Member Author

franky47 commented Aug 3, 2022

Great! How would you go about it?

@phr3nzy
Copy link

phr3nzy commented Aug 4, 2022

I'm more in favor of using threads.js than the other 3 purely because of its (subjectively) straightforward APIs.

We can add a flag like useBackgroundWorkers that will only accept onTick functions that are enclosed in a threads.js expose function and then through the Worker API from threads.js execute them. Unfortunately, that means each function enclosed in an expose will have to be on a separate file, but other then that there's no issue.

We can either install threads.js manually, or we can have it as a registered fastify plugin that we require whenever the useBackgroundWorkers flag is true.

@franky47
Copy link
Member Author

franky47 commented Aug 5, 2022

I'm fine with workers having to be declared in separate files, it's the same for web workers.

Is the flag necessary? Could we detect somehow that the onTick function is wrapped automatically? That would just be a nice-to-have though, I'm also happy with a flag.

Regarding dependencies, I'd rather leave threads.js as a peer dependency (ie: not bundle it). I'm not sure it needs to go through the Fastify plugin wrapping, though I guess it depends if/how consumers of this package already use threads.js.

@phr3nzy
Copy link

phr3nzy commented Aug 8, 2022

I guess a workaround we can do is have them give the path of the file that houses the function that must be executed on a worker_thread, detect which lib they have installed (of the few we support) and then run it.

Some implementations might have differences in how they spawn a worker, pass data and shutdown a working thread so we might have to create some sort of interface that abstracts whatever lib is detected.

@franky47
Copy link
Member Author

franky47 commented Aug 9, 2022

Up to you if you want to support multiple multithreading libraries, but I like the idea of a common interface (and have separate packages that implement them, so we don't need peer dependencies).

Regarding data transfer, I'd wager to only do a one way main thread -> worker thread when we pass the Fastify instance, as we do as the argument of the cron callback in the single-threaded version. Not sure how the multithreading would impact references and things changing on the main thread (eg: changing the value of a property on the Fastify instance in a worker thread while there are things reading the value on request handlers).

@phr3nzy
Copy link

phr3nzy commented Aug 20, 2022

Apologies for the late reply.

Sounds good, will fork it and push a PR when ready :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants