ROCK-8447 Optimizations to reduce load when a server restarts

[stphnlee]

What changed and why

CheckinCache.cs

Change	Effect
IsReadyToPublish gate	Suppresses all bus publishing until IsRockStarted is True() plus a 30-second grace period. The recycling server silently hydrates its cache from the DB and consumes messages from other nodes, without disturbing them.
Clear() rewritten	Removes items locally in a loop but publishes only 1 message (the clear-all with key=null) instead of N+1 messages. This alone eliminates the biggest message amplification.

CheckinCacheConsumer.cs

Change	Effect
IsFromSelf(IRockMessage) check	Skips messages this node sent. The local cache was already updated before publishing, so processing your own message was doubling every operation.
Surgical AllKeys updates	Instead of calling RockCache.Remove(allKeysListCacheKey) on every message (which forces a keyFactory() DB call on next access), the consumer now adds/removes the single key directly in the cached list. The AllKeys list only gets invalidated on errors or full clears.

[Copilot]

Pull request overview

Optimizes the FamilyCheckin cache bus behavior to reduce message amplification and DB load during server restarts and normal cache churn.

Changes:

• Adds a warm-up gate to suppress cache bus publishing until Rock is started plus a grace period.
• Reworks cache clearing to publish a single “clear-all” message instead of per-item messages.
• Updates the cache consumer to skip self-sent messages and to update the cached AllKeys list incrementally instead of invalidating it.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File	Description
Plugins/org.secc.FamilyCheckin/Cache/CheckinCacheConsumer.cs	Skips self-sent messages and performs “surgical” AllKeys list maintenance on update/remove to avoid invalidation/DB refresh.
Plugins/org.secc.FamilyCheckin/Cache/CheckinCache.cs	Adds warm-up publish suppression and changes Clear() to local-remove + single clear-all publish.

[Copilot]

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]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

Plugins/org.secc.FamilyCheckin/Cache/CheckinCache.cs:144

• In Remove(), the cache publishes the bus message before updating the local AllKeys list. With the new consumer behavior that skips self-sent messages, this node will never process its own remove message, so AllKeys can remain stale and still include the removed key until a later refresh. Update the local key list (UpdateKeys/removeKey) before publishing (or ensure local key removal happens independently of self-message processing).

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

[Copilot]

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]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

[Copilot]

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]

In UpdateKeys(), the throttle-path always clones currentKeys into updatedKeys even when ensureKey/removeKey don’t result in any change. This creates unnecessary allocations under load (and returns a list instance that isn’t the cached reference when modified is false). Consider only cloning when you actually need to add/remove a key; otherwise return currentKeys directly.

[Copilot]

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.