feat(session): share the session directory between instances #33
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: unkin/jellyfin-ha-src#33
Reference in New Issue
Block a user
Delete Branch "benvin/session-directory"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Without sticky sessions a device's websocket sits on one pod while its requests land on either, so the session list hid half the estate and remote control reached nothing.
Emby.Server.Implementations/Session/SessionManager.cs:319,:353— sessionIdisMD5(client+deviceId+userId)(:637,:714), identical on every pod, andLogSessionActivitypublishesOwnerPod = this podon every authenticated request (RequestHelpers.GetSession, e.g.POST /Sessions/Playing/Progress). Without sticky sessions both pods hold the same session and both blind-SETthe same key, so ownership flaps and half the time names the pod with no websocket -> only publish/own while this pod holds a liveISessionController, and write ownership with a compare-and-set likeRedisTranscodeSessionStore'sRenewScript.Emby.Server.Implementations/Session/SessionManager.cs:1383—GetSessionToRemoteControlprefers the localSessionInfounconditionally. On a pod that holds the session but not the websocket (normal, per above)SendMessageToSessioniterates zero controllers and the command is silently dropped — the exact failure this PR claims to close -> fall through to the directory when the local session has no active controller.Emby.Server.Implementations/Session/SessionManager.cs:2231-2234— local and remote lists are concatenated with no dedupe byId; the same session appears twice (with contradictorySupportsRemoteControl/IsActive) whenever both pods hold it ->DistinctBy(i => i.Id), preferring the entry whose owner has a live controller.Emby.Server.Implementations/Session/RedisSessionDirectory.cs:78,SessionManager.cs:370,:2443—RemoveAsyncdeletesjellyfin:session:<id>unconditionally. A websocket closing on pod A, or pod A shutting down, erases the entry for a session pod B currently owns; it vanishes from every list and is unroutable until B's next refresh -> compare-and-delete onOwnerPod.Emby.Server.Implementations/Session/RedisPodMessageBus.cs:51-54—CommandFlags.FireAndForgetdiscards the PUBLISH receiver count, so a command routed to a pod that died inside the 60s TTL returns 204 and reaches nothing -> publish without FireAndForget, treat 0 receivers as not-found and drop the stale entry.Emby.Server.Implementations/Session/SessionManager.cs:405—GetRemoteSessiondoes a fullSCAN jellyfin:session:*plus a GET per key to find one id, andSendGeneralCommand/SendPlaystateCommandcall it twice per request -> addISessionDirectory.TryGetAsync(sessionId)backed by a singleStringGetAsync.Emby.Server.Implementations/Session/SessionManager.cs:319— the directory write is awaited on the hot path of every authenticated request, and the payload is the wholeSessionInfoDtoincludingNowPlayingItem(a fullBaseItemDto). Multi-KB round trip per request with no timeout; a slow valkey stalls requests instead of degrading -> publish from the refresh timer and lifecycle transitions only, or fire-and-forget it.Emby.Server.Implementations/Session/SessionManager.cs:1773,:1813,:1586,:1594—AddAdditionalUser/RemoveAdditionalUser/SendSyncPlay*still useGetSession(sessionId), which 404s for a session this pod does not hold, whileGET /Sessionsnow advertises that session from every pod -> route them too, or say in the body whichsessionId-addressed endpoints stay local.tests/Jellyfin.Server.Tests/HighAvailability/SessionDirectoryReplicaTests.cs:106,:129— no test has both replicas holding the same session id, which is the steady state without sticky sessions, so the three findings above all pass; and the handshake is simulated with a secondLogSessionActivityrather thanOnSessionControllerConnected, leaving the fire-and-forget republish atSessionManager.cs:338untested -> add a case where replica B has also logged activity for the device before the command is sent.Emby.Server.Implementations/Session/RedisSessionDirectory.cs:83-98— one failingStringGetAsyncfaultsTask.WhenAlland the catch returns an empty list, discarding every entry that did load;cancellationTokenis not passed to the GETs.MediaBrowser.Controller/Session/PodIdentity.cs:13— the same expression is already inline atRedisConfigurationInvalidationBus.cs:35,RedisScanLeaderLease.cs:56andDynamicHlsController.cs:48-> switch them over or drop the helper; alsoCurrentre-reads the env var on every access.Jellyfin.Server/Extensions/SessionDirectoryServiceCollectionExtensions.cs:64—Microsoft.Extensions.Options.IOptions<...>fully qualified; add the using.Emby.Server.Implementations/Session/RemoteSessionController.cs:68,SessionManager.cs:482,:515—HoldsConnectionis a snapshot up toRefreshIntervalSeconds(20s) old, and the PUBLISH subscriber count only proves the owner's Redis connection is alive, not that it still holds the socket. Owner-side the message hitscontrollers.Count == 0, logs a warning and returns; the caller seesdelivered == 1and gets 204 — the exact silent drop this PR targets -> make the owner acknowledge (reply channel / correlation id) and 404 on no ack, rather than trusting the subscriber count.Emby.Server.Implementations/Session/SessionManager.cs:1479,:1568—GetRemoteSession(...) ?? localfalls back to the request-serving pod's controller-less copy whenever the directory entry is absent, expired, orRedisSessionDirectory.GetAsyncswallowed a valkey error tonull;SendMessageToSessionthen iterates zero controllers and returns 204 having delivered nothing. The body claims the request fails when the owner is no longer listening -> whenlocalhas no active controller and the directory has no entry, throw instead of returninglocal.Emby.Server.Implementations/Session/SessionManager.cs:676,:1030,:2402-2405—OnPlaybackStart/OnPlaybackProgressmutate only the copy on the pod that servedPOST /Sessions/Playing*, the entry is published only by the owner from the owner's copy, and the non-owner's copy is now filtered out byownedElsewhere. Without sticky sessions a playback start served by the non-owner is never reflected anywhere, soGET /Sessionsreports the session as idle on every pod (and the non-owner runs the 1sStartAutomaticProgresstimer for a session nobody reports) -> publish playback transitions to the owner over the bus, or fold the non-owner's newerLastActivityDate/PlayStateinto the listed entry.Emby.Server.Implementations/Session/RedisSessionDirectory.cs:39— the claim is refused only when the recordedconnected > 0, so two pods that both hold no controller (connectedUtcTicks == 0) take ownership from each other on every publish. A device with no websocket in active playback has its listedPlayState/NowPlayingItemflip between the two pods' partial copies -> refuse a zero-epoch claim from a different pod while an entry exists, or tie-break on something stable.Emby.Server.Implementations/Session/RedisSessionDirectory.cs:39,SessionManager.cs:403— the epoch isDateTime.UtcNow.Tickstaken on the claiming pod and compared against ticks written by a different machine. A pod whose clock lags cannot claim a genuinely newer connection until the old owner publishes a zero epoch or the entry expires (60s default), and commands route to a pod with no socket for that whole window -> derive the epoch from a valkey-side counter (INCR) so the comparison is against one clock.Emby.Server.Implementations/Session/SessionManager.cs:1685,:1700—GetSession(sessionId, false)returns the non-owner's controller-less copy in the common case, notnull, so the "not held by this instance" debug line never fires andSendMessageToSessiondrops the SyncPlay command with no log and no error at all. The skip path is dead where it matters -> branch on "no active controller", not on "no local copy".Jellyfin.Server/Extensions/SessionDirectoryServiceCollectionExtensions.cs:50,:57—IPodMessageBusandISessionDirectoryfall back to their null implementations independently. A Redis directory paired withNullPodMessageBusleaves_directoryEnabledtrue withPublishAsyncalways returning 0, so every routed command 404s permanently -> resolve both from one attempt and fall back together.Emby.Server.Implementations/Session/SessionManager.cs:419—RefreshSessionDirectoryis anasync voidtimer callback with no re-entrancy guard, awaiting each publish serially at up toOperationTimeoutSecondseach; it also republishes every non-owned copy purely to have the claim refused.Emby.Server.Implementations/Session/RedisSessionDirectory.cs:175— one failingStringGetAsyncfaultsTask.WhenAlland the catch discards every entry that did load, contrary to the "degrade" comment at:195.Emby.Server.Implementations/Session/RedisPodMessageBus.cs:19— the 5s publish timeout is hard-coded and ignoresOperationTimeoutSeconds; combined with the blanket catch a transient valkey blip is surfaced to the user as "session not found".tests/Jellyfin.Server.Tests/HighAvailability/SessionDirectoryReplicaTests.cs:400—CreateBusmutates the process-wideJELLYFIN_INSTANCE_IDwhile other test classes may be readingPodIdentity.Currentin parallel -> pass the pod id toRedisPodMessageBusinstead of going through the environment.tests/Jellyfin.Server.Tests/HighAvailability/SessionDirectoryReplicaTests.cs:125— no case has both replicas holding a live controller, and none reports playback to the non-owner, so the flap and split-playback-state findings above pass.Emby.Server.Implementations/Session/SessionManager.cs:1040—CheckForIdlePlaybacksweeps_activeConnections, i.e. local copies, butOnPlaybackStoppednow routes to the owner. A non-owner copy that ever applied a report locally (the failed-route fallback, or a second live socket on the other replica — the stateBothReplicasHoldingAConnection_AgreeOnOneOwnerestablishes as normal) keepsNowPlayingItemwith a frozenLastPlaybackCheckIn, so five minutes later it routes a stop into a live playback on the owner and saves user data at its staleLastPlaybackCheckInPositionTicks-> skip sessions this instance holds no live controller for, or callOnPlaybackStoppedCoredirectly from the sweep.Emby.Server.Implementations/Session/SessionManager.cs:1074— same shape:CheckForInactiveSteamsdecides from the local copy'sPlayState.IsPaused/LastPausedDateandSendPlaystateCommandnow routes, so a stale paused non-owner copy sends a realStopto the owner's live socket -> gate the loop onGetConnectedSession(session.Id).Emby.Server.Implementations/Session/SessionManager.cs:2403,Jellyfin.Server.Implementations/Devices/DeviceManager.cs:33—ReportCapabilitiesis sessionId-addressed, still resolves withGetSession, is not routed, and_capabilitiesMapis per-process. APOST /Sessions/Capabilities/Fulllanding on the non-owner never reaches the owner, so the owner publishesSupportsRemoteControl = falseand an emptySupportedCommandsandGET /Sessions?controllableByUserId=hides the device from every replica — the symptom this PR targets, on roughly half of connections.?id=<remotely-owned>also 404s here whileAddAdditionalUseron the same id succeeds -> route it like the additional-user change, or name it in the local-only boundary.Emby.Server.Implementations/Session/SessionManager.cs:2526— route failed and this instance holds no copy:ReportNowViewingItemreturns 204 having done nothing, against "unacked is a 404, never a silent 204" -> throw when neither the owner nor a local copy took it.Emby.Server.Implementations/Session/RedisPodMessageBus.cs:57— a failedSubscribeis swallowed, leaving a bus that publishes but can never receive an ack; every routed operation then burns the fullOperationTimeoutSecondsbefore reporting undelivered, and the paired fallback inSharedSessionServices.Createcannot see it -> let it throw.Emby.Server.Implementations/Session/RedisPodMessageBus.cs:84,:91— the publish and the ack wait each get their own_timeout, so one routed call on a request path can block for 2xOperationTimeoutSeconds-> one deadline for both.Emby.Server.Implementations/Session/SessionManager.cs:1478— an ack lost after the owner applied the report, or a partial apply beforeOnRoutedPlaybackReport's catch, makes the origin apply it a second time; idempotent for progress, but the stop path re-firesPlaybackStoppedand re-saves user data -> make the stop idempotent or say the reports are at-least-once.MediaBrowser.Controller/Session/SessionDirectoryEntry.cs:19—HoldsConnectionis written on every publish and asserted by the tests but never read by production code -> drop it or gate on it.tests/Jellyfin.Server.Tests/HighAvailability/SessionDirectoryReplicaTests.cs:485— asserts absolute counter values (1, then 2) against the valkey CI shares across the whole class run; it passes only because nothing else touches that key -> assert the ordering instead.Jellyfin.Api/WebSocketListeners/SessionInfoWebSocketListener.cs:58,:64— theSessions/SessionsStartpush is the only session-list consumer left on_sessionManager.Sessions. On a non-owner it omits every remotely-owned session and pushes the local copy thatSessionManager.cs:2706deliberately filters out — playback now routes to the owner, so that copy hasNowPlayingItemnull. The dashboard's Active Devices panel therefore reports a playing device as idle whileGET /Sessionson the same pod reports it playing, and the panel flips between the two -> feed bothGetDataToSendoverloads fromGetSessions.Emby.Server.Implementations/Session/SessionManager.cs:2619,:1761,:2706—ReportTranscodingInfois local-only and deviceId-addressed, but segment requests land on either pod, soTranscodingInfois set on the copyownedElsewherenow drops. It went from visible on one pod to invisible on both;ClearTranscodingInfoat:1235/:1386also only ever clears the owner's (empty) copy -> route it to the owner, or name it in the local-only boundary in the body.Emby.Server.Implementations/Session/SessionManager.cs:2477-2488— an unacked capabilities route with a local copy present applies the report only to the copyGetSessionsfilters out and answers 204; the owner's published entry keeps the staleSupportsRemoteControl/SupportedCommandsuntil the entry TTL expires or the client re-reports. "An unreachable owner applies it locally, at least once" does not hold for capabilities, since locally is not visible anywhere.Emby.Server.Implementations/Session/RedisSessionDirectory.cs:184-207—GetAllAsyncruns a keyspaceSCAN jellyfin:session:*plus a GET per key on everyGET /Sessions, which clients poll and the cast picker calls withcontrollableByUserId; the single-session lookup was the only scan replaced -> keep a set of live session ids and read that.tests/Jellyfin.Server.Tests/HighAvailability/SessionDirectoryReplicaTests.cs:637,:685— both sweeps' negative halves wait 5s and 3s for something not to happen, while theOwnsSessionAsyncread they depend on is itself allowedOperationTimeoutSeconds(5s). A slow valkey passes the assertion for the wrong reason, and the non-vacuity control then fires on the non-owner's message -> assert the sweep skipped rather than on elapsed time.