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

Extended functionality for Workers to specify queue to run on, and to perform after a duration #759

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

isaacdonaldson
Copy link
Contributor

Loco workers is using sidekiq-rs but not making it easy for users to use some of the features. Aliasing the sidekiq::worker::WorkerOpts to loco_rs::worker::AppWorkerOpts allows the ability to implement fn opts()... for the loco_rs::worker::Worker trait, and enables users to specify (and use) a custom queue for their worker to run on. As well, support is added to use the perform_in(...) function that allows a worker to be run after a provided std::time::Duration has elapsed. Support for BackgroundQueue (through sidekiq-rs), ForegroundBlocking, and BackgroundAsync was added, but could possibly not be ideal (ForegroundBlocking does block 🤷‍♀️).

Caveats: The specified queue for the worker needs to be defined in the config.yml file.

@jondot
Copy link
Contributor

jondot commented Sep 22, 2024

Thanks, this looks good!
Can you explain a bit about the 64 seconds interval (e.g. why '4'?)

@isaacdonaldson
Copy link
Contributor Author

Thanks, this looks good! Can you explain a bit about the 64 seconds interval (e.g. why '4'?)

Yeah, don't know what I was thinking there 😅. Changed it to sleep the proper amount with the remaining time.

@jondot
Copy link
Contributor

jondot commented Oct 2, 2024

@isaacdonaldson thanks for this!
I'm almost finishing a rewrite of the queue subsystem:

  • Added postgres based queue
  • "seamless" interface to pick between redis (sidekiq) and postgres queue
  • ability to "compile out" irrelevant queue providers
  • only a single trait is now needed: BackgroundWorker

Custom queues: In the original sidekiq it was used mostly for ordering. You ordered the critical queues first:

- critical
- system_messages
- default

And sidekiq would iterate in-series over these, it would only pull from the next queue if the preceding one was empty -- creating a kind of a priority mechanism.

I'm wondering if you experience the same and/or if you have other uses for custom queues?

PS: every worker is defined as its own separate, statically typed job, so we do have separation between jobs, they just go into the same queue in redis (the same physical list: default)

@isaacdonaldson
Copy link
Contributor Author

@jondot - Is there somewhere where I can check out your queue implementation? Would love to take a look :)

I agree that a better defined trait like BackgroundWorker would be a good idea when choosing between Postgres, Redis, or tokio 👍 .

I don't think that extra/custom queues are a necessity, but I do think storing a unique 'queue label' with the job (in Postgres and Redis) could be a good idea for debugging/observability/human readability. That is what I was wanting to use them for, but it could also be optional.

I also think that having a priority mechanism is quite important. I think using the same queue (Redis list/Postgres table) could be fine, as long as there is the ability to specify priority.

I don't mind giving some feedback or expanding on what I mean once I can see the implementation :)

@jondot
Copy link
Contributor

jondot commented Oct 13, 2024

Hi @isaacdonaldson - the final implementation is now merged, I hope you can take a look at the new code -- see if there's anything to improve from this PR into the new implementation

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.

2 participants