diff --git a/src/Umbraco.Core/Runtime/CoreRuntime.cs b/src/Umbraco.Core/Runtime/CoreRuntime.cs index b46058fa82..b852aff2ff 100644 --- a/src/Umbraco.Core/Runtime/CoreRuntime.cs +++ b/src/Umbraco.Core/Runtime/CoreRuntime.cs @@ -147,7 +147,7 @@ namespace Umbraco.Core.Runtime // TODO: remove this in netcore, this is purely backwards compat hacks with the empty ctor if (MainDom == null) { - MainDom = new MainDom(Logger, new MainDomSemaphoreLock()); + MainDom = new MainDom(Logger, new MainDomSemaphoreLock(Logger)); } diff --git a/src/Umbraco.Core/Runtime/MainDom.cs b/src/Umbraco.Core/Runtime/MainDom.cs index 85cf5ad6a9..883b69b2a7 100644 --- a/src/Umbraco.Core/Runtime/MainDom.cs +++ b/src/Umbraco.Core/Runtime/MainDom.cs @@ -36,7 +36,7 @@ namespace Umbraco.Core.Runtime // actions to run before releasing the main domain private readonly List> _callbacks = new List>(); - private const int LockTimeoutMilliseconds = 90000; // (1.5 * 60 * 1000) == 1 min 30 seconds + private const int LockTimeoutMilliseconds = 10000; // 10 seconds #endregion @@ -106,6 +106,7 @@ namespace Umbraco.Core.Runtime { _logger.Info("Stopping ({SignalSource})", source); foreach (var callback in _callbacks.OrderBy(x => x.Key).Select(x => x.Value)) + { try { callback(); // no timeout on callbacks @@ -115,6 +116,8 @@ namespace Umbraco.Core.Runtime _logger.Error(e, "Error while running callback"); continue; } + } + _logger.Debug("Stopped ({SignalSource})", source); } finally @@ -142,17 +145,33 @@ namespace Umbraco.Core.Runtime _logger.Info("Acquiring."); // Get the lock - _mainDomLock.AcquireLockAsync(LockTimeoutMilliseconds).Wait(); + var acquired = _mainDomLock.AcquireLockAsync(LockTimeoutMilliseconds).Result; + + if (!acquired) + { + _logger.Info("Cannot acquire (timeout)."); + + // TODO: Previously we'd throw an exception and the appdomain would not start, what do we want to do? + + // return false; + + throw new TimeoutException("Cannot acquire MainDom"); + } try { // Listen for the signal from another AppDomain coming online to release the lock _mainDomLock.ListenAsync() - .ContinueWith(_ => OnSignal("signal")); + .ContinueWith(_ => + { + _logger.Debug("Signal heard from other appdomain."); + OnSignal("signal"); + }); } - catch (OperationCanceledException) + catch (OperationCanceledException ex) { // the waiting task could be canceled if this appdomain is naturally shutting down, we'll just swallow this exception + _logger.Warn(ex, ex.Message); } _logger.Info("Acquired."); diff --git a/src/Umbraco.Core/Runtime/MainDomSemaphoreLock.cs b/src/Umbraco.Core/Runtime/MainDomSemaphoreLock.cs index 89eef8a658..2dcc37e25f 100644 --- a/src/Umbraco.Core/Runtime/MainDomSemaphoreLock.cs +++ b/src/Umbraco.Core/Runtime/MainDomSemaphoreLock.cs @@ -1,6 +1,7 @@ using System; using System.Threading; using System.Threading.Tasks; +using Umbraco.Core.Logging; namespace Umbraco.Core.Runtime { @@ -14,16 +15,17 @@ namespace Umbraco.Core.Runtime // event wait handle used to notify current main domain that it should // release the lock because a new domain wants to be the main domain private readonly EventWaitHandle _signal; - + private readonly ILogger _logger; private IDisposable _lockRelease; - public MainDomSemaphoreLock() + public MainDomSemaphoreLock(ILogger logger) { var lockName = "UMBRACO-" + MainDom.GetMainDomId() + "-MAINDOM-LCK"; _systemLock = new SystemLock(lockName); var eventName = "UMBRACO-" + MainDom.GetMainDomId() + "-MAINDOM-EVT"; _signal = new EventWaitHandle(false, EventResetMode.AutoReset, eventName); + _logger = logger; } //WaitOneAsync (ext method) will wait for a signal without blocking the main thread, the waiting is done on a background thread @@ -47,8 +49,9 @@ namespace Umbraco.Core.Runtime _lockRelease = _systemLock.Lock(millisecondsTimeout); return Task.FromResult(true); } - catch (TimeoutException) + catch (TimeoutException ex) { + _logger.Error(ex); return Task.FromResult(false); } finally diff --git a/src/Umbraco.Core/Runtime/SqlMainDomLock.cs b/src/Umbraco.Core/Runtime/SqlMainDomLock.cs index 847a0c25df..31f8b36ed0 100644 --- a/src/Umbraco.Core/Runtime/SqlMainDomLock.cs +++ b/src/Umbraco.Core/Runtime/SqlMainDomLock.cs @@ -109,62 +109,62 @@ namespace Umbraco.Core.Runtime // Create a long running task (dedicated thread) // to poll to check if we are still the MainDom registered in the DB - return Task.Factory.StartNew(() => + return Task.Factory.StartNew(ListeningLoop, _cancellationTokenSource.Token, TaskCreationOptions.LongRunning, TaskScheduler.Default); + + } + + private void ListeningLoop() + { + while (true) { - while(true) + // poll every 1 second + Thread.Sleep(1000); + + lock (_locker) { - // poll every 1 second - Thread.Sleep(1000); + // If cancellation has been requested we will just exit. Depending on timing of the shutdown, + // we will have already flagged _mainDomChanging = true, or we're shutting down faster than + // the other MainDom is taking to startup. In this case the db row will just be deleted and the + // new MainDom will just take over. + if (_cancellationTokenSource.IsCancellationRequested) + return; - lock(_locker) + var db = GetDatabase(); + + try { - // If cancellation has been requested we will just exit. Depending on timing of the shutdown, - // we will have already flagged _mainDomChanging = true, or we're shutting down faster than - // the other MainDom is taking to startup. In this case the db row will just be deleted and the - // new MainDom will just take over. - if (_cancellationTokenSource.IsCancellationRequested) - break; + db.BeginTransaction(IsolationLevel.ReadCommitted); - var db = GetDatabase(); + // get a read lock + _sqlServerSyntax.ReadLock(db, Constants.Locks.MainDom); - try + // TODO: We could in theory just check if the main dom row doesn't exist, that could indicate that + // we are still the maindom. An empty value might be better because then we won't have any orphan rows + // if the app is terminated. Could that work? + + if (!IsMainDomValue(_lockId)) { - db.BeginTransaction(IsolationLevel.ReadCommitted); - - // get a read lock - _sqlServerSyntax.ReadLock(db, Constants.Locks.MainDom); - - // TODO: We could in theory just check if the main dom row doesn't exist, that could indicate that - // we are still the maindom. An empty value might be better because then we won't have any orphan rows - // if the app is terminated. Could that work? - - if (!IsMainDomValue(_lockId)) - { - // we are no longer main dom, another one has come online, exit - _mainDomChanging = true; - _logger.Debug("Detected new booting application, releasing MainDom."); - return; - } - } - catch (Exception ex) - { - ResetDatabase(); - // unexpected - _logger.Error(ex, "Unexpected error, listening is canceled."); - _hasError = true; + // we are no longer main dom, another one has come online, exit + _mainDomChanging = true; + _logger.Debug("Detected new booting application, releasing MainDom lock."); return; } - finally - { - db?.CompleteTransaction(); - } } - + catch (Exception ex) + { + ResetDatabase(); + // unexpected + _logger.Error(ex, "Unexpected error, listening is canceled."); + _hasError = true; + return; + } + finally + { + db?.CompleteTransaction(); + } } - - - }, _cancellationTokenSource.Token, TaskCreationOptions.LongRunning, TaskScheduler.Default); + } } private void ResetDatabase() diff --git a/src/Umbraco.Web/PublishedCache/NuCache/ContentStore.cs b/src/Umbraco.Web/PublishedCache/NuCache/ContentStore.cs index 550fd507d5..81229af9e1 100644 --- a/src/Umbraco.Web/PublishedCache/NuCache/ContentStore.cs +++ b/src/Umbraco.Web/PublishedCache/NuCache/ContentStore.cs @@ -115,11 +115,14 @@ namespace Umbraco.Web.PublishedCache.NuCache // TODO: GetScopedWriter? should the dict have a ref onto the scope provider? public IDisposable GetScopedWriteLock(IScopeProvider scopeProvider) { + _logger.Debug("GetScopedWriteLock"); return ScopeContextualBase.Get(scopeProvider, _instanceId, scoped => new ScopedWriteLock(this, scoped)); } private void Lock(WriteLockInfo lockInfo, bool forceGen = false) { + // TODO: We are in a deadlock here somehow?? + Monitor.Enter(_wlocko, ref lockInfo.Taken); var rtaken = false; @@ -193,8 +196,15 @@ namespace Umbraco.Web.PublishedCache.NuCache _localDb.Commit(); } - if (lockInfo.Count) _wlocked--; - if (lockInfo.Taken) Monitor.Exit(_wlocko); + // TODO: Shouldn't this be in a finally block? + // TODO: Shouldn't this be decremented after we exit?? + // TODO: Shouldn't the locked flag never exceed 1? + if (lockInfo.Count) + _wlocked--; + + // TODO: Shouldn't this be in a finally block? + if (lockInfo.Taken) + Monitor.Exit(_wlocko); } private void Release(ReadLockInfo lockInfo) @@ -232,21 +242,29 @@ namespace Umbraco.Web.PublishedCache.NuCache public void ReleaseLocalDb() { + _logger.Info("Releasing DB..."); var lockInfo = new WriteLockInfo(); try { try { // Trying to lock could throw exceptions so always make sure to clean up. + _logger.Info("Trying to lock before releasing DB (lock count = {LockCount}) ...", _wlocked); + Lock(lockInfo); } finally { try { + _logger.Info("Disposing local DB..."); _localDb?.Dispose(); } - catch { /* TBD: May already be throwing so don't throw again */} + catch (Exception ex) + { + /* TBD: May already be throwing so don't throw again */ + _logger.Error(ex, "Error trying to release DB"); + } finally { _localDb = null; @@ -254,8 +272,17 @@ namespace Umbraco.Web.PublishedCache.NuCache } } + catch (Exception ex) + { + _logger.Error(ex, "Error trying to lock"); + throw; + } finally { + _logger.Info("Releasing ContentStore..."); + + // TODO: I don't understand this, we would have already closed the BPlusTree store above?? + // I guess it's 'safe' and will just decrement the write lock counter? Release(lockInfo); } } @@ -275,6 +302,7 @@ namespace Umbraco.Web.PublishedCache.NuCache var lockInfo = new WriteLockInfo(); try { + _logger.Debug("NewContentTypes"); Lock(lockInfo); foreach (var type in types) @@ -297,6 +325,7 @@ namespace Umbraco.Web.PublishedCache.NuCache var lockInfo = new WriteLockInfo(); try { + _logger.Debug("UpdateContentTypes"); Lock(lockInfo); var index = types.ToDictionary(x => x.Id, x => x); @@ -324,10 +353,13 @@ namespace Umbraco.Web.PublishedCache.NuCache public void SetAllContentTypes(IEnumerable types) { - var lockInfo = new WriteLockInfo(); + // TODO: There should be NO lock here! All calls made to this are wrapped in GetScopedWriteLock + + //var lockInfo = new WriteLockInfo(); try { - Lock(lockInfo); + _logger.Debug("SetAllContentTypes"); + //Lock(lockInfo); // clear all existing content types ClearLocked(_contentTypesById); @@ -345,7 +377,7 @@ namespace Umbraco.Web.PublishedCache.NuCache } finally { - Release(lockInfo); + //Release(lockInfo); } } @@ -362,6 +394,7 @@ namespace Umbraco.Web.PublishedCache.NuCache var lockInfo = new WriteLockInfo(); try { + _logger.Debug("UpdateContentTypes"); Lock(lockInfo); var removedContentTypeNodes = new List(); @@ -435,6 +468,7 @@ namespace Umbraco.Web.PublishedCache.NuCache var lockInfo = new WriteLockInfo(); try { + _logger.Debug("UpdateDataTypes"); Lock(lockInfo); var contentTypes = _contentTypesById @@ -545,10 +579,11 @@ namespace Umbraco.Web.PublishedCache.NuCache _logger.Debug("Set content ID: {KitNodeId}", kit.Node.Id); - var lockInfo = new WriteLockInfo(); + // TODO: There should be NO locks here, all calls made to this are done within GetScopedWriteLock + //var lockInfo = new WriteLockInfo(); try { - Lock(lockInfo); + //Lock(lockInfo); // get existing _contentNodes.TryGetValue(kit.Node.Id, out var link); @@ -594,7 +629,7 @@ namespace Umbraco.Web.PublishedCache.NuCache } finally { - Release(lockInfo); + //Release(lockInfo); } return true; @@ -623,11 +658,16 @@ namespace Umbraco.Web.PublishedCache.NuCache /// internal bool SetAllFastSorted(IEnumerable kits, bool fromDb) { - var lockInfo = new WriteLockInfo(); + + //TODO: There should be NO locks here all calls made to this are done within GetScopedWriteLock + + //var lockInfo = new WriteLockInfo(); var ok = true; try { - Lock(lockInfo); + _logger.Debug("SetAllFastSorted"); + + //Lock(lockInfo); ClearLocked(_contentNodes); ClearRootLocked(); @@ -665,7 +705,7 @@ namespace Umbraco.Web.PublishedCache.NuCache previousNode = null; // there is no previous sibling } - _logger.Debug($"Set {thisNode.Id} with parent {thisNode.ParentContentId}"); + _logger.Verbose($"Set {thisNode.Id} with parent {thisNode.ParentContentId}"); SetValueLocked(_contentNodes, thisNode.Id, thisNode); // if we are initializing from the database source ensure the local db is updated @@ -689,7 +729,7 @@ namespace Umbraco.Web.PublishedCache.NuCache } finally { - Release(lockInfo); + //Release(lockInfo); } return ok; @@ -697,11 +737,15 @@ namespace Umbraco.Web.PublishedCache.NuCache public bool SetAll(IEnumerable kits) { - var lockInfo = new WriteLockInfo(); + //TODO: There should be NO locks here all calls made to this are done within GetScopedWriteLock + + //var lockInfo = new WriteLockInfo(); var ok = true; try { - Lock(lockInfo); + _logger.Debug("SetAll"); + + //Lock(lockInfo); ClearLocked(_contentNodes); ClearRootLocked(); @@ -717,7 +761,7 @@ namespace Umbraco.Web.PublishedCache.NuCache ok = false; continue; // skip that one } - _logger.Debug($"Set {kit.Node.Id} with parent {kit.Node.ParentContentId}"); + _logger.Verbose($"Set {kit.Node.Id} with parent {kit.Node.ParentContentId}"); SetValueLocked(_contentNodes, kit.Node.Id, kit.Node); if (_localDb != null) RegisterChange(kit.Node.Id, kit); @@ -728,7 +772,7 @@ namespace Umbraco.Web.PublishedCache.NuCache } finally { - Release(lockInfo); + //Release(lockInfo); } return ok; @@ -737,11 +781,15 @@ namespace Umbraco.Web.PublishedCache.NuCache // IMPORTANT kits must be sorted out by LEVEL and by SORT ORDER public bool SetBranch(int rootContentId, IEnumerable kits) { - var lockInfo = new WriteLockInfo(); + //TODO: There should be NO locks here all calls made to this are done within GetScopedWriteLock + + //var lockInfo = new WriteLockInfo(); var ok = true; try { - Lock(lockInfo); + _logger.Debug("SetBranch"); + + //Lock(lockInfo); // get existing _contentNodes.TryGetValue(rootContentId, out var link); @@ -773,7 +821,7 @@ namespace Umbraco.Web.PublishedCache.NuCache } finally { - Release(lockInfo); + //Release(lockInfo); } return ok; @@ -781,10 +829,13 @@ namespace Umbraco.Web.PublishedCache.NuCache public bool Clear(int id) { - var lockInfo = new WriteLockInfo(); + // TODO: There should be NO locks here! All calls to this are made within GetScopedWriteLock + //var lockInfo = new WriteLockInfo(); try { - Lock(lockInfo); + _logger.Debug("Clear"); + + //Lock(lockInfo); // try to find the content // if it is not there, nothing to do @@ -804,7 +855,7 @@ namespace Umbraco.Web.PublishedCache.NuCache } finally { - Release(lockInfo); + //Release(lockInfo); } } @@ -1200,6 +1251,8 @@ namespace Umbraco.Web.PublishedCache.NuCache var lockInfo = new ReadLockInfo(); try { + // TODO: This would be much simpler with just a lock(_rlocko) { } + // in this case I see no reason why we are using this syntax?! Lock(lockInfo); // if no next generation is required, and we already have one, @@ -1374,6 +1427,8 @@ namespace Umbraco.Web.PublishedCache.NuCache } } + // TODO: This is never used? Should it be? + public async Task WaitForPendingCollect() { Task task; diff --git a/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshot.cs b/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshot.cs index 3f5c1aa4d5..bbb84fc650 100644 --- a/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshot.cs +++ b/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshot.cs @@ -39,7 +39,15 @@ namespace Umbraco.Web.PublishedCache.NuCache public void Resync() { + // This is annoying since this can cause deadlocks. Since this will be cleared if something happens in the same + // thread after that calls Elements, it will re-lock the _storesLock but that might already be locked again + // and since we're most likely in a ContentStore write lock, the other thread is probably wanting that one too. + // no lock - published snapshots are single-thread + + // TODO: Instead of clearing, we could hold this value in another var in case the above call to GetElements() Or potentially + // a new TryGetElements() fails (since we might be shutting down). In that case we can use the old value. + _elements?.Dispose(); _elements = null; } diff --git a/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs b/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs index a866297d72..2439633005 100755 --- a/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs +++ b/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs @@ -190,10 +190,15 @@ namespace Umbraco.Web.PublishedCache.NuCache /// private void MainDomRelease() { + _logger.Debug("Releasing from MainDom..."); + lock (_storesLock) { + _logger.Debug("Releasing content store..."); _contentStore?.ReleaseLocalDb(); //null check because we could shut down before being assigned _localContentDb = null; + + _logger.Debug("Releasing media store..."); _mediaStore?.ReleaseLocalDb(); //null check because we could shut down before being assigned _localMediaDb = null; @@ -663,10 +668,20 @@ namespace Umbraco.Web.PublishedCache.NuCache publishedChanged = publishedChanged2; } + // TODO: These resync's are a problem, they cause deadlocks because when this is called, it's generally called within a writelock + // and then we clear out the snapshot and then if there's some event handler that needs the content cache, it tries to re-get it + // which first tries locking on the _storesLock which may have already been acquired and in this case we deadlock because + // we're still holding the write lock. + // We resync at the end of a ScopedWriteLock + // BUT if we don't resync here then the current snapshot is out of date for any event handlers that wish to use the most up to date + // data... + // so we need to figure out how to deal with the _storesLock + if (draftChanged || publishedChanged) ((PublishedSnapshot)CurrentPublishedSnapshot)?.Resync(); } + // Calling this method means we have a lock on the contentStore (i.e. GetScopedWriteLock) private void NotifyLocked(IEnumerable payloads, out bool draftChanged, out bool publishedChanged) { publishedChanged = false; @@ -676,7 +691,7 @@ namespace Umbraco.Web.PublishedCache.NuCache // content (and content types) are read-locked while reading content // contentStore is wlocked (so readable, only no new views) // and it can be wlocked by 1 thread only at a time - // contentStore is write-locked during changes + // contentStore is write-locked during changes - see note above, calls to this method are wrapped in contentStore.GetScopedWriteLock foreach (var payload in payloads) { @@ -1131,6 +1146,12 @@ namespace Umbraco.Web.PublishedCache.NuCache ContentStore.Snapshot contentSnap, mediaSnap; SnapDictionary.Snapshot domainSnap; IAppCache elementsCache; + + // TODO: Idea... TryGetElements? Might check if we are shutting down and return false and callers to this could handle it? + // Else does a readerwriterlockslim work here? (i don't think so) + // Else we have 2x locks, one for startup/shutdown, the other for getting elements and then we can maybe do a Monitor.Try? + // That is sort of the same as a TryGetElements + lock (_storesLock) { var scopeContext = _scopeProvider.Context; @@ -1156,6 +1177,8 @@ namespace Umbraco.Web.PublishedCache.NuCache // elements // just need to make sure nothing gets elements in another enlisted action... so using // a MaxValue to make sure this one runs last, and it should be ok + // ... else there is potential to deadlock since this would recursively go back into trying + // lock on _storesLock but another thread may have already tried that scopeContext.Enlist("Umbraco.Web.PublishedCache.NuCache.PublishedSnapshotService.Resync", () => this, (completed, svc) => { ((PublishedSnapshot)svc.CurrentPublishedSnapshot)?.Resync(); diff --git a/src/Umbraco.Web/PublishedCache/NuCache/notes.txt b/src/Umbraco.Web/PublishedCache/NuCache/readme.md similarity index 70% rename from src/Umbraco.Web/PublishedCache/NuCache/notes.txt rename to src/Umbraco.Web/PublishedCache/NuCache/readme.md index ff2d8dd48b..c8e8a363e9 100644 --- a/src/Umbraco.Web/PublishedCache/NuCache/notes.txt +++ b/src/Umbraco.Web/PublishedCache/NuCache/readme.md @@ -1,10 +1,6 @@ NuCache Documentation ====================== -NOTE - RENAMING -Facade = PublishedSnapshot -and everything needs to be renamed accordingly - HOW IT WORKS ------------- @@ -22,12 +18,12 @@ When reading the cache, we read views up the chain until we find a value (which null) for the given id, and finally we read the store itself. -The FacadeService manages a ContentStore for content, and another for media. -When a Facade is created, the FacadeService gets ContentView objects from the stores. +The PublishedSnapshotService manages a ContentStore for content, and another for media. +When a PublishedSnapshot is created, the PublishedSnapshotService gets ContentView objects from the stores. Views provide an immutable snapshot of the content and media. -When the FacadeService is notified of changes, it notifies the stores. -Then it resync the current Facade, so that it requires new views, etc. +When the PublishedSnapshotService is notified of changes, it notifies the stores. +Then it resync the current PublishedSnapshot, so that it requires new views, etc. Whenever a content, media or member is modified or removed, the cmsContentNu table is updated with a json dictionary of alias => property value, so that a content, @@ -50,32 +46,32 @@ Each ContentStore has a _freezeLock object used to protect the 'Frozen' state of the store. It's a disposable object that releases the lock when disposed, so usage would be: using (store.Frozen) { ... }. -The FacadeService has a _storesLock object used to guarantee atomic access to the +The PublishedSnapshotService has a _storesLock object used to guarantee atomic access to the set of content, media stores. CACHE ------ -For each set of views, the FacadeService creates a SnapshotCache. So a SnapshotCache +For each set of views, the PublishedSnapshotService creates a SnapshotCache. So a SnapshotCache is valid until anything changes in the content or media trees. In other words, things that go in the SnapshotCache stay until a content or media is modified. -For each Facade, the FacadeService creates a FacadeCache. So a FacadeCache is valid -for the duration of the Facade (usually, the request). In other words, things that go -in the FacadeCache stay (and are visible to) for the duration of the request only. +For each PublishedSnapshot, the PublishedSnapshotService creates a PublishedSnapshotCache. So a PublishedSnapshotCache is valid +for the duration of the PublishedSnapshot (usually, the request). In other words, things that go +in the PublishedSnapshotCache stay (and are visible to) for the duration of the request only. -The FacadeService defines a static constant FullCacheWhenPreviewing, that defines +The PublishedSnapshotService defines a static constant FullCacheWhenPreviewing, that defines how caches operate when previewing: - when true, the caches in preview mode work normally. -- when false, everything that would go to the SnapshotCache goes to the FacadeCache. +- when false, everything that would go to the SnapshotCache goes to the PublishedSnapshotCache. At the moment it is true in the code, which means that eg converted values for previewed content will go in the SnapshotCache. Makes for faster preview, but uses more memory on the long term... would need some benchmarking to figure out what is best. -Members only live for the duration of the Facade. So, for members SnapshotCache is -never used, and everything goes to the FacadeCache. +Members only live for the duration of the PublishedSnapshot. So, for members SnapshotCache is +never used, and everything goes to the PublishedSnapshotCache. All cache keys are computed in the CacheKeys static class. @@ -85,15 +81,15 @@ TESTS For testing purposes the following mechanisms exist: -The Facade type has a static Current property that is used to obtain the 'current' -facade in many places, going through the PublishedCachesServiceResolver to get the -current service, and asking the current service for the current facade, which by +The PublishedSnapshot type has a static Current property that is used to obtain the 'current' +PublishedSnapshot in many places, going through the PublishedCachesServiceResolver to get the +current service, and asking the current service for the current PublishedSnapshot, which by default relies on UmbracoContext. For test purposes, it is possible to override the -entire mechanism by defining Facade.GetCurrentFacadeFunc which should return a facade. +entire mechanism by defining PublishedSnapshot.GetCurrentPublishedSnapshotFunc which should return a PublishedSnapshot. A PublishedContent keeps only id-references to its parent and children, and needs a way to retrieve the actual objects from the cache - which depends on the current -facade. It is possible to override the entire mechanism by defining PublishedContent. +PublishedSnapshot. It is possible to override the entire mechanism by defining PublishedContent. GetContentByIdFunc or .GetMediaByIdFunc. Setting these functions must be done before Resolution is frozen. @@ -110,7 +106,7 @@ possible to support detached contents & properties, even those that do not have int id, but again this should be refactored entirely anyway. Not doing any row-version checks (see XmlStore) when reloading from database, though it -is maintained in the database. Two FIXME in FacadeService. Should we do it? +is maintained in the database. Two FIXME in PublishedSnapshotService. Should we do it? There is no on-disk cache at all so everything is reloaded from the cmsContentNu table when the site restarts. This is pretty fast, but we should experiment with solutions to @@ -121,4 +117,4 @@ PublishedMember exposes properties that IPublishedContent does not, and that are to be lost soon as the member is wrapped (content set, model...) - so we probably need some sort of IPublishedMember. -/ \ No newline at end of file +/ diff --git a/src/Umbraco.Web/Umbraco.Web.csproj b/src/Umbraco.Web/Umbraco.Web.csproj index 5d29e53d4a..861b95691b 100755 --- a/src/Umbraco.Web/Umbraco.Web.csproj +++ b/src/Umbraco.Web/Umbraco.Web.csproj @@ -1216,7 +1216,7 @@ - + @@ -1279,4 +1279,4 @@ - + \ No newline at end of file diff --git a/src/Umbraco.Web/UmbracoApplication.cs b/src/Umbraco.Web/UmbracoApplication.cs index ad8dcb7e8b..f8ee238da7 100644 --- a/src/Umbraco.Web/UmbracoApplication.cs +++ b/src/Umbraco.Web/UmbracoApplication.cs @@ -22,7 +22,7 @@ namespace Umbraco.Web var mainDomLock = appSettingMainDomLock == "SqlMainDomLock" ? (IMainDomLock)new SqlMainDomLock(logger) - : new MainDomSemaphoreLock(); + : new MainDomSemaphoreLock(logger); var runtime = new WebRuntime(this, logger, new MainDom(logger, mainDomLock));