yv01p/Umbraco-CMS
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Only process each CacheInstruction once.

Browse Source
This commit is contained in:
Mole
2021-04-12 13:27:25 +02:00
parent 3dd6fcf782
commit b117f006c0
4 changed files with 99 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
src/Umbraco.Core/Services/ICacheInstructionService.cs
View File

@@ -39,6 +39,7 @@ namespace Umbraco.Cms.Core.Services
/// <param name="released">Flag indicating if process is shutting now and operations should exit.</param>
/// <param name="localIdentity">Local identity of the executing AppDomain.</param>
/// <param name="lastPruned">Date of last prune operation.</param>
CacheInstructionServiceProcessInstructionsResult ProcessInstructions(bool released, string localIdentity, DateTime lastPruned);
/// <param name="lastId">Id of the latest processed instruction</param>
CacheInstructionServiceProcessInstructionsResult ProcessInstructions(bool released, string localIdentity, DateTime lastPruned, int lastId);
}
}

9
src/Umbraco.Infrastructure/Services/Implement/CacheInstructionService.cs
View File

@@ -133,12 +133,12 @@ namespace Umbraco.Cms.Core.Services.Implement
new CacheInstruction(0, DateTime.UtcNow, JsonConvert.SerializeObject(instructions, Formatting.None), localIdentity, instructions.Sum(x => x.JsonIdCount));
/// <inheritdoc/>
public CacheInstructionServiceProcessInstructionsResult ProcessInstructions(bool released, string localIdentity, DateTime lastPruned)
public CacheInstructionServiceProcessInstructionsResult ProcessInstructions(bool released, string localIdentity, DateTime lastPruned, int lastId)
{
using (_profilingLogger.DebugDuration<CacheInstructionService>("Syncing from database..."))
using (IScope scope = ScopeProvider.CreateScope())
{
var numberOfInstructionsProcessed = ProcessDatabaseInstructions(released, localIdentity, out int lastId);
var numberOfInstructionsProcessed = ProcessDatabaseInstructions(released, localIdentity, ref lastId);
// Check for pruning throttling.
if (released || (DateTime.UtcNow - lastPruned) <= _globalSettings.DatabaseServerMessenger.TimeBetweenPruneOperations)
@@ -172,7 +172,7 @@ namespace Umbraco.Cms.Core.Services.Implement
/// Thread safety: this is NOT thread safe. Because it is NOT meant to run multi-threaded.
/// </remarks>
/// <returns>Number of instructions processed.</returns>
private int ProcessDatabaseInstructions(bool released, string localIdentity, out int lastId)
private int ProcessDatabaseInstructions(bool released, string localIdentity, ref int lastId)
{
// NOTE:
// We 'could' recurse to ensure that no remaining instructions are pending in the table before proceeding but I don't think that
@@ -199,8 +199,9 @@ namespace Umbraco.Cms.Core.Services.Implement
// It would have been nice to do this in a Query instead of Fetch using a data reader to save
// some memory however we cannot do that because inside of this loop the cache refreshers are also
// performing some lookups which cannot be done with an active reader open.
IEnumerable<CacheInstruction> pendingInstructions = _cacheInstructionRepository.GetPendingInstructions(lastId, MaxInstructionsToRetrieve);
lastId = 0;
foreach (CacheInstruction instruction in _cacheInstructionRepository.GetPendingInstructions(lastId, MaxInstructionsToRetrieve))
foreach (CacheInstruction instruction in pendingInstructions)
{
// If this flag gets set it means we're shutting down! In this case, we need to exit asap and cannot
// continue processing anything otherwise we'll hold up the app domain shutdown.

6
src/Umbraco.Infrastructure/Sync/DatabaseServerMessenger.cs
View File

@@ -286,7 +286,7 @@ namespace Umbraco.Cms.Infrastructure.Sync
try
{
CacheInstructionServiceProcessInstructionsResult result = CacheInstructionService.ProcessInstructions(_released, LocalIdentity, _lastPruned);
CacheInstructionServiceProcessInstructionsResult result = CacheInstructionService.ProcessInstructions(_released, LocalIdentity, _lastPruned, _lastId);
if (result.InstructionsWerePruned)
{
_lastPruned = _lastSync;
@@ -307,8 +307,8 @@ namespace Umbraco.Cms.Infrastructure.Sync
_syncIdle.Set();
}
}
}
/// <summary>
/// Reads the last-synced id from file into memory.
/// </summary>

92
src/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/CacheInstructionServiceTests.cs
View File

@@ -152,7 +152,7 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services
// Create three instruction records, each with two instructions. First two records are for a different identity.
CreateAndDeliveryMultipleInstructions(sut);
CacheInstructionServiceProcessInstructionsResult result = sut.ProcessInstructions(false, LocalIdentity, DateTime.UtcNow.AddSeconds(-1));
CacheInstructionServiceProcessInstructionsResult result = sut.ProcessInstructions(false, LocalIdentity, DateTime.UtcNow.AddSeconds(-1), -1);
Assert.Multiple(() =>
{
@@ -171,7 +171,7 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services
CreateAndDeliveryMultipleInstructions(sut);
CacheInstructionServiceProcessInstructionsResult result = sut.ProcessInstructions(false, LocalIdentity, DateTime.UtcNow.AddHours(-1));
CacheInstructionServiceProcessInstructionsResult result = sut.ProcessInstructions(false, LocalIdentity, DateTime.UtcNow.AddHours(-1), -1);
Assert.IsTrue(result.InstructionsWerePruned);
}
@@ -183,7 +183,7 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services
CreateAndDeliveryMultipleInstructions(sut);
CacheInstructionServiceProcessInstructionsResult result = sut.ProcessInstructions(true, LocalIdentity, DateTime.UtcNow.AddSeconds(-1));
CacheInstructionServiceProcessInstructionsResult result = sut.ProcessInstructions(true, LocalIdentity, DateTime.UtcNow.AddSeconds(-1), -1);
Assert.Multiple(() =>
{
@@ -193,6 +193,92 @@ namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.Services
});
}
[Test]
public void Processes_Instructions_Only_Once()
{
// This test shows what's happening in issue #10112
// The DatabaseServerMessenger will run its sync operation every five seconds which calls CacheInstructionService.ProcessInstructions,
// which is why the CacheRefresherNotification keeps dispatching, because the cache instructions gets constantly processed.
var sut = (CacheInstructionService)GetRequiredService<ICacheInstructionService>();
CreateAndDeliveryMultipleInstructions(sut);
var lastId = -1;
// Run once
CacheInstructionServiceProcessInstructionsResult result = sut.ProcessInstructions(false, LocalIdentity, DateTime.UtcNow.AddSeconds(-1), lastId);
Assert.Multiple(() =>
{
Assert.AreEqual(3, result.LastId); // 3 records found.
Assert.AreEqual(2, result.NumberOfInstructionsProcessed); // 2 records processed (as one is for the same identity).
Assert.IsFalse(result.InstructionsWerePruned);
});
// DatabaseServerMessenger stores the LastID after ProcessInstructions has been run.
lastId = result.LastId;
// The instructions has now been processed and shouldn't be processed on the next call...
// Run again.
CacheInstructionServiceProcessInstructionsResult secondResult = sut.ProcessInstructions(false, LocalIdentity, DateTime.UtcNow.AddSeconds(-1), lastId);
Assert.Multiple(() =>
{
Assert.AreEqual(0, secondResult.LastId); // No instructions was processed so LastId is 0, this is consistent with behavior from V8
Assert.AreEqual(0, secondResult.NumberOfInstructionsProcessed); // Nothing was processed.
Assert.IsFalse(secondResult.InstructionsWerePruned);
});
}
[Test]
public void Processes_New_Instructions()
{
var sut = (CacheInstructionService)GetRequiredService<ICacheInstructionService>();
CreateAndDeliveryMultipleInstructions(sut);
var lastId = -1;
CacheInstructionServiceProcessInstructionsResult result = sut.ProcessInstructions(false, LocalIdentity, DateTime.UtcNow.AddSeconds(-1), lastId);
Assert.AreEqual(3, result.LastId); // Make sure LastId is 3, the rest is tested in other test.
lastId = result.LastId;
// Add new instruction
List<RefreshInstruction> instructions = CreateInstructions();
sut.DeliverInstructions(instructions, AlternateIdentity);
CacheInstructionServiceProcessInstructionsResult secondResult = sut.ProcessInstructions(false, LocalIdentity, DateTime.UtcNow.AddSeconds(-1), lastId);
Assert.Multiple(() =>
{
Assert.AreEqual(4, secondResult.LastId);
Assert.AreEqual(1, secondResult.NumberOfInstructionsProcessed);
Assert.IsFalse(secondResult.InstructionsWerePruned);
});
}
[Test]
public void Correct_ID_For_Instruction_With_Same_Identity()
{
var sut = (CacheInstructionService)GetRequiredService<ICacheInstructionService>();
CreateAndDeliveryMultipleInstructions(sut);
var lastId = -1;
CacheInstructionServiceProcessInstructionsResult result = sut.ProcessInstructions(false, LocalIdentity, DateTime.UtcNow.AddSeconds(-1), lastId);
Assert.AreEqual(3, result.LastId); // Make sure LastId is 3, the rest is tested in other test.
lastId = result.LastId;
// Add new instruction
List<RefreshInstruction> instructions = CreateInstructions();
sut.DeliverInstructions(instructions, LocalIdentity);
CacheInstructionServiceProcessInstructionsResult secondResult = sut.ProcessInstructions(false, LocalIdentity, DateTime.UtcNow.AddSeconds(-1), lastId);
Assert.Multiple(() =>
{
Assert.AreEqual(4, secondResult.LastId);
Assert.AreEqual(0, secondResult.NumberOfInstructionsProcessed);
Assert.IsFalse(secondResult.InstructionsWerePruned);
});
}
private void CreateAndDeliveryMultipleInstructions(CacheInstructionService sut)
{
for (int i = 0; i < 3; i++)