-
Notifications
You must be signed in to change notification settings - Fork 518
chore: refactor lock management into separate class #3398
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
base: master
Are you sure you want to change the base?
Conversation
…eat/dedicated-thread-lock-extension
…eat/dedicated-thread-lock-extension
| ### Bug Fixes | ||
|
|
||
| * **deps:** upgrade bullmq to 1.80.4 ([#46](https://github.com/taskforcesh/bullmq-pro/issues/46)) ([fc2818f](https://github.com/taskforcesh/bullmq-pro/commit/fc2818f09ddd273b7cc16c54fcd9650e1d456d04)) | ||
| - **deps:** upgrade bullmq to 1.80.4 ([#46](https://github.com/taskforcesh/bullmq-pro/issues/46)) ([fc2818f](https://github.com/taskforcesh/bullmq-pro/commit/fc2818f09ddd273b7cc16c54fcd9650e1d456d04)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no need to apply lint in changelog files, need to restore format
| try { | ||
| await this.checkConnectionError(() => this.moveStalledJobsToWait()); | ||
| } catch (err) { | ||
| this.emit('error', <Error>err); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no need for this extra wrapper of try catch as checkConnectionError handles this same emission
| } | ||
|
|
||
| await this.startStalledCheckTimer(); | ||
| this.initializeLockManager(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not see useDedicatedLockThread option being used. It's referenced in docs but I cannot see it in lock-manager class or in worker
…eat/dedicated-thread-lock-extension
Why
This PR aims to provide a new option when instantiating Workers, so that it is possible to use a dedicated thread for lock extension and job stalling detection. The reason for allowing this, is so that we can have a safer lock renewal mechanism even in the cases where the jobs are not respecting the best practice of not keeping the event loop busy for long periods of time.
How
We refactor the lock handling in a separate class "LockManager" and then we add support to this class to run in a separate thread that communicates with the main process. Note that this approach is not a fail proof solution to the issue of jobs that keep the event loop busy, as it could also happen that the communication between the main thread and the lock extension thread stalls and issues related to locks may still happen, however it should reduce the number of cases when this happens.
Additional Notes (Optional)
Any extra info here.