retry building the quick connect store in the startup probe, not just reading it

Emby.Server.Implementations/ApplicationHost.cs

@@ -52,6 +52,7 @@ using Jellyfin.Server.Implementations.SystemBackupService;
 using MediaBrowser.Common;
 using MediaBrowser.Common.Configuration;
 using MediaBrowser.Common.Events;
+using MediaBrowser.Common.Extensions;
 using MediaBrowser.Common.Net;
 using MediaBrowser.Common.Plugins;
 using MediaBrowser.Common.Updates;
@@ -670,29 +671,45 @@ namespace Emby.Server.Implementations
         }
 
         /// <summary>
-        /// Reads the quick connect store here so a store that cannot be reached stops startup, rather than
+        /// Builds and reads the quick connect store here so a store that cannot be reached stops startup,
-        /// being discovered on the first request that needs it. A read rather than a resolve because a
+        /// rather than being discovered on the first request that needs it. Both halves sit inside the
-        /// shared store built with <c>abortConnect=false</c> constructs without touching the network, and
+        /// retry: a store built with <c>abortConnect=false</c> constructs without touching the network and
-        /// retried until <see cref="_quickConnectProbeDeadline"/> so a starting instance rides out the blip
+        /// only the read settles it, while one built without that option connects eagerly and fails at the
-        /// a running one already tolerates.
+        /// resolve. Retried until <see cref="_quickConnectProbeDeadline"/> so a starting instance rides out
+        /// the blip a running one already tolerates.
         /// </summary>
         private async Task ProbeQuickConnectStoreAsync()
         {
-            var store = Resolve<IQuickConnectStore>();
             var startTimestamp = Stopwatch.GetTimestamp();
+            var reported = false;
 
             while (true)
             {
                 try
                 {
+                    // A throwing singleton factory is not cached, so the resolve is retried along with the read.
+                    var store = Resolve<IQuickConnectStore>();
                     await store.GetRequestBySecretAsync(StartupProbeSecret).ConfigureAwait(false);
                     return;
                 }
                 catch (Exception ex)
                 {
-                    if (Stopwatch.GetElapsedTime(startTimestamp) + _quickConnectProbeRetryDelay < _quickConnectProbeDeadline)
+                    var elapsed = Stopwatch.GetElapsedTime(startTimestamp);
+                    if (elapsed + _quickConnectProbeRetryDelay < _quickConnectProbeDeadline)
                     {
-                        Logger.LogWarning(ex, "Quick connect store is not reachable yet, retrying.");
+                        if (!reported)
+                        {
+                            reported = true;
+                            Logger.LogWarning(
+                                ex,
+                                "Quick connect store is not reachable yet, retrying for up to {Seconds}s.",
+                                (int)_quickConnectProbeDeadline.TotalSeconds);
+                        }
+                        else
+                        {
+                            Logger.LogDebug(ex, "Quick connect store is still not reachable, retrying.");
+                        }
+
                         await Task.Delay(_quickConnectProbeRetryDelay).ConfigureAwait(false);
                         continue;
                     }
@@ -701,8 +718,14 @@ namespace Emby.Server.Implementations
                         ex,
                         "Quick connect is configured against the shared valkey/Redis store at {Key} and it is UNREACHABLE after {Seconds}s, so the server will not start. Bring valkey up, or clear that setting to keep quick connect state on this instance alone.",
                         TranscodeStoreOptions.RedisConnectionStringKey,
-                        (int)_quickConnectProbeDeadline.TotalSeconds);
+                        (int)elapsed.TotalSeconds);
-                    throw;
+
+                    if (ex is ServiceUnavailableException)
+                    {
+                        throw;
+                    }
+
+                    throw new ServiceUnavailableException("Quick connect store is unreachable.", ex);
                 }
             }
         }

Jellyfin.Server/Program.cs

@@ -267,7 +267,7 @@ namespace Jellyfin.Server
                 if (_setupServer!.IsAlive && !configurationCompleted)
                 {
                     _setupServer!.SoftStop();
-                    if (options.StartupMode is null or Configuration.StartupMode.MediaServer && !IsRunningInContainer())
+                    if ((options.StartupMode is null or Configuration.StartupMode.MediaServer) && !IsRunningInContainer())
                     {
                         await Task.Delay(_setupServerHoldAfterFailedStart).ConfigureAwait(false);
                     }

tests/Jellyfin.Server.Tests/QuickConnect/QuickConnectStartupTests.cs

@@ -46,6 +46,7 @@ public sealed class QuickConnectStartupTests : IAsyncLifetime
     private const string RedisConnectionStringVariable = "Jellyfin__TranscodeStore__RedisConnectionString";
     private const string FfmpegNoValidationVariable = "JELLYFIN_FFMPEG__NOVALIDATION";
     private const string DeadStore = "127.0.0.1:1,abortConnect=false,connectTimeout=250,connectRetry=0,syncTimeout=250";
+    private const string EagerDeadStore = "127.0.0.1:1,connectTimeout=250,connectRetry=0,syncTimeout=250";
 
     private RedisTestServer _redis = null!;
 
@@ -85,6 +86,33 @@ public sealed class QuickConnectStartupTests : IAsyncLifetime
                 && entry.Contains("valkey", StringComparison.Ordinal));
     }
 
