Optimize CloseInactiveSessionsJob: batch Redis calls

[ejsmith]

Summary

The CloseInactiveSessionsJob was generating excessive Redis traffic — primarily individual GET commands — causing noisy traces and unnecessary latency. This PR batches all cache reads into just 2 Redis MGET calls per page instead of 100–300 sequential GET calls.

Problem

For each open session (up to 100 per page), the job previously called GetHeartbeatAsync which issued 1–3 individual Redis GETs:

1. GET for the session-ID-based heartbeat timestamp
2. GET for the user-identity-based heartbeat timestamp (fallback if Add Requirements Section #1 misses)
3. GET for the close flag (if a heartbeat was found)

With 100 sessions per page, this produced 100–300 sequential Redis roundtrips per page, each generating its own trace span.

Solution

Replaced the per-session GetHeartbeatAsync / GetLastHeartbeatActivityUtcAsync methods with a single GetHeartbeatsBatchAsync method that processes an entire page in 5 phases:

1. Collect keys — Gathers all possible heartbeat cache keys for the page (in-memory)
2. Batch MGET Add Requirements Section #1 — Single GetAllAsync<DateTime> for all heartbeat timestamps
3. Resolve matches — Determines which key applies per session (session ID takes priority over user identity), collects close-flag keys
4. Batch MGET What do we need to fully integrate with nancyfx? #2 — Single GetAllAsync<bool> for all close flags
5. Build results — Assembles results in-memory using index-aligned arrays

Additional improvement

• Replaced Dictionary<PersistentEvent, HeartbeatResult> keyed by domain objects with index-aligned arrays (HeartbeatResult?[]), avoiding reliance on GetHashCode/Equals and reducing allocations.

Impact

Metric	Before	After
Redis calls per page	100–300	2
Trace spans per page	100–300	2
Business logic	Unchanged	Unchanged

All existing session-closing semantics (duplicate detection, inactive period, close flags) are preserved.

[niemyjski]

this is weird

[ejsmith]

Fixed — removed the weird as IReadOnlyList ?? [..] cast. The method now accepts IReadOnlyCollection and calls .ToList() internally, so the call site is clean.

[niemyjski]

should this just return a tuple with the doc and heartbeat and simplify below with a simple foreach?

[ejsmith]

Good call — refactored GetHeartbeatsBatchAsync to return (PersistentEvent Session, HeartbeatResult? Heartbeat)[] tuples. The caller is now a simple foreach with deconstruction instead of index-based iteration.

[Copilot]

Pull request overview

This PR optimizes the CloseInactiveSessionsJob by batching Redis cache operations to reduce network roundtrips and trace noise. Previously, the job made 100–300 sequential GET requests per page of sessions (one per session for heartbeat data and close flags). The new implementation consolidates these into just 2 MGET calls per page using the cache client's GetAllAsync method.

Changes:

• Replaced per-session GetHeartbeatAsync method with batch-oriented GetHeartbeatsBatchAsync that processes an entire page at once
• Changed from Dictionary keyed by domain objects to index-aligned arrays for tracking heartbeat results
• Maintained all existing business logic including session ID priority over user identity fallback, duplicate detection, and close flag handling

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[niemyjski]

not sure why this couldn't be inferred like this, I've noticed this a lot lately with llms and not generating modern c# or giving loops good variable names.

Suggested change

	IDictionary<string, CacheValue<bool>> closeValues = closeKeys.Count > 0
	? await _cache.GetAllAsync<bool>(closeKeys)
	: new Dictionary<string, CacheValue<bool>>();
	IDictionary<string, CacheValue<bool>> closeValues = closeKeys.Count > 0
	? await _cache.GetAllAsync<bool>(closeKeys)
	: [];

[ejsmith]

Unfortunately [] doesn't compile here — IDictionary<string, CacheValue<bool>> is an interface and the compiler reports CS9174: Cannot initialize type 'IDictionary<...>' with a collection expression because the type is not constructible. We'd need the concrete Dictionary<> type, but GetAllAsync returns IDictionary. Keeping the explicit new Dictionary<...>() for now.

[github-actions]

Package	Line Rate	Branch Rate	Complexity	Health
Exceptionless.Insulation	24%	23%	208	❌
Exceptionless.Core	66%	59%	7476	➖
Exceptionless.AppHost	26%	14%	55	❌
Exceptionless.Web	56%	42%	3489	➖
Summary	61% (11944 / 19562)	53% (5714 / 10736)	11228	➖