U4-10538 - Disable Ping on front-end nodes when load balancing

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.

Comments

Shannon Deminick 16 Oct 2017, 05:29:58

PR https://github.com/umbraco/Umbraco-CMS/pull/2242

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


Sebastiaan Janssen 18 Oct 2017, 18:08:28

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.


Shannon Deminick 19 Oct 2017, 04:28:14

@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 PerformRun instead.

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 PerformRunAsync


Sebastiaan Janssen 24 Oct 2017, 15:28:24

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.


Shannon Deminick 24 Oct 2017, 22:29:11

Yup this can be backported along with http://issues.umbraco.org/issue/U4-10503


Sebastiaan Janssen 25 Oct 2017, 11:07:05

Merged into 7.6, which also required merging in U4-10121 Scheduled publishing as a background task - which is a great thing! :-)

https://github.com/umbraco/Umbraco-CMS/commit/e84a963d668efa75a2b0597004764536f776175d https://github.com/umbraco/Umbraco-CMS/commit/7cbaecc53a2ef9f185806e5c2f235dc2a6005be0


Priority: Normal

Type: Bug

State: Fixed

Assignee:

Difficulty: Normal

Category:

Backwards Compatible: True

Fix Submitted:

Affected versions:

Due in version: 7.7.5, 7.6.12

Sprint: Sprint 70

Story Points: 0.5

Cycle: 5