dotnet-labs / ServiceWorkerCronJob

Schedule Cron Jobs using HostedService in ASP.NET Core
MIT License
265 stars 71 forks source link

Support heavy lifting work #1

Closed mcshaz closed 4 years ago

mcshaz commented 4 years ago

First - thank you for putting this out there - really great.

Regarding the ScheduleJob method of the CronJobService class: If the DoWork method has a significant execution time, it might be worth disposing and freeing up the timer before commencing (I realise it is a minuscule system resource, it just 'feels' more correct to me). Similarly the DoWork task might be returning a completed task because it has detected the IsCancelationRequested change to true - is it worth checking before creating the next iteration?

    _timer.Elapsed += async (sender, args) =>
    {
        _timer.Dispose();
        _timer = null;
        if (!cancellationToken.IsCancellationRequested)
        {
            await DoWork(cancellationToken);
        }
        if (!cancellationToken.IsCancellationRequested)
        {
            await ScheduleJob(cancellationToken); // reschedule next
        }
    };
changhuixu commented 4 years ago

Hi @mcshaz , thank you for your suggestions and I totally agree with you. I have updated code to reflect this change. Thanks again. 🤝