Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
136 changes: 92 additions & 44 deletions Plugins/org.secc.FamilyCheckin/Cache/CheckinCache.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,42 @@ public class CheckinCache<T> : IItemCache
private static DateTime _lastKeysRefreshUtc = DateTime.MinValue;
private static readonly TimeSpan KeysRefreshInterval = TimeSpan.FromSeconds( 10 );

// Warm-up: suppress publishing until this node is fully online.
// Anchored to the time this type was first loaded (i.e. app start),
// NOT to the first publish attempt, so the grace period cannot
// fire long after the startup burst has already passed.
// Override via web.config appSettings: <add key="CheckinCacheWarmUpSeconds" value="30" />
private static readonly DateTime _typeLoadedUtc = DateTime.UtcNow;
private static readonly TimeSpan WarmUpGracePeriod = TimeSpan.FromSeconds(
int.TryParse( System.Configuration.ConfigurationManager.AppSettings["CheckinCacheWarmUpSeconds"], out int configuredSeconds )
? configuredSeconds
: 30 );
Comment on lines +26 to +35
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The warm-up timestamp is stored in a static field on the generic type (CheckinCache), which is initialized when each closed generic type is first used—not necessarily at app start. This can unintentionally suppress publishing for WarmUpGracePeriod the first time a particular T is touched later in runtime (i.e., delayed suppression), contradicting the comment and potentially hiding updates. Consider anchoring the warm-up start to a non-generic, process-wide startup timestamp (shared across all T) or explicitly to Rock's application start time if available.

Copilot uses AI. Check for mistakes.

public void PostCached()
{
}

/// <summary>
/// Returns true once Rock is started AND the grace period (measured
/// from app start) has elapsed. During warm-up the node consumes
/// messages but does not publish.
/// </summary>
private static bool IsReadyToPublish
{
get
{
if ( !RockMessageBus.IsRockStarted )
{
return false;
}

// Grace period is always relative to when the app loaded this type.
// If the app has been running longer than the grace period,
// this is true immediately — no delayed suppression.
return ( DateTime.UtcNow - _typeLoadedUtc ) >= WarmUpGracePeriod;
}
}

public static List<string> AllKeys( Func<List<string>> keyFactory )
{
var keys = AllKeys();
Expand Down Expand Up @@ -57,7 +89,6 @@ public static T Get( string qualifiedKey, Func<T> itemFactory, Func<List<string>
}

RockCache.AddOrUpdate( qualifiedKey, item );
// PublishCacheUpdateMessage( qualifiedKey, item );
}
else
{
Expand All @@ -75,68 +106,71 @@ public static void AddOrUpdate( string qualifiedKey, T item, Func<List<string>>
return;
}

int retryCount = 3;
int retryDelayMs = 100;
Exception lastException = null;

while ( retryCount > 0 )
try
{
try
var keys = AllKeys();
if ( !keys.Any() || !keys.Contains( qualifiedKey ) )
{
UpdateKeys( keyFactory, ensureKey: qualifiedKey );
}
RockCache.AddOrUpdate( qualifiedKey, item );
PublishCacheUpdateMessage( qualifiedKey, item );
}
catch ( Exception ex )
{
// Log but don't retry synchronously — retrying with Thread.Sleep
// blocks the request thread while holding the ASP.NET session lock,
// which causes session queue exhaustion under load.
Rock.Model.ExceptionLogService.LogException(
new Exception( $"Failed to update cache for key {qualifiedKey}: {ex.Message}", ex ) );

var keys = AllKeys();
if ( !keys.Any() || !keys.Contains( qualifiedKey ) )
{
UpdateKeys( keyFactory, ensureKey: qualifiedKey );
} //RockCacheManager<T>.Instance.Cache.AddOrUpdate( qualifiedKey, item, v => item );
// Best-effort: at minimum get it into local cache
try
{
RockCache.AddOrUpdate( qualifiedKey, item );
PublishCacheUpdateMessage( qualifiedKey, item );
return;
}
catch ( Exception ex )
catch
{
lastException = ex;
retryCount--;

if ( retryCount > 0 )
{
Rock.Model.ExceptionLogService.LogException(
new Exception( $"Retrying cache update for {qualifiedKey}: {ex.Message}", ex ) );
System.Threading.Thread.Sleep( retryDelayMs );
retryDelayMs *= 2; // Exponential backoff
}
// Swallow — the item will be fetched from DB on next access
}
}

// If we get here, all retries failed
if ( lastException != null )
{
Rock.Model.ExceptionLogService.LogException(
new Exception( $"Failed to update cache after multiple attempts for key {qualifiedKey}", lastException ) );
}
}

