Throttle checkin cache key refresh when the key list is empty

[jwakefield-secc]

Problem

CheckinCache.UpdateKeys only honored its 10-second refresh interval when the cached key list was already non-empty:

if ( currentKeys.Any() && ( now - _lastKeysRefreshUtc ) < KeysRefreshInterval )

An empty list is a legitimate result — overnight, before the day's first check-in, no Attendance rows match — but this guard treated it as a cold cache. So every caller fell through to keyFactory() instead of serving from cache, and the throttle never applied.

AttendanceCache.KeyFactory() emits a full scan of dbo.Attendance (~23,900 logical reads, ~1.8s CPU) to return zero rows. Cost tracks index size, not result size — the query is forced to scan rather than seek because CreatedDateTime is datetime while EF6 sends the parameter as datetime2(7).

CheckinMonitor.BindTable calls into the cache once per open room on a 10-second render loop, so a single screen produced roughly 25,000 rebuilds an hour. On rockprod this showed as a sustained ~98% CPU plateau from 01:00 until the day's first check-in, on about 65% of nights and 84% of Sundays.

Fix

Drop currentKeys.Any(), so the throttle depends only on when the list was last rebuilt rather than on whether it happens to be empty.

The throttled branch still applies ensureKey / removeKey, so a live check-in's key is injected into the list immediately with no database round trip. _lastKeysRefreshUtc starts at DateTime.MinValue, so a cold start still loads on first call.

Evidence

Measured on RockDev, which had been rebuilding continuously because its data ends 21 July and nothing is ever written "today":

	Rebuilds/min
Before	150 (flat for 15+ hours)
After one Attendance row made the list non-empty	5.2

That's one rebuild per ~11.5s against the 10s interval — the throttle at its intended cadence. This PR produces the same behavior without needing a row to exist.

Production, 1 Aug (unmitigated) vs 2 Aug (row inserted as a stopgap), overnight hours:

Hour ET	1 Aug	2 Aug
02:00	97.65%	1.68%
03:00	98.60%	0.15%
04:00	98.63%	0.07%

Trade-off worth reviewing

If the cache entry is evicted (rather than being genuinely empty) within the throttle window, callers now get an empty list for up to 10 seconds instead of an immediate reload. AllKeys() can't distinguish "loaded and empty" from "not loaded" — both return an empty list — so this is the cost of the minimal fix.

Impact is bounded: 10 seconds, and the live check-in path is covered by ensureKey. Against a six-hour 98% CPU plateau most nights, the trade seems clearly right, but it is a real behavior change and reviewers should weigh it.

Not in this PR

• CheckinMonitor refresh guard — don't start a render while one is in flight; back off on consecutive failures. The 10s interval is hardcoded in the .ascx, so this is a code change, not block config. Affects blocks 1396 / 936 / 7167 (pages 465, 545, 3211).
• HasColumnType("datetime") on Attendance.CreatedDateTime — removes the per-row convert so the scan becomes a seek. Fixes cost rather than frequency. Note this raises executions/sec on its own, since the loop is duration-bound — it should land after this PR, not before.
• CloseOccurrence idempotency — tolerate a zero-row delete and still flip OccurrenceCache.IsActive.

Testing notes

To verify after deploy, on a database with no attendance rows for the current day:

SELECT SUM(qs.execution_count) AS ExecCount, CONVERT(varchar(19), SYSUTCDATETIME()) AS NowUtc
FROM   sys.dm_exec_query_stats qs
CROSS APPLY sys.dm_exec_sql_text(qs.sql_handle) t
WHERE  t.text LIKE '%Extent1%' AND t.text LIKE '%Attendance%';

Sample twice a minute apart with a Check-In Monitor open. Expect ~6/min, not ~150/min.

Note there are temporary stopgap rows in production and DEV holding the key list non-empty until this ships — remove them after deploy so the fix is actually exercised:

DELETE FROM Attendance WHERE ForeignKey = 'ROCK-STORM-STOPGAP';   -- prod
DELETE FROM Attendance WHERE ForeignKey = 'ROCK-STORM-DEVTEST';   -- dev

The prod row is dated 5 August, so protection lapses on the 6th regardless.

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR fixes a performance issue in the FamilyCheckin cache key refresh logic by ensuring the 10-second refresh throttle applies even when the cached key list is legitimately empty (e.g., before the first check-in of the day). This prevents repeated expensive keyFactory executions that can occur in “empty but valid” scenarios.

Changes:

• Removed the currentKeys.Any() gating from the key refresh throttle so throttling depends only on elapsed time.
• Expanded inline documentation to clarify why empty key lists must still be throttled.

[stphnlee]

Good job tracking down this issue and finding a fix. I approve with two minor adjustments: 1) accept GitHub Copilot's suggestion to update the comment and 2) add a note in the README for the familycheckin project that explains the 10s empty cache, something like this, either in the Components area or Observations:

Cache key-list throttling (PR #281): CheckinCache throttles key-list refreshes to once per 10s per node, including when the cached list is empty — an empty list is a valid state (overnight) and refreshing on every call re-ran the keyFactory query. Tradeoff: after a cache clear/flush during active check-in, AllKeys-derived views (attendance/occupancy) can read empty for up to 10 seconds before self-healing. Accepted; the ensure/remove paths still apply within the window, so individual check-ins aren't lost.