jamesmh / coravel

Near-zero config .NET library that makes advanced application features like Task Scheduling, Caching, Queuing, Event Broadcasting, and more a breeze!
https://docs.coravel.net/Installation/
MIT License
3.88k stars 256 forks source link

Question: Is there any way to run an invocable on demand while respecting PreventOverlapping? #96

Open oltronix opened 5 years ago

oltronix commented 5 years ago

So lets say we have a moderately heavy job that imports something to our db every 20 minutes. Occasionally we are asked to run this manually because the 20 minutes wait is just to long, using Coravel we could then trigger this using the Queue, but if we're unlucky this would coincide with the scheduled run every twenty minutes, which would cause some issues in legacy exports we have. Can this overlap be detected and avoided?

As an alternative it would be nice if we could schedule a one off run of an invocable, like scheduler.Schedule().Once(); That way it could take the overlappingprotection into account and share the same thread as the regular execution.

jamesmh commented 5 years ago

The "prevent overlap" feature only applies to jobs that are run within the scheduler.

Adding a ".Once()" method to the scheduler could work. But that means having to deploy the application whenever you want to run it? That doesn't sound like a great solution.

I'm thinking that introducing a new construct altogether would be better. Something like a new interface:

interface IPreventOverlap
{
     string PreventOverlapKey() { get; }
}

Implemented by an invocable, it could be used to simply tell that specific invocable that it will always prevent overlap (no need to specify when scheduling it either).

If queued, it would try to get a lock etc. (in the same way described below).

A potential fix (for now if it's critical...) is to manually try to lock the job using the same Mutex that the scheduler uses. You would inject IMutex from the DI container and call bool TryGetLock(string key, int timeoutMinutes) and void Release(string key)

// Wherever you are manually launching the job from...

IMutex mutex = this._mutex; // From DI
int timeoutInMinutes = 60; // This will release the key after 60 min in the event something bad happens.
string overlapKey = "MyPreventOverlapKey"; // Same key used in ".PreventOverlap()"
bool available = mutex.TryGetLock(overlapKey, timeoutInMinutes);

if(available)
{
    try 
    {
        await new MyInvocable().Invoke(); // run the job
    }
    finally
    {
        mutex.Release(overlapKey);
    }
}

For reference, in the scheduler, this logic is here

That being said, relying on the internals of Coravel is not a best practice lol.

I like the interface options as a potential new feature πŸ˜…

oltronix commented 5 years ago

Adding the overlap protection as an interface sounds like a good way of solving it! I'm not in a panic, was looking at building a PoC for replacing our current background workers and this looks like a pretty good fit so farπŸ‘

About the Once() suggestion, you can change the schedule at runtime tough? I just tested injecting a IScheduler into a api controller and adding to the schedule in one of its actions. Seemed to work pretty well, except it leaves a recurring thing where I just wanted to trigger it once. :)

So if I could add it as scheduled once and it would just be removed from the schedule after being executed that should work pretty well. And as a bonus there could be an overload taking a DateTime, for scheduling a one off job in the future.

Thanks for the quick feedback!

jamesmh commented 5 years ago

Nice! I like that idea. Similar some of the stuff hangfire offers.

Now that you lay out that scenario, I like the idea of a Once() method...

The only issues I can think of with being able to schedule something in the future is that, right now, schedules are not persisted. So, if you were to schedule something for, let's say, an hour from now. If your app restarts / is redeployed, that scheduled job will disappear.

I suppose for short term jobs that OK... Any thoughts?

oltronix commented 5 years ago

Yeah, should keep that as a disclaimer, I saw persistence was part of the plus project, so might not be on the road map for this version. But a motivated user could still persist these DateTimes somewhere and restore them when the application starts again. And for us the running it once on demand, without having to worry about the same thing running in parallel, is the biggest thing anyway. This just seems like a good extension point for scheduling one off jobs in the future.

jamesmh commented 4 years ago

To recap, there are two concerns here:

Ex:

scheduler.EveryHour().Once();

Related, a way to schedule once but using a duration:

scheduler.In(DateTime.FromMinutes(5))

jamesmh commented 4 years ago

@oltronix it's been a while on this issue... Curious if this is still needed?

If so, wondering if using the queue instead would work. Something like QueueIn(TimeSpan time);?

Or were you thinking that you need an exact time in the future -> like May 19th, 2020?

oltronix commented 4 years ago

Sorry for my inactivity, the prototype I was working on was put on ice and I forgot to respond here.

What I ended up doing was to inject the mutex as you suggested and it seemed to work fine.

Just to answer you questions: For us the main concern was the need to be able to manually trigger a job that respected the restriction of only running one instance of the invocable at a time. Since the queue and the scheduler are separate I felt going with the scheduler was an good idea since that would give the 'schedule one run in the future'-feature more or less for free.

jamesmh commented 4 years ago

Great, thanks for the feedback πŸ‘

renevdhoek commented 3 years ago

Hi James, I am also looking for a solution to run an invocable on demand while respecting PreventOverlapping. Can you give some insight on how it is implemented in Coravel Pro? And maybe you have new information on the 'schedule one run in the future'-feature

jamesmh commented 3 years ago

Hey,

Right now you would have to use the Mutex explicitly as mentioned above. I've got another feature on the go that's similar to this one so after that one this might be a good next step to address the two issues around (a) running a one-off invocable and (b) making sure that one off can prevent overlap too.

I'll ping this thread once progress is made πŸ‘

nicoletacristian commented 2 years ago

I also need this feature. Meanwhile, I'm going to implement the Mutex version.

mnj commented 1 year ago

Same here, it would be nice, if we could schedule a "one of" run of a scheduled task, let's say something runs daily, and we need to rerun something during the day in special cases, it would be nice to be able to just trigger the task, and prevent it from overlapping.

jamesmh commented 1 year ago

Implemented Once() functionality. See docs.

This contained a refactoring that will help with setting up queue and scheduler linked overlap prevention.