public static void Remove( string qualifiedKey, Func<List<string>> keyFactory )
{
RockCache.Remove( qualifiedKey );
PublishCacheUpdateMessage( qualifiedKey, default( T ) );
UpdateKeys( keyFactory, removeKey: qualifiedKey );
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove() now calls UpdateKeys() before publishing the bus message. If keyFactory/UpdateKeys throws (DB/transient issues), the method will exit without publishing the remove, leaving other nodes’ caches stale. Consider publishing the remove regardless of key list update success (e.g., publish before updating keys, or wrap UpdateKeys in a try/catch so publish still happens).

Suggested change
UpdateKeys( keyFactory, removeKey: qualifiedKey );
try
{
UpdateKeys( keyFactory, removeKey: qualifiedKey );
}
catch ( Exception ex )
{
Rock.Model.ExceptionLogService.LogException(
new Exception( $"Failed to update cache keys for removed key {qualifiedKey}: {ex.Message}", ex ) );
}

Copilot uses AI. Check for mistakes.
PublishCacheUpdateMessage( qualifiedKey, default( T ) );
}

public static void Clear( Func<List<string>> keyFactory )
{
UpdateKeys( keyFactory );
// Get the definitive list of keys from the DB to ensure complete
// local cleanup, even if the cached AllKeys list was evicted or stale.
var keysToRemove = new HashSet<string>( keyFactory().Select( k => QualifiedKey( k ) ) );

// Create a copy of the keys to avoid collection modification during enumeration
foreach ( var key in AllKeys().ToList() )
// Also include any locally cached keys that might not be in DB
// (e.g. recently added but not yet persisted).
foreach ( var cachedKey in AllKeys() )
{
FlushItem( key, keyFactory );
keysToRemove.Add( cachedKey );
}

// Remove each cached item locally without publishing per-item messages
foreach ( var key in keysToRemove )
{
RockCache.Remove( key );
}

// Clear the AllKeys list
RockCache.Remove( AllKey, AllRegion );

Comment on lines 146 to +167
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clear(Func<List<string>> keyFactory) no longer uses keyFactory and only iterates over the current cached AllKeys() list. If the AllKeys entry is missing/stale (e.g., removed due to a prior error or eviction), this loop can be empty and the local cache items will not be cleared—especially since the consumer now skips self-sent clear-all messages. Consider refreshing keys via UpdateKeys(keyFactory) when AllKeys() is empty, or use RockCache.ClearCachedItemsForType(typeof(T)) to ensure the local clear is complete, and then publish the single clear-all message.

Copilot uses AI. Check for mistakes.
// Publish a single clear-all message instead of N+1
PublishCacheUpdateMessage( null, default( T ) );
}

