Changes PublishedSnapshotService to lazily load it's caches on demand when they are required instead of relying on an external initializer to load them.

This commit is contained in:
Shannon
2020-12-17 16:27:28 +11:00
parent 868c9d02df
commit cc1404747b
11 changed files with 131 additions and 172 deletions

View File

@@ -24,13 +24,9 @@ namespace Umbraco.Web.PublishedCache
*/
/// <summary>
/// Loads the caches on startup - called once during startup
/// TODO: Temporary, this is temporal coupling, we cannot use IUmbracoApplicationLifetime.ApplicationInit (which we want to delete)
/// handler because that is executed with netcore's IHostApplicationLifetime.ApplicationStarted mechanism which fires async
/// which we don't want since this will not have initialized before our endpoints execute. So for now this is explicitly
/// called on UseUmbracoContentCaching on startup.
/// Gets the published snapshot accessor.
/// </summary>
void LoadCachesOnStartup();
IPublishedSnapshotAccessor PublishedSnapshotAccessor { get; }
/// <summary>
/// Creates a published snapshot.
@@ -42,11 +38,6 @@ namespace Umbraco.Web.PublishedCache
/// which is not specified and depends on the actual published snapshot service implementation.</remarks>
IPublishedSnapshot CreatePublishedSnapshot(string previewToken);
/// <summary>
/// Gets the published snapshot accessor.
/// </summary>
IPublishedSnapshotAccessor PublishedSnapshotAccessor { get; }
/// <summary>
/// Ensures that the published snapshot has the proper environment to run.
/// </summary>
@@ -54,17 +45,15 @@ namespace Umbraco.Web.PublishedCache
/// <returns>A value indicating whether the published snapshot has the proper environment to run.</returns>
bool EnsureEnvironment(out IEnumerable<string> errors);
#region Rebuild
/// <summary>
/// Rebuilds internal caches (but does not reload).
/// Rebuilds internal database caches (but does not reload).
/// </summary>
/// <param name="groupSize">The operation batch size to process the items</param>
/// <param name="contentTypeIds">If not null will process content for the matching content types, if empty will process all content</param>
/// <param name="mediaTypeIds">If not null will process content for the matching media types, if empty will process all media</param>
/// <param name="memberTypeIds">If not null will process content for the matching members types, if empty will process all members</param>
/// <remarks>
/// <para>Forces the snapshot service to rebuild its internal caches. For instance, some caches
/// <para>Forces the snapshot service to rebuild its internal database caches. For instance, some caches
/// may rely on a database table to store pre-serialized version of documents.</para>
/// <para>This does *not* reload the caches. Caches need to be reloaded, for instance via
/// <see cref="DistributedCache" /> RefreshAllPublishedSnapshot method.</para>
@@ -75,10 +64,6 @@ namespace Umbraco.Web.PublishedCache
IReadOnlyCollection<int> mediaTypeIds = null,
IReadOnlyCollection<int> memberTypeIds = null);
#endregion
#region Changes
/* An IPublishedCachesService implementation can rely on transaction-level events to update
* its internal, database-level data, as these events are purely internal. However, it cannot
* rely on cache refreshers CacheUpdated events to update itself, as these events are external
@@ -123,8 +108,6 @@ namespace Umbraco.Web.PublishedCache
/// <param name="payloads">The changes.</param>
void Notify(DomainCacheRefresher.JsonPayload[] payloads);
#endregion
// TODO: This is weird, why is this is this a thing? Maybe IPublishedSnapshotStatus?
string GetStatus();

View File

@@ -6,6 +6,7 @@ using Umbraco.Web.Cache;
namespace Umbraco.Web.PublishedCache
{
// TODO: This base class probably shouldn't exist
public abstract class PublishedSnapshotServiceBase : IPublishedSnapshotService
{
/// <summary>
@@ -51,15 +52,12 @@ namespace Umbraco.Web.PublishedCache
/// <inheritdoc/>
public abstract void Notify(DomainCacheRefresher.JsonPayload[] payloads);
// TODO: Why is this virtual?
/// <inheritdoc/>
public virtual void Rebuild(
public abstract void Rebuild(
int groupSize = 5000,
IReadOnlyCollection<int> contentTypeIds = null,
IReadOnlyCollection<int> mediaTypeIds = null,
IReadOnlyCollection<int> memberTypeIds = null)
{ }
IReadOnlyCollection<int> memberTypeIds = null);
/// <inheritdoc/>
public virtual void Dispose()
@@ -76,6 +74,5 @@ namespace Umbraco.Web.PublishedCache
{
}
public abstract void LoadCachesOnStartup();
}
}

View File

@@ -1,6 +1,11 @@
using System;
using System;
namespace Umbraco.Web.PublishedCache
{
// TODO: This is a mess. This is a circular reference:
// IPublishedSnapshotAccessor -> PublishedSnapshotService -> UmbracoContext -> PublishedSnapshotService -> IPublishedSnapshotAccessor
// Injecting IPublishedSnapshotAccessor into PublishedSnapshotService seems pretty strange
// The underlying reason for this mess is because IPublishedContent is both a service and a model.
// Until that is fixed, IPublishedContent will need to have a IPublishedSnapshotAccessor
public class UmbracoContextPublishedSnapshotAccessor : IPublishedSnapshotAccessor
{
private readonly IUmbracoContextAccessor _umbracoContextAccessor;

View File

@@ -75,7 +75,7 @@ namespace Umbraco.Infrastructure.PublishedCache.Persistence
void RefreshEntity(IContentBase content);
/// <summary>
/// Rebuilds the caches for content, media and/or members based on the content type ids specified
/// Rebuilds the database caches for content, media and/or members based on the content type ids specified
/// </summary>
/// <param name="groupSize">The operation batch size to process the items</param>
/// <param name="contentTypeIds">If not null will process content for the matching content types, if empty will process all content</param>

View File

@@ -1,4 +1,4 @@
using System;
using System;
using System.Collections.Generic;
using System.Xml.Serialization;
using Umbraco.Core;
@@ -158,6 +158,7 @@ namespace Umbraco.Web.PublishedCache.NuCache
default:
throw new InvalidOperationException("Invalid cache level.");
}
return cacheValues;
}

View File

@@ -1,4 +1,4 @@
using System;
using System;
using System.Collections.Generic;
using System.Linq;
using Umbraco.Core;
@@ -43,7 +43,7 @@ namespace Umbraco.Web.PublishedCache.NuCache
// add one property per property type - this is required, for the indexing to work
// if contentData supplies pdatas, use them, else use null
contentData.Properties.TryGetValue(propertyType.Alias, out var pdatas); // else will be null
properties[i++] =new Property(propertyType, this, pdatas, _publishedSnapshotAccessor);
properties[i++] = new Property(propertyType, this, pdatas, _publishedSnapshotAccessor);
}
PropertiesArray = properties;
}

View File

@@ -3,6 +3,7 @@ using System.Collections.Generic;
using System.Globalization;
using System.IO;
using System.Linq;
using System.Threading;
using CSharpTest.Net.Collections;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;
@@ -46,7 +47,9 @@ namespace Umbraco.Web.PublishedCache.NuCache
private readonly NuCacheSettings _config;
// volatile because we read it with no lock
private volatile bool _isReady;
private bool _isReady;
private bool _isReadSet;
private object _isReadyLock;
private readonly ContentStore _contentStore;
private readonly ContentStore _mediaStore;
@@ -139,20 +142,20 @@ namespace Umbraco.Web.PublishedCache.NuCache
}
}
#region Id <-> Key methods
// NOTE: These aren't used within this object but are made available internally to improve the IdKey lookup performance
// when nucache is enabled.
// TODO: Does this need to be here?
// TODO: Does this need to be here?
internal int GetDocumentId(Guid udi) => GetId(_contentStore, udi);
internal int GetMediaId(Guid udi) => GetId(_mediaStore, udi);
internal Guid GetDocumentUid(int id) => GetUid(_contentStore, id);
internal Guid GetMediaUid(int id) => GetUid(_mediaStore, id);
private int GetId(ContentStore store, Guid uid) => store.LiveSnapshot.Get(uid)?.Id ?? 0;
private Guid GetUid(ContentStore store, int id) => store.LiveSnapshot.Get(id)?.Uid ?? Guid.Empty;
#endregion
internal int GetMediaId(Guid udi) => GetId(_mediaStore, udi);
internal Guid GetDocumentUid(int id) => GetUid(_contentStore, id);
internal Guid GetMediaUid(int id) => GetUid(_mediaStore, id);
private int GetId(ContentStore store, Guid uid) => store.LiveSnapshot.Get(uid)?.Id ?? 0;
private Guid GetUid(ContentStore store, int id) => store.LiveSnapshot.Get(id)?.Uid ?? Guid.Empty;
/// <summary>
/// Install phase of <see cref="IMainDom"/>
@@ -204,66 +207,6 @@ namespace Umbraco.Web.PublishedCache.NuCache
}
}
/// <summary>
/// Populates the stores
/// </summary>
public override void LoadCachesOnStartup()
{
lock (_storesLock)
{
if (_isReady)
{
throw new InvalidOperationException("The caches can only be loaded on startup one time");
}
var okContent = false;
var okMedia = false;
try
{
if (_localContentDbExists)
{
okContent = LockAndLoadContent(() => LoadContentFromLocalDbLocked(true));
if (!okContent)
{
_logger.LogWarning("Loading content from local db raised warnings, will reload from database.");
}
}
if (_localMediaDbExists)
{
okMedia = LockAndLoadMedia(() => LoadMediaFromLocalDbLocked(true));
if (!okMedia)
{
_logger.LogWarning("Loading media from local db raised warnings, will reload from database.");
}
}
if (!okContent)
{
LockAndLoadContent(() => LoadContentFromDatabaseLocked(true));
}
if (!okMedia)
{
LockAndLoadMedia(() => LoadMediaFromDatabaseLocked(true));
}
LockAndLoadDomains();
}
catch (Exception ex)
{
_logger.LogCritical(ex, "Panic, exception while loading cache data.");
throw;
}
// finally, cache is ready!
_isReady = true;
}
}
#region Local files
private string GetLocalFilesPath()
{
var path = Path.Combine(_hostingEnvironment.LocalTempPath, "NuCache");
@@ -278,7 +221,7 @@ namespace Umbraco.Web.PublishedCache.NuCache
private void DeleteLocalFilesForContent()
{
if (_isReady && _localContentDb != null)
if (Volatile.Read(ref _isReady) && _localContentDb != null)
{
throw new InvalidOperationException("Cannot delete local files while the cache uses them.");
}
@@ -293,7 +236,7 @@ namespace Umbraco.Web.PublishedCache.NuCache
private void DeleteLocalFilesForMedia()
{
if (_isReady && _localMediaDb != null)
if (Volatile.Read(ref _isReady) && _localMediaDb != null)
{
throw new InvalidOperationException("Cannot delete local files while the cache uses them.");
}
@@ -306,10 +249,6 @@ namespace Umbraco.Web.PublishedCache.NuCache
}
}
#endregion
#region Environment
public override bool EnsureEnvironment(out IEnumerable<string> errors)
{
// must have app_data and be able to write files into it
@@ -318,9 +257,62 @@ namespace Umbraco.Web.PublishedCache.NuCache
return ok;
}
#endregion
/// <summary>
/// Populates the stores
/// </summary>
private void EnsureCaches() => LazyInitializer.EnsureInitialized(
ref _isReady,
ref _isReadSet,
ref _isReadyLock,
() =>
{
// even though we are ready locked here we want to ensure that the stores lock is also locked
lock (_storesLock)
{
var okContent = false;
var okMedia = false;
#region Populate Stores
try
{
if (_localContentDbExists)
{
okContent = LockAndLoadContent(() => LoadContentFromLocalDbLocked(true));
if (!okContent)
{
_logger.LogWarning("Loading content from local db raised warnings, will reload from database.");
}
}
if (_localMediaDbExists)
{
okMedia = LockAndLoadMedia(() => LoadMediaFromLocalDbLocked(true));
if (!okMedia)
{
_logger.LogWarning("Loading media from local db raised warnings, will reload from database.");
}
}
if (!okContent)
{
LockAndLoadContent(() => LoadContentFromDatabaseLocked(true));
}
if (!okMedia)
{
LockAndLoadMedia(() => LoadMediaFromDatabaseLocked(true));
}
LockAndLoadDomains();
}
catch (Exception ex)
{
_logger.LogCritical(ex, "Panic, exception while loading cache data.");
throw;
}
return true;
}
});
// sudden panic... but in RepeatableRead can a content that I haven't already read, be removed
// before I read it? NO! because the WHOLE content tree is read-locked using WithReadLocked.
@@ -482,10 +474,6 @@ namespace Umbraco.Web.PublishedCache.NuCache
}
}
#endregion
#region Handle Notifications
// note: if the service is not ready, ie _isReady is false, then notifications are ignored
// SetUmbracoVersionStep issues a DistributedCache.Instance.RefreshAll...() call which should cause
@@ -512,7 +500,7 @@ namespace Umbraco.Web.PublishedCache.NuCache
public override void Notify(ContentCacheRefresher.JsonPayload[] payloads, out bool draftChanged, out bool publishedChanged)
{
// no cache, trash everything
if (_isReady == false)
if (Volatile.Read(ref _isReady) == false)
{
DeleteLocalFilesForContent();
draftChanged = publishedChanged = true;
@@ -613,7 +601,7 @@ namespace Umbraco.Web.PublishedCache.NuCache
public override void Notify(MediaCacheRefresher.JsonPayload[] payloads, out bool anythingChanged)
{
// no cache, trash everything
if (_isReady == false)
if (Volatile.Read(ref _isReady) == false)
{
DeleteLocalFilesForMedia();
anythingChanged = true;
@@ -711,7 +699,7 @@ namespace Umbraco.Web.PublishedCache.NuCache
public override void Notify(ContentTypeCacheRefresher.JsonPayload[] payloads)
{
// no cache, nothing we can do
if (_isReady == false)
if (Volatile.Read(ref _isReady) == false)
{
return;
}
@@ -812,7 +800,7 @@ namespace Umbraco.Web.PublishedCache.NuCache
public override void Notify(DataTypeCacheRefresher.JsonPayload[] payloads)
{
// no cache, nothing we can do
if (_isReady == false)
if (Volatile.Read(ref _isReady) == false)
{
return;
}
@@ -856,7 +844,7 @@ namespace Umbraco.Web.PublishedCache.NuCache
public override void Notify(DomainCacheRefresher.JsonPayload[] payloads)
{
// no cache, nothing we can do
if (_isReady == false)
if (Volatile.Read(ref _isReady) == false)
{
return;
}
@@ -894,12 +882,9 @@ namespace Umbraco.Web.PublishedCache.NuCache
// Methods used to prevent allocations of lists
private void AddToList(ref List<int> list, int val) => GetOrCreateList(ref list).Add(val);
private List<int> GetOrCreateList(ref List<int> list) => list ?? (list = new List<int>());
#endregion
#region Content Types
private IReadOnlyCollection<IPublishedContentType> CreateContentTypes(PublishedItemType itemType, int[] ids)
{
// XxxTypeService.GetAll(empty) returns everything!
@@ -1028,14 +1013,12 @@ namespace Umbraco.Web.PublishedCache.NuCache
}
}
#endregion
#region Create, Get Published Snapshot
public override IPublishedSnapshot CreatePublishedSnapshot(string previewToken)
{
EnsureCaches();
// no cache, no joy
if (_isReady == false)
if (Volatile.Read(ref _isReady) == false)
{
throw new InvalidOperationException("The published snapshot service has not properly initialized.");
}
@@ -1049,6 +1032,8 @@ namespace Umbraco.Web.PublishedCache.NuCache
// even though the underlying elements may not change (store snapshots)
public PublishedSnapshot.PublishedSnapshotElements GetElements(bool previewDefault)
{
EnsureCaches();
// note: using ObjectCacheAppCache for elements and snapshot caches
// is not recommended because it creates an inner MemoryCache which is a heavy
// thing - better use a dictionary-based cache which "just" creates a concurrent
@@ -1129,10 +1114,7 @@ namespace Umbraco.Web.PublishedCache.NuCache
};
}
#endregion
#region Rebuild Database PreCache
/// <inheritdoc />
public override void Rebuild(
int groupSize = 5000,
IReadOnlyCollection<int> contentTypeIds = null,
@@ -1176,12 +1158,10 @@ namespace Umbraco.Web.PublishedCache.NuCache
}
}
#endregion
#region Instrument
public override string GetStatus()
{
EnsureCaches();
var dbCacheIsOk = VerifyContentDbCache()
&& VerifyMediaDbCache()
&& VerifyMemberDbCache();
@@ -1203,20 +1183,26 @@ namespace Umbraco.Web.PublishedCache.NuCache
" and " + ms + " snapshot" + (ms > 1 ? "s" : "") + ".";
}
// TODO: This should be async since it's calling into async
public override void Collect()
{
EnsureCaches();
var contentCollect = _contentStore.CollectAsync();
var mediaCollect = _mediaStore.CollectAsync();
System.Threading.Tasks.Task.WaitAll(contentCollect, mediaCollect);
}
#endregion
internal ContentStore GetContentStore()
{
EnsureCaches();
return _contentStore;
}
#region Internals/Testing
internal ContentStore GetContentStore() => _contentStore;
internal ContentStore GetMediaStore() => _mediaStore;
#endregion
internal ContentStore GetMediaStore()
{
EnsureCaches();
return _mediaStore;
}
}
}

View File

@@ -40,10 +40,6 @@ namespace Umbraco.Web.PublishedCache.NuCache
return false;
}
// this initializes the caches.
// TODO: This is still temporal coupling (i.e. Initialize)
_publishedSnapshotService.LoadCachesOnStartup();
// we always want to handle repository events, configured or not
// assuming no repository event will trigger before the whole db is ready
// (ideally we'd have Upgrading.App vs Upgrading.Data application states...)

View File

@@ -1,4 +1,4 @@
using System.Linq;
using System.Linq;
using System.Net;
using System.Net.Http;
using System.Threading.Tasks;
@@ -22,13 +22,12 @@ namespace Umbraco.Tests.Integration.TestServerTest.Controllers
/// <summary>
/// Returns 404 if the content wasn't found based on the ID specified
/// </summary>
/// <returns></returns>
[Test]
public async Task PostSave_Validate_Existing_Content()
{
var localizationService = GetRequiredService<ILocalizationService>();
//Add another language
// Add another language
localizationService.Save(new LanguageBuilder()
.WithCultureInfo("da-DK")
.WithIsDefault(false)
@@ -76,10 +75,10 @@ namespace Umbraco.Tests.Integration.TestServerTest.Controllers
// Assert
Assert.AreEqual(HttpStatusCode.NotFound, response.StatusCode);
// Assert.AreEqual(")]}',\n{\"Message\":\"content was not found\"}", response.Item1.Content.ReadAsStringAsync().Result);
//
// //var obj = JsonConvert.DeserializeObject<PagedResult<UserDisplay>>(response.Item2);
// //Assert.AreEqual(0, obj.TotalItems);
// Assert.AreEqual(")]}',\n{\"Message\":\"content was not found\"}", response.Item1.Content.ReadAsStringAsync().Result);
//
// //var obj = JsonConvert.DeserializeObject<PagedResult<UserDisplay>>(response.Item2);
// //Assert.AreEqual(0, obj.TotalItems);
}
[Test]
@@ -88,7 +87,7 @@ namespace Umbraco.Tests.Integration.TestServerTest.Controllers
var localizationService = GetRequiredService<ILocalizationService>();
//Add another language
// Add another language
localizationService.Save(new LanguageBuilder()
.WithCultureInfo("da-DK")
.WithIsDefault(false)
@@ -96,7 +95,6 @@ namespace Umbraco.Tests.Integration.TestServerTest.Controllers
var url = PrepareUrl<ContentController>(x => x.PostSave(null));
var contentTypeService = GetRequiredService<IContentTypeService>();
var contentType = new ContentTypeBuilder()
@@ -128,7 +126,7 @@ namespace Umbraco.Tests.Integration.TestServerTest.Controllers
.Build();
// HERE we force the test to fail
model.Variants = model.Variants.Select(x=>
model.Variants = model.Variants.Select(x =>
{
x.Save = false;
return x;
@@ -141,7 +139,6 @@ namespace Umbraco.Tests.Integration.TestServerTest.Controllers
});
// Assert
var body = await response.Content.ReadAsStringAsync();
Assert.Multiple(() =>
@@ -155,13 +152,12 @@ namespace Umbraco.Tests.Integration.TestServerTest.Controllers
/// <summary>
/// Returns 404 if any of the posted properties dont actually exist
/// </summary>
/// <returns></returns>
[Test]
public async Task PostSave_Validate_Properties_Exist()
{
var localizationService = GetRequiredService<ILocalizationService>();
//Add another language
// Add another language
localizationService.Save(new LanguageBuilder()
.WithCultureInfo("da-DK")
.WithIsDefault(false)
@@ -215,12 +211,11 @@ namespace Umbraco.Tests.Integration.TestServerTest.Controllers
});
// Assert
var body = await response.Content.ReadAsStringAsync();
body = body.TrimStart(AngularJsonMediaTypeFormatter.XsrfPrefix);
Assert.AreEqual(HttpStatusCode.NotFound, response.StatusCode);
Assert.AreEqual(HttpStatusCode.NotFound, response.StatusCode);
}
[Test]

View File

@@ -1,4 +1,4 @@

using System;
using System.Linq.Expressions;
using System.Net.Http;
@@ -116,20 +116,20 @@ namespace Umbraco.Tests.Integration.TestServerTest
}
protected HttpClient Client { get; private set; }
protected LinkGenerator LinkGenerator { get; private set; }
protected WebApplicationFactory<UmbracoTestServerTestBase> Factory { get; private set; }
[TearDown]
public override void TearDown()
{
base.TearDown();
base.TerminateCoreRuntime();
TerminateCoreRuntime();
Factory.Dispose();
}
#region IStartup
public override void ConfigureServices(IServiceCollection services)
{
services.AddTransient<TestUmbracoDatabaseFactoryProvider>();
@@ -160,9 +160,5 @@ namespace Umbraco.Tests.Integration.TestServerTest
{
app.UseUmbraco();
}
#endregion
}
}

View File

@@ -253,6 +253,6 @@ namespace Umbraco.Tests.LegacyXmlPublishedCache
return "Test status";
}
public override void LoadCachesOnStartup() { }
public override void Rebuild(int groupSize = 5000, IReadOnlyCollection<int> contentTypeIds = null, IReadOnlyCollection<int> mediaTypeIds = null, IReadOnlyCollection<int> memberTypeIds = null) { }
}
}