Skip to content

HybridCache: Add locks to prevent potential SQL Server deadlocks #21294

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.

Sign up for GitHub

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

Open
lauraneto wants to merge 1 commit into
base: main
Choose a base branch
from
Open

HybridCache: Add locks to prevent potential SQL Server deadlocks #21294

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions ...mbraco.PublishedCache.HybridCache/SeedKeyProviders/Document/ContentTypeSeedKeyProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using Microsoft.Extensions.Options;
using Umbraco.Cms.Core;
using Umbraco.Cms.Core.Models;
using Umbraco.Cms.Core.Scoping;
using Umbraco.Cms.Core.Services.Navigation;
Expand Down Expand Up @@ -28,6 +29,7 @@ public ContentTypeSeedKeyProvider(
public ISet<Guid> GetSeedKeys()
{
using ICoreScope scope = _scopeProvider.CreateCoreScope();
scope.ReadLock(Constants.Locks.ContentTree);
Copy link
Contributor

@AndyButland AndyButland Jan 16, 2026

Choose a reason for hiding this comment

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

I'm not sure that this lock should be necessary, or, if we do need one, should it be a new lock ID that is dedicated for reads from the cmsContentNu table? What I'm thinking is that it shouldn't be necessary to lock reads for the content tree - which will be umbracoNode, umbracoContent, umbracoDocument etc., to read from a table that's unrelated in the sense of the database schema.

I think the same applies for all the others added in the PR.

So - of course if in the testing you've since done or can do with a large database you see improvements, then probably we should discount my point noted here. But if we can't see any issue, then I'm not sure there's value in adding these locks as they are.

var documentKeys = _databaseCacheRepository
.GetDocumentKeysByContentTypeKeys(_cacheSettings.ContentTypeKeys, published: true)
.Where(key => _publishStatusService.IsDocumentPublishedInAnyCulture(key))
Expand Down
4 changes: 4 additions & 0 deletions src/Umbraco.PublishedCache.HybridCache/Services/DocumentCacheService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ public DocumentCacheService(
async cancel =>
{
using ICoreScope scope = _scopeProvider.CreateCoreScope();
scope.ReadLock(Constants.Locks.ContentTree);
ContentCacheNode? contentCacheNode = await _databaseCacheRepository.GetContentSourceAsync(key, preview);

// If we can resolve the content cache node, we still need to check if the ancestor path is published.
Expand Down Expand Up @@ -159,6 +160,7 @@ public DocumentCacheService(
public IEnumerable<IPublishedContent> GetByContentType(IPublishedContentType contentType)
{
using ICoreScope scope = _scopeProvider.CreateCoreScope();
scope.ReadLock(Constants.Locks.ContentTree);
IEnumerable<ContentCacheNode> nodes = _databaseCacheRepository.GetContentByContentTypeKey([contentType.Key], ContentCacheDataSerializerEntityType.Document);
scope.Complete();

Expand Down Expand Up @@ -237,6 +239,7 @@ public async Task SeedAsync(CancellationToken cancellationToken)
}

using ICoreScope scope = _scopeProvider.CreateCoreScope();
scope.ReadLock(Constants.Locks.ContentTree);

IEnumerable<ContentCacheNode> cacheNodes = await _databaseCacheRepository.GetContentSourcesAsync(uncachedKeys);

Expand Down Expand Up @@ -354,6 +357,7 @@ public void Rebuild(IReadOnlyCollection<int> contentTypeIds)
public async Task RebuildMemoryCacheByContentTypeAsync(IEnumerable<int> contentTypeIds)
{
using ICoreScope scope = _scopeProvider.CreateCoreScope();
scope.ReadLock(Constants.Locks.ContentTree);

IEnumerable<ContentCacheNode> contentByContentTypeKey = _databaseCacheRepository.GetContentByContentTypeKey(contentTypeIds.Select(x => _idKeyMap.GetKeyForId(x, UmbracoObjectTypes.DocumentType).Result), ContentCacheDataSerializerEntityType.Document);
scope.Complete();
Expand Down
5 changes: 5 additions & 0 deletions src/Umbraco.PublishedCache.HybridCache/Services/MediaCacheService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ public MediaCacheService(
async cancel =>
{
using ICoreScope scope = _scopeProvider.CreateCoreScope();
scope.ReadLock(Constants.Locks.MediaTree);
ContentCacheNode? mediaCacheNode = await _databaseCacheRepository.GetMediaSourceAsync(key);
scope.Complete();
return mediaCacheNode;
Expand Down Expand Up @@ -201,6 +202,7 @@ public async Task SeedAsync(CancellationToken cancellationToken)
}

using ICoreScope scope = _scopeProvider.CreateCoreScope();
scope.ReadLock(Constants.Locks.MediaTree);

IEnumerable<ContentCacheNode> cacheNodes = await _databaseCacheRepository.GetMediaSourcesAsync(uncachedKeys);

Expand Down Expand Up @@ -231,6 +233,7 @@ await _hybridCache.SetAsync(
public async Task RefreshMemoryCacheAsync(Guid key)
{
using ICoreScope scope = _scopeProvider.CreateCoreScope();
scope.ReadLock(Constants.Locks.MediaTree);

ContentCacheNode? publishedNode = await _databaseCacheRepository.GetMediaSourceAsync(key);
if (publishedNode is not null)
Expand All @@ -257,6 +260,7 @@ public async Task RemoveFromMemoryCacheAsync(Guid key)
public async Task RebuildMemoryCacheByContentTypeAsync(IEnumerable<int> mediaTypeIds)
{
using ICoreScope scope = _scopeProvider.CreateCoreScope();
scope.ReadLock(Constants.Locks.MediaTree);

IEnumerable<ContentCacheNode> contentByContentTypeKey = _databaseCacheRepository.GetContentByContentTypeKey(mediaTypeIds.Select(x => _idKeyMap.GetKeyForId(x, UmbracoObjectTypes.MediaType).Result), ContentCacheDataSerializerEntityType.Media);

Expand Down Expand Up @@ -301,6 +305,7 @@ public void Rebuild(IReadOnlyCollection<int> contentTypeIds)
public IEnumerable<IPublishedContent> GetByContentType(IPublishedContentType contentType)
{
using ICoreScope scope = _scopeProvider.CreateCoreScope();
scope.ReadLock(Constants.Locks.MediaTree);
IEnumerable<ContentCacheNode> nodes = _databaseCacheRepository.GetContentByContentTypeKey([contentType.Key], ContentCacheDataSerializerEntityType.Media);
scope.Complete();

Expand Down
Loading