diff --git a/src/Umbraco.Core/Services/CacheInstructionServiceInitializationResult.cs b/src/Umbraco.Core/Services/CacheInstructionServiceInitializationResult.cs
deleted file mode 100644
index e7cea8ef33..0000000000
--- a/src/Umbraco.Core/Services/CacheInstructionServiceInitializationResult.cs
+++ /dev/null
@@ -1,31 +0,0 @@
-namespace Umbraco.Cms.Core.Services
-{
- ///
- /// Defines a result object for the operation.
- ///
- public class CacheInstructionServiceInitializationResult
- {
- private CacheInstructionServiceInitializationResult()
- {
- }
-
- public bool Initialized { get; private set; }
-
- public bool ColdBootRequired { get; private set; }
-
- public int MaxId { get; private set; }
-
- public int LastId { get; private set; }
-
- public static CacheInstructionServiceInitializationResult AsUninitialized() => new CacheInstructionServiceInitializationResult { Initialized = false };
-
- public static CacheInstructionServiceInitializationResult AsInitialized(bool coldBootRequired, int maxId, int lastId) =>
- new CacheInstructionServiceInitializationResult
- {
- Initialized = true,
- ColdBootRequired = coldBootRequired,
- MaxId = maxId,
- LastId = lastId,
- };
- }
-}
diff --git a/src/Umbraco.Core/Services/ICacheInstructionService.cs b/src/Umbraco.Core/Services/ICacheInstructionService.cs
index 4589053fa8..41e04e06d0 100644
--- a/src/Umbraco.Core/Services/ICacheInstructionService.cs
+++ b/src/Umbraco.Core/Services/ICacheInstructionService.cs
@@ -7,9 +7,21 @@ namespace Umbraco.Cms.Core.Services
public interface ICacheInstructionService
{
///
- /// Ensures that the cache instruction service is initialized and can be used for syncing messages.
+ /// Checks to see if a cold boot is required, either because instructions exist and none have been synced or
+ /// because the last recorded synced instruction can't be found in the database.
///
- CacheInstructionServiceInitializationResult EnsureInitialized(bool released, int lastId);
+ bool IsColdBootRequired(int lastId);
+
+ ///
+ /// Checks to see if the number of pending instructions are over the configured limit.
+ ///
+ bool IsInstructionCountOverLimit(int lastId, int limit, out int count);
+
+ ///
+ /// Gets the most recent cache instruction record Id.
+ ///
+ ///
+ int GetMaxInstructionId();
///
/// Creates a cache instruction record from a set of individual instructions and saves it.
diff --git a/src/Umbraco.Infrastructure/Services/Implement/CacheInstructionService.cs b/src/Umbraco.Infrastructure/Services/Implement/CacheInstructionService.cs
index 31c018d41c..0d4464d811 100644
--- a/src/Umbraco.Infrastructure/Services/Implement/CacheInstructionService.cs
+++ b/src/Umbraco.Infrastructure/Services/Implement/CacheInstructionService.cs
@@ -55,105 +55,51 @@ namespace Umbraco.Cms.Core.Services.Implement
}
///
- public CacheInstructionServiceInitializationResult EnsureInitialized(bool released, int lastId)
+ public bool IsColdBootRequired(int lastId)
{
using (IScope scope = ScopeProvider.CreateScope(autoComplete: true))
{
- lastId = EnsureInstructions(lastId); // reset _lastId if instructions are missing
- return Initialize(released, lastId); // boot
- }
- }
-
- ///
- /// Ensure that the last instruction that was processed is still in the database.
- ///
- ///
- /// If the last instruction is not in the database anymore, then the messenger
- /// should not try to process any instructions, because some instructions might be lost,
- /// and it should instead cold-boot.
- /// However, if the last synced instruction id is '0' and there are '0' records, then this indicates
- /// that it's a fresh site and no user actions have taken place, in this circumstance we do not want to cold
- /// boot. See: http://issues.umbraco.org/issue/U4-8627
- ///
- private int EnsureInstructions(int lastId)
- {
- if (lastId == 0)
- {
- var count = _cacheInstructionRepository.CountAll();
-
- // If there are instructions but we haven't synced, then a cold boot is necessary.
- if (count > 0)
+ if (lastId == 0)
{
- lastId = -1;
- }
- }
- else
- {
- // If the last synced instruction is not found in the db, then a cold boot is necessary.
- if (!_cacheInstructionRepository.Exists(lastId))
- {
- lastId = -1;
- }
- }
+ var count = _cacheInstructionRepository.CountAll();
- return lastId;
- }
-
- ///
- /// Initializes a server that has never synchronized before.
- ///
- ///
- /// Thread safety: this is NOT thread safe. Because it is NOT meant to run multi-threaded.
- /// Callers MUST ensure thread-safety.
- ///
- private CacheInstructionServiceInitializationResult Initialize(bool released, int lastId)
- {
- lock (_locko)
- {
- if (released)
- {
- return CacheInstructionServiceInitializationResult.AsUninitialized();
- }
-
- var coldboot = false;
-
- // Never synced before.
- if (lastId < 0)
- {
- // We haven't synced - in this case we aren't going to sync the whole thing, we will assume this is a new
- // server and it will need to rebuild it's own caches, e.g. Lucene or the XML cache file.
- _logger.LogWarning("No last synced Id found, this generally means this is a new server/install."
- + " The server will build its caches and indexes, and then adjust its last synced Id to the latest found in"
- + " the database and maintain cache updates based on that Id.");
-
- coldboot = true;
+ // If there are instructions but we haven't synced, then a cold boot is necessary.
+ if (count > 0)
+ {
+ return true;
+ }
}
else
{
- // Check for how many instructions there are to process. Each row contains a count of the number of instructions contained in each
- // row so we will sum these numbers to get the actual count.
- var count = _cacheInstructionRepository.CountPendingInstructions(lastId);
- if (count > _globalSettings.DatabaseServerMessenger.MaxProcessingInstructionCount)
+ // If the last synced instruction is not found in the db, then a cold boot is necessary.
+ if (!_cacheInstructionRepository.Exists(lastId))
{
- // Too many instructions, proceed to cold boot
- _logger.LogWarning(
- "The instruction count ({InstructionCount}) exceeds the specified MaxProcessingInstructionCount ({MaxProcessingInstructionCount})."
- + " The server will skip existing instructions, rebuild its caches and indexes entirely, adjust its last synced Id"
- + " to the latest found in the database and maintain cache updates based on that Id.",
- count, _globalSettings.DatabaseServerMessenger.MaxProcessingInstructionCount);
-
- coldboot = true;
+ return true;
}
}
- // If cold boot is required, go get the last id in the db and store it.
- // Note: do it BEFORE initializing otherwise some instructions might get lost.
- // When doing it before, some instructions might run twice - not an issue.
- var maxId = coldboot
- ? _cacheInstructionRepository.GetMaxId()
- : 0;
+ return false;
+ }
+ }
- return CacheInstructionServiceInitializationResult.AsInitialized(coldboot, maxId, lastId);
+ ///
+ public bool IsInstructionCountOverLimit(int lastId, int limit, out int count)
+ {
+ using (IScope scope = ScopeProvider.CreateScope(autoComplete: true))
+ {
+ // Check for how many instructions there are to process, each row contains a count of the number of instructions contained in each
+ // row so we will sum these numbers to get the actual count.
+ count = _cacheInstructionRepository.CountPendingInstructions(lastId);
+ return count > limit;
+ }
+ }
+
+ ///
+ public int GetMaxInstructionId()
+ {
+ using (IScope scope = ScopeProvider.CreateScope(autoComplete: true))
+ {
+ return _cacheInstructionRepository.GetMaxId();
}
}
diff --git a/src/Umbraco.Infrastructure/Sync/DatabaseServerMessenger.cs b/src/Umbraco.Infrastructure/Sync/DatabaseServerMessenger.cs
index 7d1ef9f360..2bd66d4954 100644
--- a/src/Umbraco.Infrastructure/Sync/DatabaseServerMessenger.cs
+++ b/src/Umbraco.Infrastructure/Sync/DatabaseServerMessenger.cs
@@ -168,28 +168,87 @@ namespace Umbraco.Cms.Infrastructure.Sync
ReadLastSynced(); // get _lastId
- CacheInstructionServiceInitializationResult result = CacheInstructionService.EnsureInitialized(_released, _lastId);
-
- if (result.ColdBootRequired)
+ if (CacheInstructionService.IsColdBootRequired(_lastId))
{
- // If there is a max currently, or if we've never synced.
- if (result.MaxId > 0 || result.LastId < 0)
- {
- SaveLastSynced(result.MaxId);
- }
-
- // Execute initializing callbacks.
- if (Callbacks.InitializingCallbacks != null)
- {
- foreach (Action callback in Callbacks.InitializingCallbacks)
- {
- callback();
- }
- }
+ _lastId = -1; // reset _lastId if instructions are missing
}
- return result.Initialized;
- }
+ return Initialize(); // boot
+ }
+
+ //
+ /// Initializes a server that has never synchronized before.
+ ///
+ ///
+ /// Thread safety: this is NOT thread safe. Because it is NOT meant to run multi-threaded.
+ /// Callers MUST ensure thread-safety.
+ ///
+ private bool Initialize()
+ {
+ lock (_locko)
+ {
+ if (_released)
+ {
+ return false;
+ }
+
+ var coldboot = false;
+
+ // Never synced before.
+ if (_lastId < 0)
+ {
+ // We haven't synced - in this case we aren't going to sync the whole thing, we will assume this is a new
+ // server and it will need to rebuild it's own caches, e.g. Lucene or the XML cache file.
+ Logger.LogWarning("No last synced Id found, this generally means this is a new server/install."
+ + " The server will build its caches and indexes, and then adjust its last synced Id to the latest found in"
+ + " the database and maintain cache updates based on that Id.");
+
+ coldboot = true;
+ }
+ else
+ {
+ // Check for how many instructions there are to process, each row contains a count of the number of instructions contained in each
+ // row so we will sum these numbers to get the actual count.
+ var limit = GlobalSettings.DatabaseServerMessenger.MaxProcessingInstructionCount;
+ if (CacheInstructionService.IsInstructionCountOverLimit(_lastId, limit, out int count))
+ {
+ // Too many instructions, proceed to cold boot.
+ Logger.LogWarning(
+ "The instruction count ({InstructionCount}) exceeds the specified MaxProcessingInstructionCount ({MaxProcessingInstructionCount})."
+ + " The server will skip existing instructions, rebuild its caches and indexes entirely, adjust its last synced Id"
+ + " to the latest found in the database and maintain cache updates based on that Id.",
+ count, limit);
+
+ coldboot = true;
+ }
+ }
+
+ if (coldboot)
+ {
+ // Get the last id in the db and store it.
+ // Note: Do it BEFORE initializing otherwise some instructions might get lost
+ // when doing it before. Some instructions might run twice but this is not an issue.
+ var maxId = CacheInstructionService.GetMaxInstructionId();
+
+ // If there is a max currently, or if we've never synced.
+ if (maxId > 0 || _lastId < 0)
+ {
+ SaveLastSynced(maxId);
+ }
+
+ // Execute initializing callbacks.
+ if (Callbacks.InitializingCallbacks != null)
+ {
+ foreach (Action callback in Callbacks.InitializingCallbacks)
+ {
+ callback();
+ }
+ }
+ }
+
+ return true;
+ }
+ }
///
/// Synchronize the server (throttled).
diff --git a/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/CacheInstructionServiceTests.cs b/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/CacheInstructionServiceTests.cs
index eaf92efe6c..53496e32a6 100644
--- a/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/CacheInstructionServiceTests.cs
+++ b/src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/CacheInstructionServiceTests.cs
@@ -27,66 +27,68 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services
private const string AlternateIdentity = "alternateIdentity";
[Test]
- public void Can_Ensure_Initialized_With_No_Instructions()
- {
- var sut = (CacheInstructionService)GetRequiredService();
-
- CacheInstructionServiceInitializationResult result = sut.EnsureInitialized(false, 0);
-
- Assert.Multiple(() =>
- {
- Assert.IsTrue(result.Initialized);
- Assert.IsFalse(result.ColdBootRequired);
- Assert.AreEqual(0, result.MaxId);
- Assert.AreEqual(0, result.LastId);
- });
- }
-
- [Test]
- public void Is_Not_Initialized_When_Released()
- {
- var sut = (CacheInstructionService)GetRequiredService();
-
- CacheInstructionServiceInitializationResult result = sut.EnsureInitialized(true, 0);
-
- Assert.IsFalse(result.Initialized);
- }
-
- [Test]
- public void Can_Ensure_Initialized_With_UnSynced_Instructions()
+ public void Confirms_Cold_Boot_Required_When_Instructions_Exist_And_None_Have_Been_Synced()
{
var sut = (CacheInstructionService)GetRequiredService();
List instructions = CreateInstructions();
sut.DeliverInstructions(instructions, LocalIdentity);
- CacheInstructionServiceInitializationResult result = sut.EnsureInitialized(false, 0);
+ var result = sut.IsColdBootRequired(0);
- Assert.Multiple(() =>
- {
- Assert.IsTrue(result.Initialized);
- Assert.IsTrue(result.ColdBootRequired);
- Assert.AreEqual(1, result.MaxId);
- Assert.AreEqual(-1, result.LastId);
- });
+ Assert.IsTrue(result);
}
[Test]
- public void Can_Ensure_Initialized_With_Synced_Instructions()
+ public void Confirms_Cold_Boot_Required_When_Last_Synced_Instruction_Not_Found()
{
var sut = (CacheInstructionService)GetRequiredService();
List instructions = CreateInstructions();
- sut.DeliverInstructions(instructions, LocalIdentity);
+ sut.DeliverInstructions(instructions, LocalIdentity); // will create with Id = 1
- CacheInstructionServiceInitializationResult result = sut.EnsureInitialized(false, 1);
+ var result = sut.IsColdBootRequired(2);
- Assert.Multiple(() =>
- {
- Assert.IsTrue(result.Initialized);
- Assert.IsFalse(result.ColdBootRequired);
- Assert.AreEqual(1, result.LastId);
- });
+ Assert.IsTrue(result);
+ }
+
+ [Test]
+ public void Confirms_Cold_Boot_Not_Required_When_Last_Synced_Instruction_Found()
+ {
+ var sut = (CacheInstructionService)GetRequiredService();
+
+ List instructions = CreateInstructions();
+ sut.DeliverInstructions(instructions, LocalIdentity); // will create with Id = 1
+
+ var result = sut.IsColdBootRequired(1);
+
+ Assert.IsFalse(result);
+ }
+
+ [TestCase(1, 10, false, 4)]
+ [TestCase(1, 3, true, 4)]
+ public void Confirms_Instruction_Count_Over_Limit(int lastId, int limit, bool expectedResult, int expectedCount)
+ {
+ var sut = (CacheInstructionService)GetRequiredService();
+
+ CreateAndDeliveryMultipleInstructions(sut); // 3 records, each with 2 instructions = 6.
+
+ var result = sut.IsInstructionCountOverLimit(lastId, limit, out int count);
+
+ Assert.AreEqual(expectedResult, result);
+ Assert.AreEqual(expectedCount, count);
+ }
+
+ [Test]
+ public void Can_Get_Max_Instruction_Id()
+ {
+ var sut = (CacheInstructionService)GetRequiredService();
+
+ CreateAndDeliveryMultipleInstructions(sut);
+
+ var result = sut.GetMaxInstructionId();
+
+ Assert.AreEqual(3, result);
}
[Test]
@@ -148,7 +150,7 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services
var sut = (CacheInstructionService)GetRequiredService();
// Create three instruction records, each with two instructions. First two records are for a different identity.
- CreateMultipleInstructions(sut);
+ CreateAndDeliveryMultipleInstructions(sut);
CacheInstructionServiceProcessInstructionsResult result = sut.ProcessInstructions(false, LocalIdentity, DateTime.UtcNow.AddSeconds(-1));
@@ -167,7 +169,7 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services
EnsureServerRegistered();
var sut = (CacheInstructionService)GetRequiredService();
- CreateMultipleInstructions(sut);
+ CreateAndDeliveryMultipleInstructions(sut);
CacheInstructionServiceProcessInstructionsResult result = sut.ProcessInstructions(false, LocalIdentity, DateTime.UtcNow.AddHours(-1));
@@ -179,7 +181,7 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services
{
var sut = (CacheInstructionService)GetRequiredService();
- CreateMultipleInstructions(sut);
+ CreateAndDeliveryMultipleInstructions(sut);
CacheInstructionServiceProcessInstructionsResult result = sut.ProcessInstructions(true, LocalIdentity, DateTime.UtcNow.AddSeconds(-1));
@@ -191,7 +193,7 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services
});
}
- private void CreateMultipleInstructions(CacheInstructionService sut)
+ private void CreateAndDeliveryMultipleInstructions(CacheInstructionService sut)
{
for (int i = 0; i < 3; i++)
{