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

SqlMainDomLock will stop listening if Sql Server connection terminates #9543

Merged
merged 1 commit into from
Dec 14, 2020

Conversation

Shazwazza
Copy link
Contributor

@Shazwazza Shazwazza commented Dec 14, 2020

NOTE: This PR is targeting the 8.6 branch

Related to #8398 which ensured that if any Azure Sql transient errors occurred that SqlMainDomLock would not stop listening. This didn't take into account exception that occur when trying to create a transaction which can occur if the Sql server cannot be connected to at all - for example if the service is stopped or paused or if the sql user can no longer connect.

If this unhandled exception occurs, MainDom stops listening and shuts itself down which is not what we want to happen. This extends the work from #8398 to prevent MainDom from shutting down if Sql server can no longer be reached. This in theory shouldn't happen but it is in some installs which is impacting sites.

Testing

Prior to this fix, do this:

  • ensure you are using Sql Server for your DB
  • enable sqlmaindomlock in appsettings <add key="Umbraco.Core.MainDom.Lock" value="SqlMainDomLock"/>
  • run with the debugger attached
  • put a breakpoint on MainDom.OnSignal
  • open sql server management, right click on your server node and select "Pause" to pause the service
  • your breakpoint will hit MainDom.OnSignal which means it will shutdown - this is unwanted behavior

With this fix applied

  • repeat the steps (ensure you restart the Sql server service first!) and you will not see MainDom.OnSignal hit
  • while you are still debugging, resume the Sql server service
  • open the ClientDependency.config and add some whitespace and save the file - this will restart the site since it's a .net config file - MainDom.OnSignal will be hit - this is expected behavior

You will see a huge amount of errors in the logs, one every 2 seconds while the SQL server is unreachable. This is expected.


This item has been added to our backlog AB#9730

@Shazwazza Shazwazza added the state/sprint-candidate We're trying to get this in a sprint at HQ in the next few weeks label Dec 14, 2020
Copy link
Member

@bergmania bergmania left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code changes looks fine.. I will test

@bergmania
Copy link
Member

bergmania commented Dec 14, 2020

Behaviour is as expected from the test cases 💪

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants