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

V10 load balanced #44

Merged
merged 10 commits into from
Aug 30, 2024
Merged

V10 load balanced #44

merged 10 commits into from
Aug 30, 2024

Conversation

craigagnew
Copy link

@craigagnew craigagnew commented Aug 12, 2024

Resolves #33

Summary

This PR adds support for load balanced environments. It proposes the introduction of a few ideas:

  • An IStorageProvider in the service code to access the appropriate place to read/write settings
  • An implementation of it - DatabaseStorageProvider for use in load balanced setups
  • A custom notification to broadcast when settings change (useful for clearing cache or sending emails)

Testing

How to test?
Add the following to appsettings:

  "MaintenanceMode": {
    "StorageMode": "Database"
  }

This will trigger the switch into database storage mode, observe that settings are written to and read from the database.

Note that without the app setting above, the code will attempt to determine whether Umbraco is in a load balanced setup and behave accordingly.

Also @KevinJump sorry it looks like a lot of files changed but there are a lot of one-liners in the notification handlers :)

@KevinJump
Copy link
Member

Thanks, this looks great, we will take a look this week, and see if we can get it merged and released 👍

craigagnew and others added 4 commits August 12, 2024 20:43
fixed moving  and recycling to check for IsContentFrozen instead of IsInMaintenanceMode
makes sure code works on first boot before migration is ran to create table
@henryjump
Copy link
Collaborator

Hi

added a couple of changes

  • Changed loadbalance detection to use the server role
  • fixed a couple notifications that were checking for maintenance instead of content freeze
  • added some extra checks so the setting works on a fresh install before database tables have been creates

we think checking the server role to see if umbraco thinks it's running loadbalanced is more reliable than using the config settings, but if you think that's not right, please let us know.

@craigagnew
Copy link
Author

Hi @henryjump

Using server role totally makes sense, I was looking for something that did exactly what you've got but couldn't find it so thanks!

And makes sense on the DB check to reduce the warnings 😄

@craigagnew
Copy link
Author

Anything else I can contribute for this one @henryjump @KevinJump?

@henryjump
Copy link
Collaborator

we wanted to have a cache for the database lookups so the database doesn't get hit for every request

so this version has a _lastChecked value so that we only hit the database every n seconds.

the CacheSeconds option sets how many seconds it waits, default is 30

    "MaintenanceMode": {
        "StorageMode": "Database",
        "CacheSeconds": 30
    }

on a local sever changes to and from maintenance mode happens instantly, but on remote servers it may take up to CacheSeconds

Umbraco does this for the cache on load balancing, but it checks every five seconds so maybe we could have the value lower

@craigagnew
Copy link
Author

Good idea @henryjump.

I wonder, to make it clear it's only for use in Database storage mode if we should name the setting WaitTimeBetweenDatabaseCalls (riffing off Umbraco's DatabaseServerRegistrar.WaitTimeBetweenCalls here).

Thoughts?

@KevinJump KevinJump merged commit 141b692 into Jumoo:v10/main Aug 30, 2024
@KevinJump
Copy link
Member

Woot! -

Merged and in v10.2.0 release - https://www.nuget.org/packages/Our.Umbraco.MaintenanceMode/10.2.0

@craigagnew craigagnew deleted the v10-load-balanced branch September 4, 2024 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants