Reworked initialization to keep all file read/write within the lock.

This commit is contained in:
Andy Butland
2021-03-09 18:00:31 +01:00
parent 6422a9a58c
commit 3767ba41e9
5 changed files with 174 additions and 186 deletions

View File

@@ -1,31 +0,0 @@
namespace Umbraco.Cms.Core.Services
{
/// <summary>
/// Defines a result object for the <see cref="ICacheInstructionService.EnsureInitialized(bool, int, object)"/> operation.
/// </summary>
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,
};
}
}

View File

@@ -7,9 +7,21 @@ namespace Umbraco.Cms.Core.Services
public interface ICacheInstructionService
{
/// <summary>
/// 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.
/// </summary>
CacheInstructionServiceInitializationResult EnsureInitialized(bool released, int lastId);
bool IsColdBootRequired(int lastId);
/// <summary>
/// Checks to see if the number of pending instructions are over the configured limit.
/// </summary>
bool IsInstructionCountOverLimit(int lastId, int limit, out int count);
/// <summary>
/// Gets the most recent cache instruction record Id.
/// </summary>
/// <returns></returns>
int GetMaxInstructionId();
/// <summary>
/// Creates a cache instruction record from a set of individual instructions and saves it.

View File

@@ -55,105 +55,51 @@ namespace Umbraco.Cms.Core.Services.Implement
}
/// <inheritdoc/>
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
}
}
/// <summary>
/// Ensure that the last instruction that was processed is still in the database.
/// </summary>
/// <remarks>
/// 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
/// </remarks>
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;
}
/// <summary>
/// Initializes a server that has never synchronized before.
/// </summary>
/// <remarks>
/// Thread safety: this is NOT thread safe. Because it is NOT meant to run multi-threaded.
/// Callers MUST ensure thread-safety.
/// </remarks>
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);
/// <inheritdoc/>
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;
}
}
/// <inheritdoc/>
public int GetMaxInstructionId()
{
using (IScope scope = ScopeProvider.CreateScope(autoComplete: true))
{
return _cacheInstructionRepository.GetMaxId();
}
}

View File

@@ -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
}
// <summary>
/// Initializes a server that has never synchronized before.
/// </summary>
/// <remarks>
/// Thread safety: this is NOT thread safe. Because it is NOT meant to run multi-threaded.
/// Callers MUST ensure thread-safety.
/// </remarks>
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;
}
}
/// <summary>
/// Synchronize the server (throttled).

View File

@@ -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<ICacheInstructionService>();
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<ICacheInstructionService>();
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<ICacheInstructionService>();
List<RefreshInstruction> 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<ICacheInstructionService>();
List<RefreshInstruction> 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<ICacheInstructionService>();
List<RefreshInstruction> 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<ICacheInstructionService>();
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<ICacheInstructionService>();
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<ICacheInstructionService>();
// 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<ICacheInstructionService>();
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<ICacheInstructionService>();
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++)
{