Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handled flushing of cache instruction messages when created from handling events in a background thread #11757

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
27 changes: 21 additions & 6 deletions src/Umbraco.Web/BatchedDatabaseServerMessenger.cs
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,11 @@ protected override void DeliverRemote(ICacheRefresher refresher, MessageType mes
BatchMessage(refresher, messageType, idsA, arrayType, json);
}

public void FlushBatch()
public void FlushBatch() => FlushBatch(null);

internal void FlushBatch(HttpContextBase httpContext)
{
var batch = GetBatch(false);
var batch = httpContext != null ? GetBatch(false, httpContext) : GetBatch(false);
if (batch == null) return;

var instructions = batch.SelectMany(x => x.Instructions).ToArray();
Expand All @@ -83,9 +85,9 @@ public void FlushBatch()
{
WriteInstructions(scope, instructionsBatch);
}

scope.Complete();
}

}

private void WriteInstructions(IScope scope, IEnumerable<RefreshInstruction> instructions)
Expand All @@ -111,10 +113,15 @@ protected ICollection<RefreshInstructionEnvelope> GetBatch(bool create)
// the case if the asp.net synchronization context has kicked in
?? (HttpContext.Current == null ? null : new HttpContextWrapper(HttpContext.Current));

// if no context was found, return null - we cannot not batch
// if no context was found, return null - we cannot batch
if (httpContext == null) return null;

var key = typeof (BatchedDatabaseServerMessenger).Name;
return GetBatch(create, httpContext);
}

protected ICollection<RefreshInstructionEnvelope> GetBatch(bool create, HttpContextBase httpContext)
bergmania marked this conversation as resolved.
Show resolved Hide resolved
{
var key = typeof(BatchedDatabaseServerMessenger).Name;

// no thread-safety here because it'll run in only 1 thread (request) at a time
var batch = (ICollection<RefreshInstructionEnvelope>)httpContext.Items[key];
Expand All @@ -128,9 +135,17 @@ protected ICollection<RefreshInstructionEnvelope> GetBatch(bool create)
MessageType messageType,
IEnumerable<object> ids = null,
Type idType = null,
string json = null) => BatchMessage(refresher, messageType, null, ids, idType, json);

protected void BatchMessage(
ICacheRefresher refresher,
MessageType messageType,
HttpContextBase httpContext,
IEnumerable<object> ids = null,
Type idType = null,
string json = null)
{
var batch = GetBatch(true);
var batch = httpContext != null ? GetBatch(true, httpContext) : GetBatch(true);
var instructions = RefreshInstruction.GetInstructions(refresher, messageType, ids, idType, json);

// batch if we can, else write to DB immediately
Expand Down
35 changes: 32 additions & 3 deletions src/Umbraco.Web/Cache/DistributedCacheBinder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using Umbraco.Core.Models;
using Umbraco.Core.Services;
using Umbraco.Core.Services.Changes;
using Umbraco.Core.Sync;

namespace Umbraco.Web.Cache
{
Expand All @@ -21,17 +22,28 @@ public partial class DistributedCacheBinder : IDistributedCacheBinder
private readonly DistributedCache _distributedCache;
private readonly IUmbracoContextFactory _umbracoContextFactory;
private readonly ILogger _logger;
private readonly BatchedDatabaseServerMessenger _serverMessenger;

/// <summary>
/// Initializes a new instance of the <see cref="DistributedCacheBinder"/> class.
/// </summary>
[Obsolete("Please use the constructor accepting an instance of IServerMessenger. This constructor will be removed in a future version.")]
public DistributedCacheBinder(DistributedCache distributedCache, IUmbracoContextFactory umbracoContextFactory, ILogger logger)
{
_distributedCache = distributedCache;
_logger = logger;
_umbracoContextFactory = umbracoContextFactory;
}

/// <summary>
/// Initializes a new instance of the <see cref="DistributedCacheBinder"/> class.
/// </summary>
public DistributedCacheBinder(DistributedCache distributedCache, IUmbracoContextFactory umbracoContextFactory, ILogger logger, IServerMessenger serverMessenger)
: this(distributedCache, umbracoContextFactory, logger)
{
_serverMessenger = serverMessenger as BatchedDatabaseServerMessenger;
}

// internal for tests
internal static MethodInfo FindHandler(IEventDefinition eventDefinition)
{
Expand All @@ -42,7 +54,6 @@ internal static MethodInfo FindHandler(IEventDefinition eventDefinition)

private static readonly Lazy<MethodInfo[]> CandidateHandlers = new Lazy<MethodInfo[]>(() =>
{

return typeof(DistributedCacheBinder)
.GetMethods(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic)
.Select(x =>
Expand All @@ -66,9 +77,9 @@ public void HandleEvents(IEnumerable<IEventDefinition> events)
{
// Ensure we run with an UmbracoContext, because this may run in a background task,
// yet developers may be using the 'current' UmbracoContext in the event handlers.
using (_umbracoContextFactory.EnsureUmbracoContext())
using (var umbracoContextReference = _umbracoContextFactory.EnsureUmbracoContext())
{
// When it comes to content types types, a change to any single one will trigger a reload of the content and media caches.
// When it comes to content types, a change to any single one will trigger a reload of the content and media caches.
// We can reduce the impact of that by grouping the events to invoke just one per type, providing a collection of the individual arguments.
var groupedEvents = GetGroupedEventList(events);
foreach (var e in groupedEvents)
Expand All @@ -84,6 +95,24 @@ public void HandleEvents(IEnumerable<IEventDefinition> events)

handler.Invoke(this, new[] { e.Sender, e.Args });
}

// Handled events may be triggering messages to be sent for load balanced servers to refresh their caches.
// When the state changes that initiate the events are handled outside of an Umbraco request and rather in a
// background task, we'll have ensured an Umbraco context, but using a newly created HttpContext.
//
// An example of this is when using an Umbraco Deploy content transfer operation
// (see: https://github.com/umbraco/Umbraco.Deploy.Issues/issues/90).
//
// This will be used in the event handlers, and when the methods on BatchedDatabaseServerMessenger are called,
// they'll be using this "ensured" HttpContext, populating a batch of message stored in HttpContext.Items.
// When the FlushBatch method is called on the end of an Umbraco request (via the event handler wired up in
// DatabaseServerRegistrarAndMessengerComponent), this will use the HttpContext associated with the request,
// which will be a different one, and so won't have the batch stored in it's HttpContext.Items.
//
// As such by making an explicit call here, and providing the ensured HttpContext that will have had it's
// Items dictionary populated with the batch of messages, we'll make sure the batch is flushed, and the
// database instructions written.
_serverMessenger?.FlushBatch(umbracoContextReference.UmbracoContext.HttpContext);
}
}

Expand Down