From c682bbcaf1dc3fcc19724e57224311c3a802522d Mon Sep 17 00:00:00 2001 From: Shannon Date: Thu, 31 Oct 2019 15:44:50 +1100 Subject: [PATCH 1/7] Adds notes and better logging to PublishedSnapshotService, fixes log output from HealthCheckNotifier, changes initial delay time of scheduled task to be longer. --- .../NuCache/PublishedSnapshotService.cs | 35 +++++++++++++++++-- .../Scheduling/HealthCheckNotifier.cs | 2 +- .../Scheduling/SchedulerComponent.cs | 19 ++++++---- 3 files changed, 46 insertions(+), 10 deletions(-) diff --git a/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs b/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs index 9522e76622..d7480baf67 100755 --- a/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs +++ b/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs @@ -128,10 +128,14 @@ namespace Umbraco.Web.PublishedCache.NuCache var path = GetLocalFilesPath(); var localContentDbPath = Path.Combine(path, "NuCache.Content.db"); var localMediaDbPath = Path.Combine(path, "NuCache.Media.db"); - _localDbExists = File.Exists(localContentDbPath) && File.Exists(localMediaDbPath); + var localContentDbExists = File.Exists(localContentDbPath); + var localMediaDbExists = File.Exists(localMediaDbPath); + _localDbExists = localContentDbExists && localMediaDbExists; // if both local databases exist then GetTree will open them, else new databases will be created _localContentDb = BTree.GetTree(localContentDbPath, _localDbExists); _localMediaDb = BTree.GetTree(localMediaDbPath, _localDbExists); + + _logger.Info("Registered with MainDom, localContentDbExists? {LocalContentDbExists}, localMediaDbExists? {LocalMediaDbExists}", localContentDbExists, localMediaDbExists); }, () => { @@ -143,18 +147,25 @@ namespace Umbraco.Web.PublishedCache.NuCache _localContentDb = null; _mediaStore?.ReleaseLocalDb(); //null check because we could shut down before being assigned _localMediaDb = null; + + _logger.Info("Released from MainDom"); } }); // stores are created with a db so they can write to it, but they do not read from it, // stores need to be populated, happens in OnResolutionFrozen which uses _localDbExists to // figure out whether it can read the databases or it should populate them from sql + + _logger.Info("Creating the content store, localContentDbExists? {LocalContentDbExists}", _localContentDb != null); _contentStore = new ContentStore(publishedSnapshotAccessor, variationContextAccessor, logger, _localContentDb); + _logger.Info("Creating the media store, localMediaDbExists? {LocalMediaDbExists}", _localMediaDb != null); _mediaStore = new ContentStore(publishedSnapshotAccessor, variationContextAccessor, logger, _localMediaDb); } else { + _logger.Info("Creating the content store (local db ignored)"); _contentStore = new ContentStore(publishedSnapshotAccessor, variationContextAccessor, logger); + _logger.Info("Creating the media store (local db ignored)"); _mediaStore = new ContentStore(publishedSnapshotAccessor, variationContextAccessor, logger); } @@ -371,7 +382,27 @@ namespace Umbraco.Web.PublishedCache.NuCache var kits = _localContentDb.Select(x => x.Value) .OrderBy(x => x.Node.Level) .ThenBy(x => x.Node.ParentContentId) - .ThenBy(x => x.Node.SortOrder); // IMPORTANT sort by level + parentId + sortOrder + .ThenBy(x => x.Node.SortOrder) // IMPORTANT sort by level + parentId + sortOrder + .ToList(); + + if (kits.Count == 0) + { + // If there's nothing in the local cache file, we should return false? YES even though the site legitately might be empty. + // Is it possible that the cache file is empty but the database is not? YES... + // * A new file is created when one doesn't exist, this will only be done when MainDom is acquired + // * The new file will be populated as soon as LoadCachesOnStartup is called + // * If the appdomain is going down the moment after MainDom was acquired and we've created an empty cache file, + // then the MainDom release callback is triggered from on a different thread, which will close the file and + // set the cache file reference to null. At this moment, it is possible that the file is closed and the + // reference is set to null BEFORE LoadCachesOnStartup which would mean that the current appdomain would load + // in the in-mem cache via DB calls, BUT this now means that there is an empty cache file which will be + // loaded by the next appdomain and it won't check if it's empty, it just assumes that since the cache + // file is there, that is correct. + + _logger.Info("Tried to load content from the local cache file but it was empty."); + return false; + } + return onStartup ? _contentStore.SetAllFastSorted(kits) : _contentStore.SetAll(kits); } } diff --git a/src/Umbraco.Web/Scheduling/HealthCheckNotifier.cs b/src/Umbraco.Web/Scheduling/HealthCheckNotifier.cs index 4f66af11bd..746bc61e34 100644 --- a/src/Umbraco.Web/Scheduling/HealthCheckNotifier.cs +++ b/src/Umbraco.Web/Scheduling/HealthCheckNotifier.cs @@ -51,7 +51,7 @@ namespace Umbraco.Web.Scheduling return false; // do NOT repeat, going down } - using (_logger.DebugDuration("Health checks executing", "Health checks complete")) + using (_logger.DebugDuration("Health checks executing", "Health checks complete")) { var healthCheckConfig = Current.Configs.HealthChecks(); diff --git a/src/Umbraco.Web/Scheduling/SchedulerComponent.cs b/src/Umbraco.Web/Scheduling/SchedulerComponent.cs index c334bcee1a..1416393f46 100644 --- a/src/Umbraco.Web/Scheduling/SchedulerComponent.cs +++ b/src/Umbraco.Web/Scheduling/SchedulerComponent.cs @@ -18,6 +18,11 @@ namespace Umbraco.Web.Scheduling { internal sealed class SchedulerComponent : IComponent { + private const int DefaultDelayMilliseconds = 180000; // 3 mins + private const int OneMinuteMilliseconds = 60000; + private const int FiveMinuteMilliseconds = 300000; + private const int OneHourMilliseconds = 3600000; + private readonly IRuntimeState _runtime; private readonly IContentService _contentService; private readonly IAuditService _auditService; @@ -111,7 +116,7 @@ namespace Umbraco.Web.Scheduling { // ping/keepalive // on all servers - var task = new KeepAlive(_keepAliveRunner, 60000, 300000, _runtime, _logger); + var task = new KeepAlive(_keepAliveRunner, DefaultDelayMilliseconds, FiveMinuteMilliseconds, _runtime, _logger); _keepAliveRunner.TryAdd(task); return task; } @@ -120,7 +125,7 @@ namespace Umbraco.Web.Scheduling { // scheduled publishing/unpublishing // install on all, will only run on non-replica servers - var task = new ScheduledPublishing(_publishingRunner, 60000, 60000, _runtime, _contentService, _umbracoContextFactory, _logger); + var task = new ScheduledPublishing(_publishingRunner, DefaultDelayMilliseconds, OneMinuteMilliseconds, _runtime, _contentService, _umbracoContextFactory, _logger); _publishingRunner.TryAdd(task); return task; } @@ -133,15 +138,15 @@ namespace Umbraco.Web.Scheduling int delayInMilliseconds; if (string.IsNullOrEmpty(healthCheckConfig.NotificationSettings.FirstRunTime)) { - delayInMilliseconds = 60000; + delayInMilliseconds = DefaultDelayMilliseconds; } else { // Otherwise start at scheduled time delayInMilliseconds = DateTime.Now.PeriodicMinutesFrom(healthCheckConfig.NotificationSettings.FirstRunTime) * 60 * 1000; - if (delayInMilliseconds < 60000) + if (delayInMilliseconds < DefaultDelayMilliseconds) { - delayInMilliseconds = 60000; + delayInMilliseconds = DefaultDelayMilliseconds; } } @@ -155,7 +160,7 @@ namespace Umbraco.Web.Scheduling { // log scrubbing // install on all, will only run on non-replica servers - var task = new LogScrubber(_scrubberRunner, 60000, LogScrubber.GetLogScrubbingInterval(settings, _logger), _runtime, _auditService, settings, _scopeProvider, _logger); + var task = new LogScrubber(_scrubberRunner, DefaultDelayMilliseconds, LogScrubber.GetLogScrubbingInterval(settings, _logger), _runtime, _auditService, settings, _scopeProvider, _logger); _scrubberRunner.TryAdd(task); return task; } @@ -164,7 +169,7 @@ namespace Umbraco.Web.Scheduling { // temp file cleanup, will run on all servers - even though file upload should only be handled on the master, this will // ensure that in the case it happes on replicas that they are cleaned up. - var task = new TempFileCleanup(_fileCleanupRunner, 60000, 3600000 /* 1 hr */, + var task = new TempFileCleanup(_fileCleanupRunner, DefaultDelayMilliseconds, OneHourMilliseconds, new[] { new DirectoryInfo(IOHelper.MapPath(SystemDirectories.TempFileUploads)) }, TimeSpan.FromDays(1), //files that are over a day old _runtime, _logger); From aff4814e0bcdd4c24d38ccd4716578f6d115cd03 Mon Sep 17 00:00:00 2001 From: Shannon Date: Thu, 31 Oct 2019 16:06:43 +1100 Subject: [PATCH 2/7] Ensures that PublishedSnapshotService cannot close the local cache files after they are created but before they are populated --- .../NuCache/PublishedSnapshotService.cs | 188 ++++++++++-------- 1 file changed, 101 insertions(+), 87 deletions(-) diff --git a/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs b/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs index d7480baf67..276498ec93 100755 --- a/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs +++ b/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs @@ -113,65 +113,37 @@ namespace Umbraco.Web.PublishedCache.NuCache if (runtime.Level != RuntimeLevel.Run) return; - if (options.IgnoreLocalDb == false) + // lock this entire call, we only want a single thread to be accessing the stores at once and within + // the call below to mainDom.Register, a callback may occur on a threadpool thread to MainDomRelease + // at the same time as we are trying to write to the stores. MainDomRelease also locks on _storesLock so + // it will not be able to close the stores until we are done populating (if the store is empty) + lock (_storesLock) { - var registered = mainDom.Register( - () => - { - //"install" phase of MainDom - //this is inside of a lock in MainDom so this is guaranteed to run if MainDom was acquired and guaranteed - //to not run if MainDom wasn't acquired. - //If MainDom was not acquired, then _localContentDb and _localMediaDb will remain null which means this appdomain - //will load in published content via the DB and in that case this appdomain will probably not exist long enough to - //serve more than a page of content. + if (options.IgnoreLocalDb == false) + { + var registered = mainDom.Register(MainDomRegister, MainDomRelease); - var path = GetLocalFilesPath(); - var localContentDbPath = Path.Combine(path, "NuCache.Content.db"); - var localMediaDbPath = Path.Combine(path, "NuCache.Media.db"); - var localContentDbExists = File.Exists(localContentDbPath); - var localMediaDbExists = File.Exists(localMediaDbPath); - _localDbExists = localContentDbExists && localMediaDbExists; - // if both local databases exist then GetTree will open them, else new databases will be created - _localContentDb = BTree.GetTree(localContentDbPath, _localDbExists); - _localMediaDb = BTree.GetTree(localMediaDbPath, _localDbExists); + // stores are created with a db so they can write to it, but they do not read from it, + // stores need to be populated, happens in OnResolutionFrozen which uses _localDbExists to + // figure out whether it can read the databases or it should populate them from sql - _logger.Info("Registered with MainDom, localContentDbExists? {LocalContentDbExists}, localMediaDbExists? {LocalMediaDbExists}", localContentDbExists, localMediaDbExists); - }, - () => - { - //"release" phase of MainDom + _logger.Info("Creating the content store, localContentDbExists? {LocalContentDbExists}", _localContentDb != null); + _contentStore = new ContentStore(publishedSnapshotAccessor, variationContextAccessor, logger, _localContentDb); + _logger.Info("Creating the media store, localMediaDbExists? {LocalMediaDbExists}", _localMediaDb != null); + _mediaStore = new ContentStore(publishedSnapshotAccessor, variationContextAccessor, logger, _localMediaDb); + } + else + { + _logger.Info("Creating the content store (local db ignored)"); + _contentStore = new ContentStore(publishedSnapshotAccessor, variationContextAccessor, logger); + _logger.Info("Creating the media store (local db ignored)"); + _mediaStore = new ContentStore(publishedSnapshotAccessor, variationContextAccessor, logger); + } - lock (_storesLock) - { - _contentStore?.ReleaseLocalDb(); //null check because we could shut down before being assigned - _localContentDb = null; - _mediaStore?.ReleaseLocalDb(); //null check because we could shut down before being assigned - _localMediaDb = null; + _domainStore = new SnapDictionary(); - _logger.Info("Released from MainDom"); - } - }); - - // stores are created with a db so they can write to it, but they do not read from it, - // stores need to be populated, happens in OnResolutionFrozen which uses _localDbExists to - // figure out whether it can read the databases or it should populate them from sql - - _logger.Info("Creating the content store, localContentDbExists? {LocalContentDbExists}", _localContentDb != null); - _contentStore = new ContentStore(publishedSnapshotAccessor, variationContextAccessor, logger, _localContentDb); - _logger.Info("Creating the media store, localMediaDbExists? {LocalMediaDbExists}", _localMediaDb != null); - _mediaStore = new ContentStore(publishedSnapshotAccessor, variationContextAccessor, logger, _localMediaDb); + LoadCachesOnStartup(); } - else - { - _logger.Info("Creating the content store (local db ignored)"); - _contentStore = new ContentStore(publishedSnapshotAccessor, variationContextAccessor, logger); - _logger.Info("Creating the media store (local db ignored)"); - _mediaStore = new ContentStore(publishedSnapshotAccessor, variationContextAccessor, logger); - } - - _domainStore = new SnapDictionary(); - - LoadCachesOnStartup(); Guid GetUid(ContentStore store, int id) => store.LiveSnapshot.Get(id)?.Uid ?? default; int GetId(ContentStore store, Guid uid) => store.LiveSnapshot.Get(uid)?.Id ?? default; @@ -183,47 +155,89 @@ namespace Umbraco.Web.PublishedCache.NuCache } } - private void LoadCachesOnStartup() + /// + /// Install phase of + /// + /// + /// This is inside of a lock in MainDom so this is guaranteed to run if MainDom was acquired and guaranteed + /// to not run if MainDom wasn't acquired. + /// If MainDom was not acquired, then _localContentDb and _localMediaDb will remain null which means this appdomain + /// will load in published content via the DB and in that case this appdomain will probably not exist long enough to + /// serve more than a page of content. + /// + private void MainDomRegister() + { + var path = GetLocalFilesPath(); + var localContentDbPath = Path.Combine(path, "NuCache.Content.db"); + var localMediaDbPath = Path.Combine(path, "NuCache.Media.db"); + var localContentDbExists = File.Exists(localContentDbPath); + var localMediaDbExists = File.Exists(localMediaDbPath); + _localDbExists = localContentDbExists && localMediaDbExists; + // if both local databases exist then GetTree will open them, else new databases will be created + _localContentDb = BTree.GetTree(localContentDbPath, _localDbExists); + _localMediaDb = BTree.GetTree(localMediaDbPath, _localDbExists); + + _logger.Info("Registered with MainDom, localContentDbExists? {LocalContentDbExists}, localMediaDbExists? {LocalMediaDbExists}", localContentDbExists, localMediaDbExists); + } + + /// + /// Release phase of MainDom + /// + /// + /// This will execute on a threadpool thread + /// + private void MainDomRelease() { lock (_storesLock) { - // populate the stores + _contentStore?.ReleaseLocalDb(); //null check because we could shut down before being assigned + _localContentDb = null; + _mediaStore?.ReleaseLocalDb(); //null check because we could shut down before being assigned + _localMediaDb = null; - - var okContent = false; - var okMedia = false; - - try - { - if (_localDbExists) - { - okContent = LockAndLoadContent(scope => LoadContentFromLocalDbLocked(true)); - if (!okContent) - _logger.Warn("Loading content from local db raised warnings, will reload from database."); - okMedia = LockAndLoadMedia(scope => LoadMediaFromLocalDbLocked(true)); - if (!okMedia) - _logger.Warn("Loading media from local db raised warnings, will reload from database."); - } - - if (!okContent) - LockAndLoadContent(scope => LoadContentFromDatabaseLocked(scope, true)); - - if (!okMedia) - LockAndLoadMedia(scope => LoadMediaFromDatabaseLocked(scope, true)); - - LockAndLoadDomains(); - } - catch (Exception ex) - { - _logger.Fatal(ex, "Panic, exception while loading cache data."); - throw; - } - - // finally, cache is ready! - _isReady = true; + _logger.Info("Released from MainDom"); } } + /// + /// Populates the stores + /// + /// This is called inside of a lock for _storesLock + private void LoadCachesOnStartup() + { + var okContent = false; + var okMedia = false; + + try + { + if (_localDbExists) + { + okContent = LockAndLoadContent(scope => LoadContentFromLocalDbLocked(true)); + if (!okContent) + _logger.Warn("Loading content from local db raised warnings, will reload from database."); + okMedia = LockAndLoadMedia(scope => LoadMediaFromLocalDbLocked(true)); + if (!okMedia) + _logger.Warn("Loading media from local db raised warnings, will reload from database."); + } + + if (!okContent) + LockAndLoadContent(scope => LoadContentFromDatabaseLocked(scope, true)); + + if (!okMedia) + LockAndLoadMedia(scope => LoadMediaFromDatabaseLocked(scope, true)); + + LockAndLoadDomains(); + } + catch (Exception ex) + { + _logger.Fatal(ex, "Panic, exception while loading cache data."); + throw; + } + + // finally, cache is ready! + _isReady = true; + } + private void InitializeRepositoryEvents() { // TODO: The reason these events are in the repository is for legacy, the events should exist at the service From 40e656de46d1a6e14d44d1ba555f50c6534e9c59 Mon Sep 17 00:00:00 2001 From: Shannon Date: Thu, 31 Oct 2019 16:27:34 +1100 Subject: [PATCH 3/7] notes --- .../PublishedCache/NuCache/PublishedSnapshotService.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs b/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs index 276498ec93..533c13a992 100755 --- a/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs +++ b/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs @@ -402,7 +402,7 @@ namespace Umbraco.Web.PublishedCache.NuCache if (kits.Count == 0) { // If there's nothing in the local cache file, we should return false? YES even though the site legitately might be empty. - // Is it possible that the cache file is empty but the database is not? YES... + // Is it possible that the cache file is empty but the database is not? YES... (well, it used to be possible) // * A new file is created when one doesn't exist, this will only be done when MainDom is acquired // * The new file will be populated as soon as LoadCachesOnStartup is called // * If the appdomain is going down the moment after MainDom was acquired and we've created an empty cache file, @@ -413,6 +413,9 @@ namespace Umbraco.Web.PublishedCache.NuCache // loaded by the next appdomain and it won't check if it's empty, it just assumes that since the cache // file is there, that is correct. + // Update: We will still return false here even though the above mentioned race condition has been fixed since we now + // lock the entire operation of creating/populating the cache file with the same lock as releasing/closing the cache file + _logger.Info("Tried to load content from the local cache file but it was empty."); return false; } From bf37bbf4fef36d3dacbd93afa2f6c259ecccff65 Mon Sep 17 00:00:00 2001 From: Shannon Date: Thu, 31 Oct 2019 16:34:30 +1100 Subject: [PATCH 4/7] oops fixes the log output --- .../PublishedCache/NuCache/PublishedSnapshotService.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs b/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs index 533c13a992..5801e0b3e7 100755 --- a/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs +++ b/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs @@ -127,9 +127,9 @@ namespace Umbraco.Web.PublishedCache.NuCache // stores need to be populated, happens in OnResolutionFrozen which uses _localDbExists to // figure out whether it can read the databases or it should populate them from sql - _logger.Info("Creating the content store, localContentDbExists? {LocalContentDbExists}", _localContentDb != null); + _logger.Info("Creating the content store, localContentDbExists? {LocalContentDbExists}", _localDbExists); _contentStore = new ContentStore(publishedSnapshotAccessor, variationContextAccessor, logger, _localContentDb); - _logger.Info("Creating the media store, localMediaDbExists? {LocalMediaDbExists}", _localMediaDb != null); + _logger.Info("Creating the media store, localMediaDbExists? {LocalMediaDbExists}", _localDbExists); _mediaStore = new ContentStore(publishedSnapshotAccessor, variationContextAccessor, logger, _localMediaDb); } else From 68db41fe9358babff39b9954b95489e7f4e4d4cc Mon Sep 17 00:00:00 2001 From: Shannon Date: Thu, 31 Oct 2019 18:58:07 +1100 Subject: [PATCH 5/7] Ensures that when there is no cache file, or if a cache file is empty, on startup any call to SetAllFastSorted was not ensuring the local db cache file was updated if the data was loaded from the database. --- .../PublishedCache/NuCache/ContentStore.cs | 6 +- .../NuCache/PublishedSnapshotService.cs | 71 ++++++++++--------- 2 files changed, 41 insertions(+), 36 deletions(-) diff --git a/src/Umbraco.Web/PublishedCache/NuCache/ContentStore.cs b/src/Umbraco.Web/PublishedCache/NuCache/ContentStore.cs index f71abd6aa7..64bffc7a49 100644 --- a/src/Umbraco.Web/PublishedCache/NuCache/ContentStore.cs +++ b/src/Umbraco.Web/PublishedCache/NuCache/ContentStore.cs @@ -601,13 +601,14 @@ namespace Umbraco.Web.PublishedCache.NuCache /// /// All kits sorted by Level + Parent Id + Sort order /// + /// True if the data is coming from the database (not the local cache db) /// /// /// This requires that the collection is sorted by Level + ParentId + Sort Order. /// This should be used only on a site startup as the first generations. /// This CANNOT be used after startup since it bypasses all checks for Generations. /// - internal bool SetAllFastSorted(IEnumerable kits) + internal bool SetAllFastSorted(IEnumerable kits, bool fromDb) { var lockInfo = new WriteLockInfo(); var ok = true; @@ -654,6 +655,9 @@ namespace Umbraco.Web.PublishedCache.NuCache _logger.Debug($"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 + if (fromDb && _localDb != null) RegisterChange(thisNode.Id, kit); + // this node is always the last child parent.LastChildContentId = thisNode.Id; diff --git a/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs b/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs index 5801e0b3e7..bf6012816a 100755 --- a/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs +++ b/src/Umbraco.Web/PublishedCache/NuCache/PublishedSnapshotService.cs @@ -378,7 +378,7 @@ namespace Umbraco.Web.PublishedCache.NuCache // IMPORTANT GetAllContentSources sorts kits by level + parentId + sortOrder var kits = _dataSource.GetAllContentSources(scope); - return onStartup ? _contentStore.SetAllFastSorted(kits) : _contentStore.SetAll(kits); + return onStartup ? _contentStore.SetAllFastSorted(kits, true) : _contentStore.SetAll(kits); } } @@ -393,34 +393,7 @@ namespace Umbraco.Web.PublishedCache.NuCache // beware! at that point the cache is inconsistent, // assuming we are going to SetAll content items! - var kits = _localContentDb.Select(x => x.Value) - .OrderBy(x => x.Node.Level) - .ThenBy(x => x.Node.ParentContentId) - .ThenBy(x => x.Node.SortOrder) // IMPORTANT sort by level + parentId + sortOrder - .ToList(); - - if (kits.Count == 0) - { - // If there's nothing in the local cache file, we should return false? YES even though the site legitately might be empty. - // Is it possible that the cache file is empty but the database is not? YES... (well, it used to be possible) - // * A new file is created when one doesn't exist, this will only be done when MainDom is acquired - // * The new file will be populated as soon as LoadCachesOnStartup is called - // * If the appdomain is going down the moment after MainDom was acquired and we've created an empty cache file, - // then the MainDom release callback is triggered from on a different thread, which will close the file and - // set the cache file reference to null. At this moment, it is possible that the file is closed and the - // reference is set to null BEFORE LoadCachesOnStartup which would mean that the current appdomain would load - // in the in-mem cache via DB calls, BUT this now means that there is an empty cache file which will be - // loaded by the next appdomain and it won't check if it's empty, it just assumes that since the cache - // file is there, that is correct. - - // Update: We will still return false here even though the above mentioned race condition has been fixed since we now - // lock the entire operation of creating/populating the cache file with the same lock as releasing/closing the cache file - - _logger.Info("Tried to load content from the local cache file but it was empty."); - return false; - } - - return onStartup ? _contentStore.SetAllFastSorted(kits) : _contentStore.SetAll(kits); + return LoadEntitiesFromLocalDbLocked(onStartup, _localContentDb, _contentStore, "content"); } } @@ -477,7 +450,7 @@ namespace Umbraco.Web.PublishedCache.NuCache _logger.Debug("Loading media from database..."); // IMPORTANT GetAllMediaSources sorts kits by level + parentId + sortOrder var kits = _dataSource.GetAllMediaSources(scope); - return onStartup ? _mediaStore.SetAllFastSorted(kits) : _mediaStore.SetAll(kits); + return onStartup ? _mediaStore.SetAllFastSorted(kits, true) : _mediaStore.SetAll(kits); } } @@ -492,15 +465,43 @@ namespace Umbraco.Web.PublishedCache.NuCache // beware! at that point the cache is inconsistent, // assuming we are going to SetAll content items! - var kits = _localMediaDb.Select(x => x.Value) - .OrderBy(x => x.Node.Level) - .ThenBy(x => x.Node.ParentContentId) - .ThenBy(x => x.Node.SortOrder); // IMPORTANT sort by level + parentId + sortOrder - return onStartup ? _mediaStore.SetAllFastSorted(kits) : _mediaStore.SetAll(kits); + return LoadEntitiesFromLocalDbLocked(onStartup, _localMediaDb, _mediaStore, "media"); } } + private bool LoadEntitiesFromLocalDbLocked(bool onStartup, BPlusTree localDb, ContentStore store, string entityType) + { + var kits = localDb.Select(x => x.Value) + .OrderBy(x => x.Node.Level) + .ThenBy(x => x.Node.ParentContentId) + .ThenBy(x => x.Node.SortOrder) // IMPORTANT sort by level + parentId + sortOrder + .ToList(); + + if (kits.Count == 0) + { + // If there's nothing in the local cache file, we should return false? YES even though the site legitately might be empty. + // Is it possible that the cache file is empty but the database is not? YES... (well, it used to be possible) + // * A new file is created when one doesn't exist, this will only be done when MainDom is acquired + // * The new file will be populated as soon as LoadCachesOnStartup is called + // * If the appdomain is going down the moment after MainDom was acquired and we've created an empty cache file, + // then the MainDom release callback is triggered from on a different thread, which will close the file and + // set the cache file reference to null. At this moment, it is possible that the file is closed and the + // reference is set to null BEFORE LoadCachesOnStartup which would mean that the current appdomain would load + // in the in-mem cache via DB calls, BUT this now means that there is an empty cache file which will be + // loaded by the next appdomain and it won't check if it's empty, it just assumes that since the cache + // file is there, that is correct. + + // Update: We will still return false here even though the above mentioned race condition has been fixed since we now + // lock the entire operation of creating/populating the cache file with the same lock as releasing/closing the cache file + + _logger.Info($"Tried to load {entityType} from the local cache file but it was empty."); + return false; + } + + return onStartup ? store.SetAllFastSorted(kits, false) : store.SetAll(kits); + } + // keep these around - might be useful //private void LoadMediaBranch(IMedia media) From 6690c2ad279640f7d3ad0b4f64b7f07fb49d2325 Mon Sep 17 00:00:00 2001 From: Kenn Jacobsen Date: Tue, 29 Oct 2019 21:48:39 +0100 Subject: [PATCH 6/7] Fix broken RTEs when reordering grid rows --- .../propertyeditors/grid/grid.controller.js | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/grid/grid.controller.js b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/grid/grid.controller.js index ccc390252b..ac14dc834d 100644 --- a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/grid/grid.controller.js +++ b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/grid/grid.controller.js @@ -72,7 +72,10 @@ angular.module("umbraco") ui.item.find(".umb-rte").each(function (key, value) { // remove all RTEs in the dragged row and save their settings var rteId = value.id; - draggedRteSettings[rteId] = _.findWhere(tinyMCE.editors, { id: rteId }).settings; + var editor = _.findWhere(tinyMCE.editors, { id: rteId }); + if (editor) { + draggedRteSettings[rteId] = editor.settings; + } }); }, @@ -84,9 +87,17 @@ angular.module("umbraco") // reset all RTEs affected by the dragging ui.item.parents(".umb-column").find(".umb-rte").each(function (key, value) { var rteId = value.id; - draggedRteSettings[rteId] = draggedRteSettings[rteId] || _.findWhere(tinyMCE.editors, { id: rteId }).settings; - tinyMCE.execCommand("mceRemoveEditor", false, rteId); - tinyMCE.init(draggedRteSettings[rteId]); + var settings = draggedRteSettings[rteId]; + if (!settings) { + var editor = _.findWhere(tinyMCE.editors, { id: rteId }); + if (editor) { + settings = editor.settings; + } + } + if (settings) { + tinyMCE.execCommand("mceRemoveEditor", false, rteId); + tinyMCE.init(settings); + } }); currentForm.$setDirty(); } From ef38fc1a6d63fea945ca0b1c564e2373b751141c Mon Sep 17 00:00:00 2001 From: Bjarke Berg Date: Thu, 31 Oct 2019 10:02:08 +0100 Subject: [PATCH 7/7] Bump version to 8.2.2 --- src/SolutionInfo.cs | 4 ++-- src/Umbraco.Web.UI/Umbraco.Web.UI.csproj | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/SolutionInfo.cs b/src/SolutionInfo.cs index eccecd359c..31a9953b23 100644 --- a/src/SolutionInfo.cs +++ b/src/SolutionInfo.cs @@ -18,5 +18,5 @@ using System.Resources; [assembly: AssemblyVersion("8.0.0")] // these are FYI and changed automatically -[assembly: AssemblyFileVersion("8.2.1")] -[assembly: AssemblyInformationalVersion("8.2.1")] +[assembly: AssemblyFileVersion("8.2.2")] +[assembly: AssemblyInformationalVersion("8.2.2")] diff --git a/src/Umbraco.Web.UI/Umbraco.Web.UI.csproj b/src/Umbraco.Web.UI/Umbraco.Web.UI.csproj index 837d632da9..10ca65357d 100644 --- a/src/Umbraco.Web.UI/Umbraco.Web.UI.csproj +++ b/src/Umbraco.Web.UI/Umbraco.Web.UI.csproj @@ -345,9 +345,9 @@ False True - 8210 + 8220 / - http://localhost:8210/ + http://localhost:8220/ False False