Context
Raised during review of #4404 (RC-16: Update Redis metadata when backend sessions expire).
Feedback
1. Redundant storage.Load in NotifyBackendExpired
NotifyBackendExpired currently performs a storage.Load as its first step, but there are no callers yet. Any caller that detects a backend expiry will almost certainly already have the session metadata or MultiSession in hand — otherwise it wouldn't know the backend expired.
Suggestion: Accept the metadata (or MultiSession) as a parameter to skip the redundant load.
2. Prefer lazy expiry handling over proactive health-checking ✅ Done
Backend expiry should be handled lazily (updating metadata when the session is next accessed or used) rather than via a health-checking mechanism that proactively scans for expired backends. Lazy updates are simpler and avoid the synchronization complexity of background health checkers.
Already implemented: NotifyBackendExpired does not immediately evict the cache entry. Eviction happens lazily on the next GetMultiSession call when checkSession detects metadata drift.
Proposed changes
- Refactor
NotifyBackendExpired to accept session metadata or MultiSession as a parameter
- Remove the internal
storage.Load call
Notes
Not blocking the original PR — this is a follow-up to be addressed when the caller is introduced.
Context
Raised during review of #4404 (RC-16: Update Redis metadata when backend sessions expire).
Feedback
1. Redundant
storage.LoadinNotifyBackendExpiredNotifyBackendExpiredcurrently performs astorage.Loadas its first step, but there are no callers yet. Any caller that detects a backend expiry will almost certainly already have the session metadata orMultiSessionin hand — otherwise it wouldn't know the backend expired.Suggestion: Accept the metadata (or
MultiSession) as a parameter to skip the redundant load.2. Prefer lazy expiry handling over proactive health-checking✅ DoneBackend expiry should be handled lazily (updating metadata when the session is next accessed or used) rather than via a health-checking mechanism that proactively scans for expired backends. Lazy updates are simpler and avoid the synchronization complexity of background health checkers.Already implemented:
NotifyBackendExpireddoes not immediately evict the cache entry. Eviction happens lazily on the nextGetMultiSessioncall whencheckSessiondetects metadata drift.Proposed changes
NotifyBackendExpiredto accept session metadata orMultiSessionas a parameterstorage.LoadcallNotes
Not blocking the original PR — this is a follow-up to be addressed when the caller is introduced.