Skip to content

[LEA-5114] Fixing the issue with using incorrect queue#5

Merged
supersujit merged 1 commit intomasterfrom
sj/LEA-5114-fixing-incorrect-queue-issue
Sep 22, 2025
Merged

[LEA-5114] Fixing the issue with using incorrect queue#5
supersujit merged 1 commit intomasterfrom
sj/LEA-5114-fixing-incorrect-queue-issue

Conversation

@supersujit
Copy link
Contributor

Issue:
https://intellum.atlassian.net/browse/LEA-5114

This issue was identified because default queue were flooded and slowed, and there were high priority jobs which got enqueued on default queue. After investigation we found that the issue is with this gem because its not respecting the queue name passed in parameter.
This PR tries to fix this issue by correctly setting the queue name using set(queue:) method.

…correctly enqueuing job to the correct queue.
@supersujit supersujit requested review from graimon and saxxi September 19, 2025 10:23
Copy link

@graimon graimon left a comment

Choose a reason for hiding this comment

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

great job!

@graimon
Copy link

graimon commented Sep 19, 2025

I've added some reviewers, as this issue is somehow related to the queuing issues we've been experiencing lately, to inform them.

@supersujit changes seem good, you don't need to wait too much.

@zpatten, regarding your work on migrating all Sidekiq Workers into ActiveJobs, this gem should be changed as well.

Copy link

@k-rudy k-rudy left a comment

Choose a reason for hiding this comment

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

🚢

@zpatten
Copy link

zpatten commented Sep 19, 2025

@zpatten, regarding your work on migrating all Sidekiq Workers into ActiveJobs, this gem should be changed as well.

Appreciate the heads up! 🙂

Copy link

@zpatten zpatten left a comment

Choose a reason for hiding this comment

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

LGTM; only one note, this is calling active job methods on the class so I'm assuming the class is a model in all cases?; if that's invoked against a job class which isn't inherited from active job (ie standard sidekiq class) I don't think it will work as intended. I can do some digging if you want to verify and I'm not 100% but I figured it was worth mentioning.

@zpatten
Copy link

zpatten commented Sep 19, 2025

One last note, if it is invoked against the job class it should be easy to do an if/else on its base class and enqueue accordingly depending on if it's active job or og sidekiq.

@supersujit
Copy link
Contributor Author

Thanks @zpatten for the review 👍
This gem is supposed to be used only with active record models. So we are good :)

@supersujit supersujit merged commit f5f0741 into master Sep 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants