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

Site Settings not updated when running on multiple instances #895

Open
MarkvDijk opened this issue Nov 8, 2022 · 5 comments · May be fixed by #909
Open

Site Settings not updated when running on multiple instances #895

MarkvDijk opened this issue Nov 8, 2022 · 5 comments · May be fixed by #909

Comments

@MarkvDijk
Copy link

When running the site on multiple instances, Site Settings are not replicated to the memory of other instances.

When updating a property in the Site Settings and publishing the content, the update is stored in the database. However the SettingsService stores a copy of the settings in memory public ConcurrentDictionary<string, Dictionary<Type, object>> SiteSettings
This is done in the PublishContent event using UpdateSettings(...). This event is only triggered on the instance you are connected to. The other instances don't trigger this event, therefore not updating the SiteSettings dictionary and returning old settings.

Issue can be replicated by using 2 browsers. Check the ARRAffinity cookie for the instance Id you are connect to. They must be different for both browsers. In both browsers open the same page that displays one of the site settings. Update the site setting in browser 1 and refresh the page. the updated setting is visible. Refresh the page in browser 2, it still displays the old setting.

Browser 2 won't display the new setting unless you publish it from that browser as well (causing the dictionary to update)

@Hinneman
Copy link

Hinneman commented Feb 1, 2023

We have exactly the same problem using Site Settings in DXP Production.

Any idea on how to solve this?

@lunchin lunchin linked a pull request Feb 13, 2023 that will close this issue
@lunchin
Copy link
Contributor

lunchin commented Feb 13, 2023

Can you please check the diff of the pull requests #909 if this fixes your issues before I merge.

@Hinneman
Copy link

I will implement this change and get back to you when verified.

@Hinneman
Copy link

We have now implemented the changes, deployed them to preprod, scaled up environment and verified with two browsers with different ARRAffinity cookies.

A change to the web site settings is still only shown in the browser where the change was published not the other one.

@jonascarlbaum
Copy link

jonascarlbaum commented Jun 17, 2024

My fix was mostly solved by updating GetSiteSettings-method like this. (splitted it up for simplification, can be optimized)

public SettingsBase GetSiteSettings<T>(Guid? siteId = null) where T : SettingsBase
{
    if (!siteId.HasValue)
    {
        siteId = ResolveSiteId();
        if (siteId == Guid.Empty)
        {
            return default;
        }
    }
    try
    {
        if (PageEditing.PageIsInEditMode)
        {
            return GetEditModeSettings<T>(siteId.Value.ToString() + "-common-draft");
        }
        else
        {
            return GetViewModeSettings<T>(siteId.Value.ToString());
        }
    }
    catch (KeyNotFoundException keyNotFoundException)
    {
        _log.Error($"[Settings] {keyNotFoundException.Message}", exception: keyNotFoundException);
    }
    catch (ArgumentNullException argumentNullException)
    {
        _log.Error($"[Settings] {argumentNullException.Message}", exception: argumentNullException);
    }

    return default;
}

private T GetEditModeSettings<T>(string cacheKey) where T : SettingsBase
{
    return _synchronizedObjectInstanceCache.ReadThrough<T>($"{typeof(T).Name}:{cacheKey}",
        () =>
        {
            UpdateSettings();

            if (SiteSettings.TryGetValue(cacheKey, out var siteSettings))
            {
                if (siteSettings.TryGetValue(typeof(T), out var setting))
                {
                    return (T)setting;
                }
            }

            return default;
        },
        (result) => new CacheEvictionPolicy(TimeSpan.FromMinutes(1), CacheTimeoutType.Absolute, null, new[] { Settings.MASTERKEY }),
        ReadStrategy.Wait
    );
}

private T GetViewModeSettings<T>(string cacheKey) where T : SettingsBase
{
    return _synchronizedObjectInstanceCache.ReadThrough($"{typeof(T).Name}:{cacheKey}",
        () =>
        {
            UpdateSettings();

            if (SiteSettings.TryGetValue(cacheKey, out var siteSettings) && siteSettings.TryGetValue(typeof(T), out var setting))
            {
                return (T)setting;
            }

            return default;
        },
        (result) => new CacheEvictionPolicy(TimeSpan.FromMinutes(10), CacheTimeoutType.Absolute, null, new[] { Settings.MASTERKEY }),
        ReadStrategy.Wait
    );
}

The key here is calling UpdateSettings(); when cache invalidates, so the Dictionary is also rebuilt.

and for invalidation purposes we added _synchronizedObjectInstanceCache.Remove(Settings.MASTERKEY); on some places, like

        private void SiteCreated(object sender, SiteDefinitionEventArgs e)
        {
            if (_contentRepository.GetChildren<SettingsFolder>(GlobalSettingsRoot)
                .Any(x => x.Name.Equals(e.Site.Name, StringComparison.InvariantCultureIgnoreCase)))
            {
                return;
            }

            CreateSiteFolder(e.Site);
            _synchronizedObjectInstanceCache.Remove(Settings.MASTERKEY);
        }

        private void SiteDeleted(object sender, SiteDefinitionEventArgs e)
        {
            var folder = _contentRepository.GetChildren<SettingsFolder>(GlobalSettingsRoot)
                .FirstOrDefault(x => x.Name.Equals(e.Site.Name, StringComparison.InvariantCultureIgnoreCase));

            if (folder == null)
            {
                return;
            }

            _contentRepository.Delete(folder.ContentLink, true, AccessLevel.NoAccess);
            _synchronizedObjectInstanceCache.Remove(Settings.MASTERKEY);
        }

        private void SiteUpdated(object sender, SiteDefinitionEventArgs e)
        {
            var folder = _contentRepository.GetChildren<SettingsFolder>(GlobalSettingsRoot)
                .FirstOrDefault(x => x.Name.Equals(e.Site.Name, StringComparison.InvariantCultureIgnoreCase));

            if (folder != null)
            {
                return;
            }

            CreateSiteFolder(e.Site);
            _synchronizedObjectInstanceCache.Remove(Settings.MASTERKEY);
        }

        private void PublishedContent(object sender, ContentEventArgs e)
        {
            if (e == null)
            {
                return;
            }

            if (e.Content is SettingsBase)
            {
                var id = ResolveSiteId();
                if (id == Guid.Empty)
                {
                    return;
                }
                UpdateSettings(id, e.Content, false);

                _synchronizedObjectInstanceCache.Remove(Settings.MASTERKEY);
            }
        }

        private void SavedContent(object sender, ContentEventArgs e)
        {
            if (e == null)
            {
                return;
            }

            if (e.Content is SettingsBase)
            {
                var id = ResolveSiteId();
                if (id == Guid.Empty)
                {
                    return;
                }
                UpdateSettings(id, e.Content, true);

                _synchronizedObjectInstanceCache.Remove(Settings.MASTERKEY);
            }
        }

Think we needed to change some stuff, like

T GetSiteSettings<T>(Guid? siteId = null);

to

SettingsBase GetSiteSettings<T>(Guid? siteId = null) where T : SettingsBase;

and do some type casting in code, otherwise the code changes above (if I remember correctly) works well for us.
Was your solution similar @Hinneman?

We have some more changes in our solution, so I can't just copy&paste the whole stuff, and won't do a PR now.
But if someone can test these changes out on the vanilla version and commit a PR it would be great... ;)

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 a pull request may close this issue.

4 participants