Hybrid Cache: Resolve start-up errors with mis-matched types (closes #20537) #20554
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…eval. Ensure nullability of ContentCacheNode is consistent in exists and retrieval.
Closed
…uring an initial request is served.
…move, and use of cancellation token.
nikolajlauridsen
approved these changes
Oct 21, 2025
Contributor
nikolajlauridsen
left a comment
nikolajlauridsen
left a comment
There was a problem hiding this comment.
Looks good, and tests good to me too.
I was only able to reproduce by adding an actually redis cache, but then the issue also became very consistent, but this PR fixes it, great work 🎉
nikolajlauridsen
pushed a commit
that referenced
this pull request
Oct 21, 2025
* Be consistent in use of GetOrCreateAsync overload in exists and retrieval. Ensure nullability of ContentCacheNode is consistent in exists and retrieval. * Applied suggestion from code review. * Move seeding to Umbraco application starting rather than started, ensuring an initial request is served. * Tighten up hybrid cache exists check with locking around check and remove, and use of cancellation token. (cherry picked from commit 81a8a0c)
Contributor
|
Cherry picked to release/17.0 👍 |
AndyButland
added a commit
that referenced
this pull request
Oct 21, 2025
* Be consistent in use of GetOrCreateAsync overload in exists and retrieval. Ensure nullability of ContentCacheNode is consistent in exists and retrieval. * Applied suggestion from code review. * Move seeding to Umbraco application starting rather than started, ensuring an initial request is served. * Tighten up hybrid cache exists check with locking around check and remove, and use of cancellation token.
AndyButland
changed the title
|
This pull request has been mentioned on Umbraco community forum. There might be relevant details there: https://forum.umbraco.com/t/umbraco-upgrade-15-4-4-16-3-1-hybridcache-with-the-same-key/6363/4 |
This was referenced Oct 27, 2025
This was referenced Nov 3, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Prerequisites
Addresses: #20537
Description
This PR makes two updates to resolve the linked issue;
For background, this "exists" check was added in #19890 which was introduced to improve start up performance (allowing us to retrieve cached documents in batches from the database rather than one at a time).
I was able to reproduce the problem manually - it seemed to be consistent at least when using the Visual Studio template and IIS Express:
The amends described above resolved the problem with the hybrid cache exception but revealed something else - that we would get an immediate page not found, and then after a refresh, the expected templated page would show. The crux of the problem here seemed to be that we are doing seeding on an application started notification, which doesn't seem correct. We should seed the initial content before we accept requests, and as such I've moved this to an application starting notification. That looks to resolve this problem.
There are some integration tests added in an attempt to replicate the problem. These didn't succeed in doing so, but are probably worth keeping anyway as additional testing around the hybrid cache.
Similarly there's some tightening up around the implementation of the "exists" check (the locking around "check and remove" and the cancellation token handling). None of this turned out to be the smoking gun, but I think they are valid improvements to consider keeping.
Testing
First try to replicate the problem consistently on
mainand then when you can, switch in the code from this PR and verify that the application starts without error and the expected initial page is shown.