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

#3667 - Bull Scheduler - Ensure Delayed Job #3995

Merged

Conversation

andrewsignori-aot
Copy link
Collaborator

Moving the latest changes from Bull Scheduler to release 2.1.

…y promoted (Ensure Next Delayed Job) (#3978)

- Ensures a scheduled job will have a delayed job created with the next
expected scheduled time based on the configured cron expression.
- DB row used as a lock to ensure the initialization code will not
recreate delayed jobs or create them in an unwanted way if two PODs
start at the same time. To be clear, the lock approach is the same used
in other areas of the application and locks a single row in the table
queue-consumers, please not, the table will not be locked. Please see
below the sample query executed.
```sql
START TRANSACTION
select
	"QueueConfiguration"."id" AS "QueueConfiguration_id"
from
	"sims"."queue_configurations" "QueueConfiguration"
where
	(("QueueConfiguration"."queue_name" = 'archive-applications')) LIMIT 1 FOR UPDATE 
COMMIT
```
- While the queue is paused, no other delayed jobs will be created.
To proceed with the investigation about the intermittent ioredis issue
on DEV, the package was updated and more logs were added to try to
narrow down the root cause of the issue.
During the prior investigation was detected that when all the schedulers
try to execute redis operations at the same time it fails after a
certain amount of concurrent connections.
The possible solutions being investigated are:
1. Allow the connection sharing as mentioned here
https://github.com/OptimalBits/bull/blob/develop/PATTERNS.md#reusing-redis-connections
2. Control the service initialization to prevent multiple concurrent
connections.

This is PR an attempt to test the first options.
Implementing the easy/fast (and stable) approach to resolve the queue
initialization issues.

During the previous approach, sharing the ioredis connections (change
being reverted in this PR) worked but there was also a possible
false-positive memory leak warning. Further investigation will be needed
if we change this approach in the feature but for now, keeping the
existing approach.

The current solution extends the current "specific queue-based" lock to
an "all queues-based" lock.
The initialization is still pretty fast (around 1 second) and even if we
double the number of schedulers in the future it still will be good
enough (the code is executed once during queue-consumers
initialization).

_Notes_: The logs before mentioned the queue-name every time and now it
has changed to only the first one. The queue-name should be in the log
context also but many schedulers are not "overriding" it, which should
be resolved in the schedulers.
@andrewsignori-aot andrewsignori-aot added Queue Consumers Branch Sync-up Branch features move between main/release/hotfix branches. labels Nov 25, 2024
@andrewsignori-aot andrewsignori-aot self-assigned this Nov 25, 2024
Copy link

Backend Unit Tests Coverage Report

Totals Coverage
Statements: 22.03% ( 3718 / 16876 )
Methods: 10.17% ( 214 / 2105 )
Lines: 25.36% ( 3226 / 12720 )
Branches: 13.55% ( 278 / 2051 )

Copy link

E2E Workflow Workers Coverage Report

Totals Coverage
Statements: 65.43% ( 583 / 891 )
Methods: 59.26% ( 64 / 108 )
Lines: 68.54% ( 464 / 677 )
Branches: 51.89% ( 55 / 106 )

Copy link

E2E Queue Consumers Coverage Report

Totals Coverage
Statements: 84.39% ( 1238 / 1467 )
Methods: 84.51% ( 120 / 142 )
Lines: 85.61% ( 1053 / 1230 )
Branches: 68.42% ( 65 / 95 )

Copy link

E2E SIMS API Coverage Report

Totals Coverage
Statements: 66.94% ( 5774 / 8625 )
Methods: 64.8% ( 718 / 1108 )
Lines: 70.87% ( 4531 / 6393 )
Branches: 46.71% ( 525 / 1124 )

@andrewsignori-aot andrewsignori-aot marked this pull request as ready for review November 25, 2024 18:13
Copy link
Collaborator

@guru-aot guru-aot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dheepak-aot dheepak-aot self-requested a review November 25, 2024 20:00
Copy link
Collaborator

@dheepak-aot dheepak-aot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@andrewsignori-aot andrewsignori-aot merged commit d81f0f4 into release/v2.1.0 Nov 25, 2024
20 checks passed
@andrewsignori-aot andrewsignori-aot deleted the feature/#3667-bull-scheduler-delayed-job branch December 4, 2024 20:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Branch Sync-up Branch features move between main/release/hotfix branches. Queue Consumers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants