Non-existent dictionary values hit database

[callumbwhyte]

Which Umbraco version are you using? (Please write the exact version, example: 10.1.0)

v8+

Bug summary

Calling the Umbraco dictionary with a key that does not exist results in a call to the database.

If the same dictionary key is attempted multiple times in the same request this leads to multiple database calls.

This happens even where a fallback value has been provided.

At a large enough scale this begins to impact performance – as discovered in load testing, where keys did not exist but fallbacks were provided.

Specifics

Since Umbraco 8 most repositories that result in database calls have been wrapped in a light caching layer (via EntityRepositoryBase) – this includes the DictionaryRepository.

The DefaultRepositoryCachePolicy will attempt to get the value (from the database) if it does not exist – but only caches the result if a value is returned.

Steps to reproduce

Add the ?umbDebug=true query parameter to load MiniProfiler (top right) and observe the database calls being made

Find a dictionary key that does not exist in the Umbraco dictionary (Translation section of CMS) – below I have used Missing.Key

Call @Umbraco.GetDictionaryValue("Missing.Key") in a Razor view – no value will be returned but a database call will be recorded in MiniProfiler

Call @Umbraco.GetDictionaryValueOrDefault("Missing.Key", "Fallback value") in a Razor view – the fallback value will be returned but a database call will still be recorded in MiniProfiler

Expected result / actual result

In the case of dictionary values, it would be good to avoid repeatedly querying the database for a value we know is not there...

It may not always be desirable to cache null values... public endpoints could be used to fill up memory, or you risk holding onto items in cache that don't need to be there – but I don't believe this applies to dictionary values that are usually requested via code. The cache key is cleared whenever it is saved, so holding onto a null value shouldn't have too many negative consequences.

This may not be a suitable change to the default cache behaviour, but perhaps would be suitable for just the dictionary repository.

[github-actions[bot]]

Hi there @callumbwhyte!

Firstly, a big thank you for raising this issue. Every piece of feedback we receive helps us to make Umbraco better.

We really appreciate your patience while we wait for our team to have a look at this but we wanted to let you know that we see this and share with you the plan for what comes next.

• We'll assess whether this issue relates to something that has already been fixed in a later version of the release that it has been raised for.
• If it's a bug, is it related to a release that we are actively supporting or is it related to a release that's in the end-of-life or security-only phase?
• We'll replicate the issue to ensure that the problem is as described.
• We'll decide whether the behavior is an issue or if the behavior is intended.

We wish we could work with everyone directly and assess your issue immediately but we're in the fortunate position of having lots of contributions to work with and only a few humans who are able to do it. We are making progress though and in the meantime, we will keep you in the loop and let you know when we have any questions.

Thanks, from your friendly Umbraco GitHub bot :robot: :slightly_smiling_face:

[marcemarc]

It's weird how often this comes up as the reason why the Umbraco site you are being asked to healthcheck is struggling under load!

What I always liked was this package for v7: https://our.umbraco.com/packages/developer-tools/dictionary-extension/ that if you specified a fallback value, and the dictionary key didn't exist, it would create it and populate it from the fallback on the first request... So subsequent requests worked from the light DB cache... :-) But really you just want to be alerted that it is happening, like in a healthcheck or in logs, you never really want the dictionary key to not exist, so allowing fallback text for that scenario just makes it really hard to detect... It would be better to throw an exception!

[bielu]

Shouldn't we just cache the fallback value in case if value doesnt exist 🤔? as if we provide fallback than we expect to be use it for most of the cases until it will be created in db. We can clear dictionary cache for this key in case if we create new value, but not sure if it wouldn't be code overkill 🤔

[Migaroez]

Hey all, thx for the reporting and discussion around this.

• I see no harm in adding a CacheNullValues bool to RepositoryCachePolicyOptions
• Adding a check in DefaultRepositoryCachePolicy.Get() to also Insert when CacheNullValues is true
• Setting the RepositoryCachePolicyOptions.CacheNullValues in DictionaryRepository.CreateCachePolicy() to true

As far as I know, there currently is no public unprotected endpoint to get dictionary items, we should keep this in mind when implementing https://github.com/umbraco/Umbraco-CMS/discussions/15214 though.

I will check with the team whether this is the way we want to go and get back to you.

[Migaroez]

Hey everyone, I have made a slight update to the implementation idea in the previous comment after going trough it with the team.

Marking this as an up for grabs as the path forward is pretty clear 😃

[github-actions[bot]]

Hi @callumbwhyte,

We're writing to let you know that we would love some help with this issue. We feel that this issue is ideal to flag for a community member to work on it. Once flagged here, folk looking for issues to work on will know to look at yours. Of course, please feel free work on this yourself ;-). If there are any changes to this status, we'll be sure to let you know.

For more information about issues and states, have a look at this blog post.

Thanks muchly, from your friendly Umbraco GitHub bot :-)