public static void FlushItem( string qualifiedKey, Func<List<string>> keyFactory )
{
//RockCacheManager<T>.Instance.Cache.Remove( qualifiedKey );
RockCache.Remove( qualifiedKey );
UpdateKeys( keyFactory, removeKey: qualifiedKey );
PublishCacheUpdateMessage( qualifiedKey, default( T ) );
Expand Down Expand Up @@ -181,26 +215,30 @@ private static List<string> UpdateKeys( Func<List<string>> keyFactory, string en
{
bool modified = false;

// Copy-on-write: clone the list so threads currently
// enumerating the original reference are not affected.
var updatedKeys = new List<string>( currentKeys );

// Ensure the specified key is present
if ( !string.IsNullOrEmpty( ensureKey ) && !currentKeys.Contains( ensureKey ) )
if ( !string.IsNullOrEmpty( ensureKey ) && !updatedKeys.Contains( ensureKey ) )
{
currentKeys.Add( ensureKey );
updatedKeys.Add( ensureKey );
modified = true;
}

// Remove the specified key
if ( !string.IsNullOrEmpty( removeKey ) && currentKeys.Contains( removeKey ) )
if ( !string.IsNullOrEmpty( removeKey ) && updatedKeys.Contains( removeKey ) )
{
currentKeys.Remove( removeKey );
updatedKeys.Remove( removeKey );
modified = true;
}

if ( modified )
{
RockCache.AddOrUpdate( AllKey, AllRegion, currentKeys );
RockCache.AddOrUpdate( AllKey, AllRegion, updatedKeys );
}

return currentKeys;
return updatedKeys;
}

var keys = keyFactory().Select( k => QualifiedKey( k ) ).ToList();
Expand All @@ -224,6 +262,16 @@ private static List<string> UpdateKeys( Func<List<string>> keyFactory, string en

private static void PublishCacheUpdateMessage( string key, T item )
{
// During warm-up, only consume — don't publish.
// This prevents a recycling node from flooding the bus and
// causing all other nodes to invalidate their caches.
if ( !IsReadyToPublish )
{
RockLogger.Log.Debug( RockLogDomains.Bus,
$"Suppressed cache publish during warm-up for {typeof( T ).Name} key={key ?? "(clear all)"}. Server: {RockMessageBus.NodeName}." );
return;
}

var message = new CheckinCacheMessage
{
Key = key,
Expand Down
46 changes: 34 additions & 12 deletions Plugins/org.secc.FamilyCheckin/Cache/CheckinCacheConsumer.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System;
using System.Collections.Generic;
using Newtonsoft.Json;
using Rock.Bus;
using Rock.Bus.Consumer;
Expand All @@ -20,6 +21,16 @@ public override void Consume( CheckinCacheMessage message )
return;
}

// Skip messages this node published — the local cache was already
// updated before the message was sent, so processing it again is
// redundant and doubles the work.
if ( RockMessageBus.IsFromSelf( message ) )
{
RockLogger.Log.Debug( RockLogDomains.Bus,
$"Skipping self-sent CheckinCache message for key {message.Key}. Server: {RockMessageBus.NodeName}." );
return;
}

try
{
var cacheType = Type.GetType( message.CacheTypeName );
Expand Down Expand Up @@ -48,11 +59,10 @@ public override void Consume( CheckinCacheMessage message )
/// </summary>
private void ProcessCacheMessage<T>( CheckinCacheMessage message )
{
// Constants for the AllKeys list, matching CheckinCache<T>
string allKeysListCacheKey = $"{typeof( T ).Name}:All";
string allKeysListCacheRegion = "AllItems"; // This is the 'AllRegion' constant from CheckinCache<T>
string allKeysListCacheRegion = "AllItems";

Comment on lines 61 to 64
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

allKeysListCacheRegion is hard-coded as the string "AllItems" here while CheckinCache<T> also defines the region name. This duplication makes it easy for producer/consumer to drift if the region constant ever changes. Consider exposing the region/key constants from CheckinCache<T> (e.g., internal const) or centralizing them in a shared helper so both sides reference the same value.

Copilot uses AI. Check for mistakes.
// If we have additional data, this is an update
// If we have additional data, this is an update — apply the value directly
if ( !string.IsNullOrEmpty( message.AdditionalData ) )
{
try
Expand All @@ -69,11 +79,17 @@ private void ProcessCacheMessage<T>( CheckinCacheMessage message )
RockCache.AddOrUpdate( message.Key, item );
}

// Invalidate the AllKeys list as an item was added/updated
RockCache.Remove( allKeysListCacheKey, allKeysListCacheRegion );
// Copy-on-write: clone the list before modifying so threads
// currently enumerating the old reference are not affected.
var keys = RockCache.Get( allKeysListCacheKey, allKeysListCacheRegion ) as List<string>;
if ( keys != null && !keys.Contains( message.Key ) )
{
var updatedKeys = new List<string>( keys ) { message.Key };
RockCache.AddOrUpdate( allKeysListCacheKey, allKeysListCacheRegion, updatedKeys );
}

RockLogger.Log.Debug( RockLogDomains.Bus,
$"Updated cache for key {message.Key}. Server: {RockMessageBus.NodeName}. AdditionalData: {message.AdditionalData}" );
$"Updated cache for key {message.Key}. Server: {RockMessageBus.NodeName}." );
}
}
catch ( Exception ex )
Expand All @@ -89,7 +105,7 @@ private void ProcessCacheMessage<T>( CheckinCacheMessage message )
{
RockCache.Remove( message.Key );
}
// invalidate AllKeys on error
// Only invalidate AllKeys on deserialization error
RockCache.Remove( allKeysListCacheKey, allKeysListCacheRegion );
}
}
Expand All @@ -104,19 +120,25 @@ private void ProcessCacheMessage<T>( CheckinCacheMessage message )
{
RockCache.Remove( message.Key );
}
// Invalidate the AllKeys list as an item was removed
RockCache.Remove( allKeysListCacheKey, allKeysListCacheRegion );

RockLogger.Log.Debug( RockLogDomains.Bus, $"Removed cache for key {message.Key}" );
// Copy-on-write: clone before removing so concurrent readers are safe.
var keys = RockCache.Get( allKeysListCacheKey, allKeysListCacheRegion ) as List<string>;
if ( keys != null && keys.Contains( message.Key ) )
{
var updatedKeys = new List<string>( keys );
updatedKeys.Remove( message.Key );
RockCache.AddOrUpdate( allKeysListCacheKey, allKeysListCacheRegion, updatedKeys );
}

RockLogger.Log.Debug( RockLogDomains.Bus, $"Removed cache for key {message.Key}. Server: {RockMessageBus.NodeName}." );
}
else
{
// This is a clear all for this cache type
string typeName = typeof( T ).Name;
RockCache.ClearCachedItemsForType( typeof( T ) );
// Clear/invalidate the AllKeys list specifically
RockCache.Remove( allKeysListCacheKey, allKeysListCacheRegion );
RockLogger.Log.Debug( RockLogDomains.Bus, $"Cleared all cache for type {typeName}" );
RockLogger.Log.Debug( RockLogDomains.Bus, $"Cleared all cache for type {typeName}. Server: {RockMessageBus.NodeName}." );
}
}

Expand Down