+    /// <summary>
+    /// Three of the four connection strings the chart documents leave <c>abortConnect</c> at its default,
+    /// which connects eagerly, so the multiplexer is what fails and it fails while the store is being
+    /// built rather than on a read. The probe has to retry the build as well as the read and end on the
+    /// same message, not let a bare <see cref="RedisConnectionException"/> out.
+    /// </summary>
+    /// <remarks>
+    /// The core initialisation migrations are skipped here because <c>JellyfinMigrationService</c> takes
+    /// an <c>IBackupService</c> eagerly, which reaches the multiplexer through the library manager, so on
+    /// this shape they fail before the probe is reached at all. That ordering is a separate problem from
+    /// what the probe does when it runs.
+    /// </remarks>
+    [Fact]
+    public void EagerlyConnectingStoreDownPastTheDeadline_StopsStartupWithTheSameMessage()
+    {
+        Environment.SetEnvironmentVariable(RedisConnectionStringVariable, EagerDeadStore);
+
+        using var server = new StartupHarness(runCoreInitialisationMigrations: false);
+
+        Assert.ThrowsAny<ServiceUnavailableException>(() => server.Services);
+        Assert.Contains(
+            server.CriticalEntries,
+            entry => entry.Contains("Quick connect", StringComparison.Ordinal)
+                && entry.Contains("UNREACHABLE", StringComparison.Ordinal)
+                && entry.Contains("valkey", StringComparison.Ordinal));
+    }
+
     /// <summary>
     /// A store that is away when the probe first reads it but back inside the deadline lets the server
     /// come up. A running instance already rides out a valkey blip; a starting one has to as well, or a
@@ -288,6 +316,7 @@ public sealed class QuickConnectStartupTests : IAsyncLifetime
         private readonly ConcurrentBag<IDisposable> _disposables = new();
         private readonly ConcurrentQueue<(LogLevel Level, string Message)> _entries = new();
         private readonly Action? _beforeInitializeServices;
+        private readonly bool _runCoreInitialisationMigrations;
         private readonly string _root = Path.Combine(
             Path.GetTempPath(),
             "jellyfin-quickconnect-startup",
@@ -298,9 +327,10 @@ public sealed class QuickConnectStartupTests : IAsyncLifetime
             StartupHelpers.PerformStaticInitialization();
         }
 
-        public StartupHarness(Action? beforeInitializeServices = null)
+        public StartupHarness(Action? beforeInitializeServices = null, bool runCoreInitialisationMigrations = true)
         {
             _beforeInitializeServices = beforeInitializeServices;
+            _runCoreInitialisationMigrations = runCoreInitialisationMigrations;
         }
 
         public IReadOnlyCollection<string> CriticalEntries => Messages(LogLevel.Critical);
@@ -356,7 +386,11 @@ public sealed class QuickConnectStartupTests : IAsyncLifetime
                 var configuration = host.Services.GetRequiredService<IConfiguration>();
 
                 Program.ApplyStartupMigrationAsync(appPaths, configuration, new StartupOptions()).GetAwaiter().GetResult();
-                Program.ApplyCoreMigrationsAsync(host.Services, JellyfinMigrationStageTypes.CoreInitialisation).GetAwaiter().GetResult();
+                if (_runCoreInitialisationMigrations)
+                {
+                    Program.ApplyCoreMigrationsAsync(host.Services, JellyfinMigrationStageTypes.CoreInitialisation).GetAwaiter().GetResult();
+                }
+
                 _beforeInitializeServices?.Invoke();
                 appHost.InitializeServices(configuration).GetAwaiter().GetResult();
                 Program.ApplyCoreMigrationsAsync(host.Services, JellyfinMigrationStageTypes.AppInitialisation).GetAwaiter().GetResult();

tests/Jellyfin.Server.Tests/QuickConnect/QuickConnectStoreWiringTests.cs

@@ -108,9 +108,11 @@ public sealed class QuickConnectStoreWiringTests : IAsyncLifetime
     }
 
     /// <summary>
-    /// An unreachable connection string that connects eagerly, the default, cannot even build the store.
+    /// An unreachable connection string that connects eagerly, the default, throws while the store is
-    /// The lazily connecting form a deployment uses builds one, and the startup read in
+    /// being built rather than on a read. A failed singleton factory is not cached, so every resolve
-    /// <see cref="QuickConnectStartupTests"/> is what stops the server coming up on that.
+    /// throws afresh, which is what lets the startup probe in <see cref="QuickConnectStartupTests"/>
+    /// retry the build and report an eager store's outage as the same operator-facing failure it reports
+    /// for the lazily connecting form.
     /// </summary>
     [Fact]
     public void UnreachableRedisAtStartup_FailsClosed()
@@ -120,6 +122,7 @@ public sealed class QuickConnectStoreWiringTests : IAsyncLifetime
         using var provider = BuildProvider();
 
         Assert.ThrowsAny<RedisConnectionException>(() => provider.GetRequiredService<IQuickConnectStore>());
+        Assert.ThrowsAny<RedisConnectionException>(() => provider.GetRequiredService<IQuickConnectStore>());
     }
 
     private static QuickConnectResult NewRequest() => new QuickConnectResult(