We have moved to GitHub Issues
Created by Shannon Deminick 16 Oct 2017, 04:55:18 Updated by Sebastiaan Janssen 25 Oct 2017, 11:07:05
Subtask of: U4-9609
The Ping is used for a 'keep alive' but when load balancing having this on doesn't make sense for non-master schedule servers. The reason keep alive exists is so that the scheduling always takes place.
Ping should be turned off for non master schedule servers and this will alleviate any issues regarding the umbraco url and the server needing to make requests to itself.
This makes the keep alive/ping disabled for slave/front-end nodes when using load balancing. This also fixes up the changes made to the scheduled publishing task - it's not longer using async await so it shouldn't be overloading the PerformRunAsync also fixes up the usage of the obsolete DisposableTimer
Can you explain this part?
it's not longer using async await so it shouldn't be overloading the PerformRunAsync
I don't understand what the goal is currently. Otherwise, the code looks good. Need to set up a load balanced environment to test though.
@sebastiaan Yes i can :) Our Background task runner can run things in one of two ways: Async and non-async. The only reason you'd need to run something in async using
PerformRunAsync with our background task runner is if your code requires the usage of async/await. If your code does not have this requirement then non-async should be used with
Previously the scheduled runner required the use of async/await because it used to use HttpClient to make a request to itself which would use async/await. When we removed this functionality and replaced scheduling with running on a background thread we no longer require any usage of async/await. Resharper was also giving us warnings saying - hey there's an 'async' definition in this method but there is no 'await' being used.
So basically, now that the scheduler runs things on a background thread and the APIs it's using don't have any async/await requirements, it should run the background task on non-async with
PerformRun instead of
Understood and that's a great reply, thanks, makes all sense!
I've set up a load balanced environment and ran this for a while, then mixed it up, turned off the master site, waited a while to make sure everything would switch up and the other site turns into master.
All good and all merged! We might want a 7.6 with this in it too. Added backport tag.
Yup this can be backported along with http://issues.umbraco.org/issue/U4-10503
Merged into 7.6, which also required merging in U4-10121 Scheduled publishing as a background task - which is a great thing! :-)
Backwards Compatible: True
Due in version: 7.7.5, 7.6.12
Sprint: Sprint 70
Story Points: 0.5