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

Ideas for improvements to Cron #392

Closed
3 tasks done
bensheldon opened this issue Oct 1, 2021 · 3 comments
Closed
3 tasks done

Ideas for improvements to Cron #392

bensheldon opened this issue Oct 1, 2021 · 3 comments

Comments

@bensheldon
Copy link
Owner

bensheldon commented Oct 1, 2021

Some ideas for improving Cron:

  • Add a cron_at column, along with a uniqueness validation on cron_key, cron_at. This would push down to the database the largest source of enqueue concurrency conflicts, and provide a better guarantee (I don't know of anyone who is using intending Cron to enqueue multiple identical jobs from multiple workers at the same point in time)
  • Turn the Cron configuration into an ActiveModel object. E.g. a CronEntry class that has .all, .find(cron_key) methods, and instance accessors for the cron configuration values, and encapsulate the next_at and enqueue logic in methods too. I think that might dry up the code (or at least avoid primitive abuse as we're doing more and more with the cron configuration hashes), as well as allow us to use some niceties like dom_id instead of calling to_param on the strings in the Dashboard.
  • The args parameter is not bulletproof across different combinations of positional args and kwargs and Ruby versions. (I would love some help on this one).
@bensheldon bensheldon changed the title Improvements to Cron Ideas for improvements to Cron Oct 1, 2021
@bensheldon bensheldon added the hacktoberfest Issues that are good for Hacktoberfest participants label Oct 1, 2021
@bensheldon
Copy link
Owner Author

Noting that there is a way for the adapter to pass back errors on enqueue:

https://github.com/rails/rails/blob/826f947ce7078a66e93276c8102dd235bb629911/activejob/lib/active_job/enqueuing.rb#L74

Thinking about that in the context of adding unique index and handling failed uniqueness exceptions.

@ssendev
Copy link

ssendev commented Oct 21, 2021

Another idea is to have a Last run at column.

An option to disable Cron jobs from the web interface is also useful since it allows quick intervention when a job is misbehaving without taking down everything.

These two and being able to run Cron tasks immediately (as mentioned in #425) are available in sidekiq-cron

A minor nitpick is, at least for me Cron Job Name and Class is always the same and pretty long which means the table is needlessly cramped.

@bensheldon bensheldon removed the hacktoberfest Issues that are good for Hacktoberfest participants label Oct 22, 2021
@bensheldon
Copy link
Owner Author

@ssendev strong agree! Thank you for suggesting.

Disabling cron globally isn't easily done (yet... GoodJob doesn't have any global configuration in the database right now).

Adding the option to run a cron task immediately is very doable.

And thanks for the UI feedback. I'm thinking of just showing the Name, schedule, and then putting all the configuration params in an expandable toggle. That should free up some space.

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

No branches or pull requests

2 participants