From a99ca458bfb16dc14d3a8e2edd3b1b3c28887366 Mon Sep 17 00:00:00 2001 From: unkin-agent Date: Sat, 26 Sep 2026 19:09:24 +1000 Subject: [PATCH 1/4] fail startup when the quick connect store is unreachable Read the configured quick connect store during InitializeServices and abort startup, with a log line naming valkey, instead of coming up and failing on the first request that needs the store. --- .../ApplicationHost.cs | 29 ++ ...ConnectStoreServiceCollectionExtensions.cs | 5 +- .../QuickConnect/QuickConnectStartupTests.cs | 264 ++++++++++++++++++ .../QuickConnectStoreWiringTests.cs | 6 +- 4 files changed, 299 insertions(+), 5 deletions(-) create mode 100644 tests/Jellyfin.Server.Tests/QuickConnect/QuickConnectStartupTests.cs diff --git a/Emby.Server.Implementations/ApplicationHost.cs b/Emby.Server.Implementations/ApplicationHost.cs index 5f42a24825..e785aa4ebf 100644 --- a/Emby.Server.Implementations/ApplicationHost.cs +++ b/Emby.Server.Implementations/ApplicationHost.cs @@ -124,6 +124,12 @@ namespace Emby.Server.Implementations /// public abstract class ApplicationHost : IServerApplicationHost, IDisposable { + /// + /// The secret the startup read of the quick connect store looks for. No flow ever mints it, so the + /// read is always a miss and only its reachability is being asked about. + /// + private const string StartupProbeSecret = "startup-probe"; + /// /// The disposable parts. /// @@ -644,6 +650,8 @@ namespace Emby.Server.Implementations /// A task representing the service initialization operation. public async Task InitializeServices(IConfiguration startupConfig) { + await ProbeQuickConnectStoreAsync().ConfigureAwait(false); + var localizationManager = (LocalizationManager)Resolve(); await localizationManager.LoadAll().ConfigureAwait(false); @@ -652,6 +660,27 @@ namespace Emby.Server.Implementations FindParts(); } + /// + /// Reads the quick connect store once here so a store that cannot be reached stops startup, rather + /// than being discovered on the first request that needs it. A read rather than a resolve because a + /// shared store built with abortConnect=false constructs without touching the network. + /// + private async Task ProbeQuickConnectStoreAsync() + { + try + { + await Resolve().GetRequestBySecretAsync(StartupProbeSecret).ConfigureAwait(false); + } + catch (Exception ex) + { + Logger.LogCritical( + ex, + "Quick connect is configured against the shared valkey/Redis store at {Key} and it is UNREACHABLE, so the server will not start. Bring valkey up, or clear that setting to keep quick connect state on this instance alone.", + TranscodeStoreOptions.RedisConnectionStringKey); + throw; + } + } + private X509Certificate2 GetCertificate(string path, string password) { if (string.IsNullOrWhiteSpace(path)) diff --git a/Jellyfin.Server/Extensions/QuickConnectStoreServiceCollectionExtensions.cs b/Jellyfin.Server/Extensions/QuickConnectStoreServiceCollectionExtensions.cs index aa13634a9e..db7bf9119c 100644 --- a/Jellyfin.Server/Extensions/QuickConnectStoreServiceCollectionExtensions.cs +++ b/Jellyfin.Server/Extensions/QuickConnectStoreServiceCollectionExtensions.cs @@ -21,8 +21,9 @@ public static class QuickConnectStoreServiceCollectionExtensions /// /// The connection string is only set for a multi-instance deployment, which is the only shape where /// the initiate, authorize and exchange legs of one flow can land on different instances. Set but - /// unreachable is a misconfigured deployment rather than a single-instance one, so it fails rather - /// than quietly handing out a store the other instances cannot see. + /// unreachable is a misconfigured deployment rather than a single-instance one, so the store is read + /// once during startup and an unreachable one stops the server coming up, rather than quietly handing + /// out a store the other instances cannot see. /// /// The service collection. /// The configuration to read the Redis connection string from. diff --git a/tests/Jellyfin.Server.Tests/QuickConnect/QuickConnectStartupTests.cs b/tests/Jellyfin.Server.Tests/QuickConnect/QuickConnectStartupTests.cs new file mode 100644 index 0000000000..ea96960f52 --- /dev/null +++ b/tests/Jellyfin.Server.Tests/QuickConnect/QuickConnectStartupTests.cs @@ -0,0 +1,264 @@ +using System; +using System.Collections.Concurrent; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Threading.Tasks; +using Emby.Server.Implementations; +using Emby.Server.Implementations.QuickConnect; +using Jellyfin.Server.Extensions; +using Jellyfin.Server.Helpers; +using Jellyfin.Server.Migrations.Stages; +using Jellyfin.Server.ServerSetupApp; +using Jellyfin.Server.Tests.HighAvailability; +using MediaBrowser.Common.Configuration; +using MediaBrowser.Common.Extensions; +using MediaBrowser.Controller.Net; +using MediaBrowser.Controller.QuickConnect; +using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.Mvc.Testing; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Hosting; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; +using StackExchange.Redis; +using Xunit; + +namespace Jellyfin.Server.Tests.QuickConnect; + +/// +/// Brings the server up the way Program does - the real host over , the +/// startup and core migrations, then - to pin down what +/// a pod does when the quick connect store it is configured against cannot be reached. +/// +[Trait("Category", "RequiresDocker")] +[Collection("JellyfinSectionConfiguration")] +public sealed class QuickConnectStartupTests : IAsyncLifetime +{ + private const string RedisConnectionStringVariable = "Jellyfin__TranscodeStore__RedisConnectionString"; + + private RedisTestServer _redis = null!; + + /// + public async ValueTask InitializeAsync() + { + _redis = await RedisTestServer.StartAsync().ConfigureAwait(false); + } + + /// + public async ValueTask DisposeAsync() + { + Environment.SetEnvironmentVariable(RedisConnectionStringVariable, null); + await _redis.DisposeAsync().ConfigureAwait(false); + } + + /// + /// A configured but unreachable store stops the server coming up, so the outage is visible where the + /// server is started instead of arriving later as a failure on every request that needs the store. The + /// connection string carries the abortConnect=false a deployment uses, so the multiplexer + /// connects lazily and only a real read settles whether the store can be served. + /// + [Fact] + public void UnreachableStore_StopsStartup() + { + Environment.SetEnvironmentVariable( + RedisConnectionStringVariable, + "127.0.0.1:1,abortConnect=false,connectTimeout=250,connectRetry=0,syncTimeout=250"); + + using var server = new StartupHarness(); + + Assert.ThrowsAny(() => server.Services); + Assert.Contains( + server.CriticalEntries, + entry => entry.Contains("Quick connect", StringComparison.Ordinal) + && entry.Contains("UNREACHABLE", StringComparison.Ordinal) + && entry.Contains("valkey", StringComparison.Ordinal)); + } + + /// + /// A reachable store lets the server come up, and the quick connect it comes up with holds its + /// requests in the shared store every instance reads. + /// + /// A representing the asynchronous operation. + [Fact] + public async Task ReachableStore_StartsAndServesQuickConnect() + { + Environment.SetEnvironmentVariable(RedisConnectionStringVariable, _redis.ConnectionString); + + using var server = new StartupHarness(); + + Assert.IsType(server.Services.GetRequiredService()); + + var quickConnect = server.Services.GetRequiredService(); + var request = await quickConnect.TryConnect(NewAuthorizationInfo()); + + Assert.Equal(request.Code, (await quickConnect.CheckRequestStatus(request.Secret)).Code); + + await using var redis = await _redis.ConnectAsync(); + Assert.True(await redis.GetDatabase().KeyExistsAsync("jellyfin:quickconnect:request:" + request.Secret)); + } + + /// + /// Without a connection string the deployment is single-instance, and it starts on the process-local + /// store upstream uses. + /// + /// A representing the asynchronous operation. + [Fact] + public async Task NoConnectionString_StartsOnTheProcessLocalStore() + { + Environment.SetEnvironmentVariable(RedisConnectionStringVariable, null); + + using var server = new StartupHarness(); + + Assert.IsType(server.Services.GetRequiredService()); + + var quickConnect = server.Services.GetRequiredService(); + var request = await quickConnect.TryConnect(NewAuthorizationInfo()); + + Assert.Equal(request.Code, (await quickConnect.CheckRequestStatus(request.Secret)).Code); + } + + private static AuthorizationInfo NewAuthorizationInfo() => new AuthorizationInfo + { + DeviceId = Guid.NewGuid().ToString("N"), + Device = "Living Room TV", + Client = "Jellyfin Web", + Version = "1.0.0" + }; + + private sealed class StartupHarness : WebApplicationFactory + { + private readonly ConcurrentBag _disposables = new(); + private readonly ConcurrentQueue _criticalEntries = new(); + private readonly string _root = Path.Combine( + Path.GetTempPath(), + "jellyfin-quickconnect-startup", + Path.GetRandomFileName()); + + static StartupHarness() + { + StartupHelpers.PerformStaticInitialization(); + } + + public IReadOnlyCollection CriticalEntries => _criticalEntries.ToArray(); + + protected override IHostBuilder CreateHostBuilder() => new HostBuilder(); + + protected override void ConfigureWebHost(IWebHostBuilder builder) + { + Environment.SetEnvironmentVariable("JELLYFIN_FFMPEG__NOVALIDATION", "true"); + + var commandLineOpts = new StartupOptions(); + Directory.CreateDirectory(Path.Combine(_root, "logs")); + Directory.CreateDirectory(Path.Combine(_root, "config")); + Directory.CreateDirectory(Path.Combine(_root, "cache")); + Directory.CreateDirectory(Path.Combine(_root, "jellyfin-web")); + var appPaths = new ServerApplicationPaths( + _root, + Path.Combine(_root, "logs"), + Path.Combine(_root, "config"), + Path.Combine(_root, "cache"), + Path.Combine(_root, "jellyfin-web")); + + StartupHelpers.InitLoggingConfigFile(appPaths).GetAwaiter().GetResult(); + + var startupConfig = Program.CreateAppConfiguration(commandLineOpts, appPaths); + + ILoggerFactory loggerFactory = LoggerFactory.Create(logging => logging + .SetMinimumLevel(LogLevel.Critical) + .AddProvider(new CriticalEntryProvider(_criticalEntries))); + _disposables.Add(loggerFactory); + + var appHost = new CoreAppHost(appPaths, loggerFactory, commandLineOpts, startupConfig); + _disposables.Add(appHost); + + builder.ConfigureServices(services => appHost.Init(services)) + .ConfigureWebHostBuilder(appHost, startupConfig, appPaths, NullLogger.Instance) + .ConfigureAppConfiguration((context, configuration) => configuration + .SetBasePath(appPaths.ConfigurationDirectoryPath) + .AddInMemoryCollection(Emby.Server.Implementations.ConfigurationOptions.DefaultConfiguration) + .AddEnvironmentVariables("JELLYFIN_") + .AddInMemoryCollection(commandLineOpts.ConvertToConfig())) + .ConfigureServices(services => services.RegisterStartupLogger()); + } + + protected override IHost CreateHost(IHostBuilder builder) + { + var host = builder.Build(); + var appHost = (CoreAppHost)host.Services.GetRequiredService(); + appHost.ServiceProvider = host.Services; + var appPaths = (ServerApplicationPaths)host.Services.GetRequiredService(); + var configuration = host.Services.GetRequiredService(); + + Program.ApplyStartupMigrationAsync(appPaths, configuration, new StartupOptions()).GetAwaiter().GetResult(); + Program.ApplyCoreMigrationsAsync(host.Services, JellyfinMigrationStageTypes.CoreInitialisation).GetAwaiter().GetResult(); + appHost.InitializeServices(configuration).GetAwaiter().GetResult(); + Program.ApplyCoreMigrationsAsync(host.Services, JellyfinMigrationStageTypes.AppInitialisation).GetAwaiter().GetResult(); + host.Start(); + + return host; + } + + protected override void Dispose(bool disposing) + { + base.Dispose(disposing); + + foreach (var disposable in _disposables) + { + disposable.Dispose(); + } + + _disposables.Clear(); + + try + { + Directory.Delete(_root, true); + } + catch (IOException) + { + // A temporary directory left behind is not worth failing a test over. + } + } + + private sealed class CriticalEntryProvider : ILoggerProvider + { + private readonly ConcurrentQueue _entries; + + public CriticalEntryProvider(ConcurrentQueue entries) + { + _entries = entries; + } + + public ILogger CreateLogger(string categoryName) => new CriticalEntryLogger(_entries); + + public void Dispose() + { + } + + private sealed class CriticalEntryLogger : ILogger + { + private readonly ConcurrentQueue _entries; + + public CriticalEntryLogger(ConcurrentQueue entries) + { + _entries = entries; + } + + public IDisposable? BeginScope(TState state) + where TState : notnull + => null; + + public bool IsEnabled(LogLevel logLevel) => logLevel == LogLevel.Critical; + + public void Log(LogLevel logLevel, EventId eventId, TState state, Exception? exception, Func formatter) + { + if (logLevel == LogLevel.Critical) + { + _entries.Enqueue(formatter!(state, exception)); + } + } + } + } + } +} diff --git a/tests/Jellyfin.Server.Tests/QuickConnect/QuickConnectStoreWiringTests.cs b/tests/Jellyfin.Server.Tests/QuickConnect/QuickConnectStoreWiringTests.cs index b492f44d98..f16fee5c40 100644 --- a/tests/Jellyfin.Server.Tests/QuickConnect/QuickConnectStoreWiringTests.cs +++ b/tests/Jellyfin.Server.Tests/QuickConnect/QuickConnectStoreWiringTests.cs @@ -108,9 +108,9 @@ public sealed class QuickConnectStoreWiringTests : IAsyncLifetime } /// - /// A connection string that is set but unreachable is a misconfigured multi-instance deployment. It - /// fails rather than handing out a store the other instances cannot see, which would put quick - /// connect back on the cross-instance behaviour this configuration exists to fix. + /// An unreachable connection string that connects eagerly, the default, cannot even build the store. + /// The lazily connecting form a deployment uses builds one, and the startup read in + /// is what stops the server coming up on that. /// [Fact] public void UnreachableRedisAtStartup_FailsClosed() -- 2.47.3 From 624d528d28e44763a587039c8386657408fdcce8 Mon Sep 17 00:00:00 2001 From: unkin-agent Date: Sat, 26 Sep 2026 19:30:43 +1000 Subject: [PATCH 2/4] install libfontconfig1 for the docker test step The startup tests build a real app host, which probes the Skia encoder, and loading libSkiaSharp needs fontconfig. --- .woodpecker/ci.yaml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.woodpecker/ci.yaml b/.woodpecker/ci.yaml index 668be16db5..1871315a0f 100644 --- a/.woodpecker/ci.yaml +++ b/.woodpecker/ci.yaml @@ -59,7 +59,9 @@ steps: JELLYFIN_TEST_REDIS: "127.0.0.1:6379" commands: - apt-get -o Acquire::Retries=3 update || apt-get -o Acquire::Retries=3 update - - DEBIAN_FRONTEND=noninteractive apt-get install -y --no-install-recommends postgresql valkey-server + # libfontconfig1 is needed here too: the startup tests build a real app host, which probes + # the Skia encoder, and loading libSkiaSharp pulls fontconfig in. + - DEBIAN_FRONTEND=noninteractive apt-get install -y --no-install-recommends postgresql valkey-server libfontconfig1 - install -d -o postgres -g postgres /tmp/pgdata /tmp/pgrun - PGBIN=$(ls -d /usr/lib/postgresql/*/bin | tail -1) - su postgres -c "$PGBIN/initdb -D /tmp/pgdata -A trust -U postgres" -- 2.47.3 From 6c7f76fd2665096a51389992b66cc12f2fd56b0a Mon Sep 17 00:00:00 2001 From: unkin-agent Date: Sat, 26 Sep 2026 22:14:14 +1000 Subject: [PATCH 3/4] retry the quick connect startup probe, and exit non-zero when a start fails --- .../ApplicationHost.cs | 52 +++- .../TranscodeStoreConnectivityProbe.cs | 11 +- .../ScheduledTasks/RedisScanLeaderLease.cs | 5 + ...anscodeStoreServiceCollectionExtensions.cs | 7 + Jellyfin.Server/Program.cs | 16 +- docs/FORK-DIFF.md | 23 ++ .../TranscodeStoreConnectivityProbeTests.cs | 7 +- .../QuickConnect/QuickConnectStartupTests.cs | 255 +++++++++++++++--- 8 files changed, 312 insertions(+), 64 deletions(-) diff --git a/Emby.Server.Implementations/ApplicationHost.cs b/Emby.Server.Implementations/ApplicationHost.cs index e785aa4ebf..4826d7e6d6 100644 --- a/Emby.Server.Implementations/ApplicationHost.cs +++ b/Emby.Server.Implementations/ApplicationHost.cs @@ -130,6 +130,15 @@ namespace Emby.Server.Implementations /// private const string StartupProbeSecret = "startup-probe"; + /// + /// How long the startup read of the quick connect store is retried before the store counts as + /// unreachable. Long enough to ride out valkey restarting alongside this instance, short enough + /// that a store which is really gone is reported inside one liveness cycle. + /// + private static readonly TimeSpan _quickConnectProbeDeadline = TimeSpan.FromSeconds(30); + + private static readonly TimeSpan _quickConnectProbeRetryDelay = TimeSpan.FromSeconds(1); + /// /// The disposable parts. /// @@ -661,23 +670,40 @@ namespace Emby.Server.Implementations } /// - /// Reads the quick connect store once here so a store that cannot be reached stops startup, rather - /// than being discovered on the first request that needs it. A read rather than a resolve because a - /// shared store built with abortConnect=false constructs without touching the network. + /// Reads the quick connect store here so a store that cannot be reached stops startup, rather than + /// being discovered on the first request that needs it. A read rather than a resolve because a + /// shared store built with abortConnect=false constructs without touching the network, and + /// retried until so a starting instance rides out the blip + /// a running one already tolerates. /// private async Task ProbeQuickConnectStoreAsync() { - try + var store = Resolve(); + var startTimestamp = Stopwatch.GetTimestamp(); + + while (true) { - await Resolve().GetRequestBySecretAsync(StartupProbeSecret).ConfigureAwait(false); - } - catch (Exception ex) - { - Logger.LogCritical( - ex, - "Quick connect is configured against the shared valkey/Redis store at {Key} and it is UNREACHABLE, so the server will not start. Bring valkey up, or clear that setting to keep quick connect state on this instance alone.", - TranscodeStoreOptions.RedisConnectionStringKey); - throw; + try + { + await store.GetRequestBySecretAsync(StartupProbeSecret).ConfigureAwait(false); + return; + } + catch (Exception ex) + { + if (Stopwatch.GetElapsedTime(startTimestamp) + _quickConnectProbeRetryDelay < _quickConnectProbeDeadline) + { + Logger.LogWarning(ex, "Quick connect store is not reachable yet, retrying."); + await Task.Delay(_quickConnectProbeRetryDelay).ConfigureAwait(false); + continue; + } + + Logger.LogCritical( + 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); + throw; + } } } diff --git a/Emby.Server.Implementations/MediaEncoding/TranscodeStoreConnectivityProbe.cs b/Emby.Server.Implementations/MediaEncoding/TranscodeStoreConnectivityProbe.cs index 2c41875ced..400b986c3e 100644 --- a/Emby.Server.Implementations/MediaEncoding/TranscodeStoreConnectivityProbe.cs +++ b/Emby.Server.Implementations/MediaEncoding/TranscodeStoreConnectivityProbe.cs @@ -10,9 +10,14 @@ using StackExchange.Redis; namespace Emby.Server.Implementations.MediaEncoding; /// -/// Pings the configured Redis transcode session store once at startup so an unreachable store is -/// reported there instead of being discovered as a silent loss of cross-pod takeover. +/// Reports the round trip to the configured Redis transcode session store once at startup, so the +/// state of cross-pod takeover is visible where the server is started. /// +/// +/// This reports, it does not gate. has already read the +/// same connection for quick connect by the time this runs and has stopped startup if it could not be +/// reached, so the error branch here only covers a store that went away in between. +/// public sealed class TranscodeStoreConnectivityProbe : IHostedService { private readonly IServiceProvider _serviceProvider; @@ -34,7 +39,7 @@ public sealed class TranscodeStoreConnectivityProbe : IHostedService { try { - // Resolved here rather than injected: connecting must not be able to abort startup. + // Resolved here rather than injected so a store lost after the quick connect gate is reported. var redis = _serviceProvider.GetRequiredService(); var roundTrip = await redis.GetDatabase().PingAsync().ConfigureAwait(false); diff --git a/Emby.Server.Implementations/ScheduledTasks/RedisScanLeaderLease.cs b/Emby.Server.Implementations/ScheduledTasks/RedisScanLeaderLease.cs index a95e06fe5d..a07fac49e3 100644 --- a/Emby.Server.Implementations/ScheduledTasks/RedisScanLeaderLease.cs +++ b/Emby.Server.Implementations/ScheduledTasks/RedisScanLeaderLease.cs @@ -13,6 +13,11 @@ namespace Emby.Server.Implementations.ScheduledTasks; /// TTL lease on a shared key. The lease value is this instance's pod identity; a leader that keeps /// renewing retains the lease, and any instance can claim it once the previous leader's lease expires. /// +/// +/// This fails open on an unreachable Redis while quick connect's startup read fails closed on the same +/// connection. They do not compete: the startup read decides whether the instance runs at all, and this +/// only decides what a running instance does about a store that went away afterwards. +/// public sealed class RedisScanLeaderLease : IScanLeaderLease { private const string LeaderKey = "jellyfin:scanleader"; diff --git a/Jellyfin.Server/Extensions/TranscodeStoreServiceCollectionExtensions.cs b/Jellyfin.Server/Extensions/TranscodeStoreServiceCollectionExtensions.cs index 60ca77817d..39bae1e4ac 100644 --- a/Jellyfin.Server/Extensions/TranscodeStoreServiceCollectionExtensions.cs +++ b/Jellyfin.Server/Extensions/TranscodeStoreServiceCollectionExtensions.cs @@ -18,6 +18,13 @@ public static class TranscodeStoreServiceCollectionExtensions /// Registers the transcode session store, Redis-backed when a connection string is configured and /// no-op otherwise, and reports the selected store at . /// + /// + /// The registered here is the one connection every Redis-backed + /// component shares, so configuring it makes valkey a hard startup dependency: quick connect reads it + /// during InitializeServices and stops the server when it cannot be reached. The softer + /// policies elsewhere - this store's connectivity report, the scan leader's fail-open - are + /// subordinate to that and only govern a store that goes away after that read has passed. + /// /// The service collection. /// The configuration to read Jellyfin:TranscodeStore from. /// The logger to report the selected store on. diff --git a/Jellyfin.Server/Program.cs b/Jellyfin.Server/Program.cs index 5530cfae93..01fc5cd2d6 100644 --- a/Jellyfin.Server/Program.cs +++ b/Jellyfin.Server/Program.cs @@ -54,6 +54,13 @@ namespace Jellyfin.Server /// public const string LoggingConfigFileSystem = "logging.json"; + /// + /// How long a failed start keeps the setup server answering before the process exits, so an + /// install without a supervisor shows the failure instead of spinning. Skipped under an + /// orchestrator, where restarting is its job and holding only stretches the crash loop. + /// + private static readonly TimeSpan _setupServerHoldAfterFailedStart = TimeSpan.FromMinutes(10); + private static readonly SerilogLoggerFactory _loggerFactory = new SerilogLoggerFactory(); private static SetupServer? _setupServer; private static CoreAppHost? _appHost; @@ -255,13 +262,14 @@ namespace Jellyfin.Server catch (Exception ex) { _restartOnShutdown = false; + Environment.ExitCode = 1; _logger.LogCritical(ex, "Error while starting server"); if (_setupServer!.IsAlive && !configurationCompleted) { _setupServer!.SoftStop(); - if (options.StartupMode is null or Configuration.StartupMode.MediaServer) + if (options.StartupMode is null or Configuration.StartupMode.MediaServer && !IsRunningInContainer()) { - await Task.Delay(TimeSpan.FromMinutes(10)).ConfigureAwait(false); + await Task.Delay(_setupServerHoldAfterFailedStart).ConfigureAwait(false); } await _setupServer!.StopAsync().ConfigureAwait(false); @@ -285,6 +293,10 @@ namespace Jellyfin.Server } } + // Set by every official .NET base image, including the one this server ships in. + private static bool IsRunningInContainer() + => string.Equals(Environment.GetEnvironmentVariable("DOTNET_RUNNING_IN_CONTAINER"), "true", StringComparison.OrdinalIgnoreCase); + /// /// [Internal]Runs the startup Migrations. /// diff --git a/docs/FORK-DIFF.md b/docs/FORK-DIFF.md index dfd4f7fb7f..36905a186f 100644 --- a/docs/FORK-DIFF.md +++ b/docs/FORK-DIFF.md @@ -83,6 +83,29 @@ Scan-leader gating is off: timer-driven library tasks run on every instance. `Enabled=true` with no Redis connection string logs a warning, because gating cannot run. +### Valkey is a hard startup dependency + +`Jellyfin:TranscodeStore:RedisConnectionString` selects one shared `IConnectionMultiplexer` and every +Redis-backed component hangs off it. Three of them disagree about an unreachable store on purpose, and +the order they run in is what makes that coherent: + +| Component | On an unreachable store | When | +|---|---|---| +| Quick connect store (`ApplicationHost.ProbeQuickConnectStoreAsync`) | **Fails closed.** Reads a sentinel secret, retried for 30s, then logs `Critical` and stops startup | `InitializeServices`, before anything is served | +| `TranscodeStoreConnectivityProbe` | Logs `Error` and carries on | `IHostedService` start, after the gate | +| `RedisScanLeaderLease` | Fails open, treats itself as leader | Per scheduled-task tick, long after the gate | + +The gate wins because it runs first: with a connection string set, an instance that reaches +`IHostedService` start has already proved the store reachable. The softer policies govern only a store +that goes away *afterwards*, where a running instance degrades rather than dying — quick connect calls +return `503`, transcode takeover stops, every instance scans. + +The 30s window is there so a rollout survives valkey restarting alongside the server. Past it the +deployment is misconfigured or broken, the process exits non-zero and the orchestrator reports the real +cause. Upstream's 10-minute hold, which keeps the setup server answering after *any* failed start, is +skipped when `DOTNET_RUNNING_IN_CONTAINER` is set, because there the restart is the supervisor's job and +holding only stretches the crash loop. + ### PostgreSQL provider `src/Jellyfin.Database/Jellyfin.Database.Providers.PostgreSQL/` is an EF Core diff --git a/tests/Jellyfin.Server.Tests/HighAvailability/TranscodeStoreConnectivityProbeTests.cs b/tests/Jellyfin.Server.Tests/HighAvailability/TranscodeStoreConnectivityProbeTests.cs index 3f952a6001..37304a033e 100644 --- a/tests/Jellyfin.Server.Tests/HighAvailability/TranscodeStoreConnectivityProbeTests.cs +++ b/tests/Jellyfin.Server.Tests/HighAvailability/TranscodeStoreConnectivityProbeTests.cs @@ -11,9 +11,10 @@ using Xunit; namespace Jellyfin.Server.Tests.HighAvailability; /// -/// A Redis store that cannot be reached degrades silently: the client is configured not to abort the -/// connection and every call site swallows failures. The probe is the only startup signal, so both of -/// its outcomes are pinned here. +/// A transcode store that cannot be reached degrades silently: the client is configured not to abort +/// the connection and every call site swallows failures. Quick connect's startup read is what stops a +/// pod coming up against a dead store; this probe only reports it, so both of its outcomes are pinned +/// here - including that it never throws, which is what keeps the two policies from competing. /// public sealed class TranscodeStoreConnectivityProbeTests { diff --git a/tests/Jellyfin.Server.Tests/QuickConnect/QuickConnectStartupTests.cs b/tests/Jellyfin.Server.Tests/QuickConnect/QuickConnectStartupTests.cs index ea96960f52..f6e29aa099 100644 --- a/tests/Jellyfin.Server.Tests/QuickConnect/QuickConnectStartupTests.cs +++ b/tests/Jellyfin.Server.Tests/QuickConnect/QuickConnectStartupTests.cs @@ -1,9 +1,15 @@ using System; using System.Collections.Concurrent; using System.Collections.Generic; +using System.Diagnostics; using System.IO; using System.Linq; +using System.Net; +using System.Net.Sockets; +using System.Text; +using System.Threading; using System.Threading.Tasks; +using System.Xml.Serialization; using Emby.Server.Implementations; using Emby.Server.Implementations.QuickConnect; using Jellyfin.Server.Extensions; @@ -13,6 +19,7 @@ using Jellyfin.Server.ServerSetupApp; using Jellyfin.Server.Tests.HighAvailability; using MediaBrowser.Common.Configuration; using MediaBrowser.Common.Extensions; +using MediaBrowser.Common.Net; using MediaBrowser.Controller.Net; using MediaBrowser.Controller.QuickConnect; using Microsoft.AspNetCore.Hosting; @@ -37,12 +44,15 @@ namespace Jellyfin.Server.Tests.QuickConnect; 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 RedisTestServer _redis = null!; /// public async ValueTask InitializeAsync() { + Environment.SetEnvironmentVariable(FfmpegNoValidationVariable, "true"); _redis = await RedisTestServer.StartAsync().ConfigureAwait(false); } @@ -50,21 +60,20 @@ public sealed class QuickConnectStartupTests : IAsyncLifetime public async ValueTask DisposeAsync() { Environment.SetEnvironmentVariable(RedisConnectionStringVariable, null); + Environment.SetEnvironmentVariable(FfmpegNoValidationVariable, null); await _redis.DisposeAsync().ConfigureAwait(false); } /// - /// A configured but unreachable store stops the server coming up, so the outage is visible where the - /// server is started instead of arriving later as a failure on every request that needs the store. The - /// connection string carries the abortConnect=false a deployment uses, so the multiplexer - /// connects lazily and only a real read settles whether the store can be served. + /// A store that stays away past the probe's deadline stops the server coming up, so the outage is + /// visible where the server is started instead of arriving later as a failure on every request that + /// needs the store. The connection string carries the abortConnect=false a deployment uses, so + /// the multiplexer connects lazily and only a real read settles whether the store can be served. /// [Fact] - public void UnreachableStore_StopsStartup() + public void StoreDownPastTheDeadline_StopsStartup() { - Environment.SetEnvironmentVariable( - RedisConnectionStringVariable, - "127.0.0.1:1,abortConnect=false,connectTimeout=250,connectRetry=0,syncTimeout=250"); + Environment.SetEnvironmentVariable(RedisConnectionStringVariable, DeadStore); using var server = new StartupHarness(); @@ -76,6 +85,85 @@ public sealed class QuickConnectStartupTests : IAsyncLifetime && entry.Contains("valkey", StringComparison.Ordinal)); } + /// + /// 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 + /// rollout is hostage to valkey restarting at the same time. + /// + /// A representing the asynchronous operation. + [Fact] + public async Task StoreBackInsideTheDeadline_StartsAnyway() + { + await using var proxy = RedisFaultProxy.Start(_redis.ConnectionString); + Environment.SetEnvironmentVariable(RedisConnectionStringVariable, proxy.ConnectionString); + + using var server = new StartupHarness(() => + { + proxy.Cut(); + _ = Task.Run(async () => + { + await Task.Delay(TimeSpan.FromSeconds(3)).ConfigureAwait(false); + proxy.Restore(); + }); + }); + + Assert.IsType(server.Services.GetRequiredService()); + Assert.Contains(server.WarningEntries, entry => entry.Contains("not reachable yet", StringComparison.Ordinal)); + Assert.Empty(server.CriticalEntries); + } + + /// + /// A start that failed has to look like a failure to whatever supervises the process. The server runs + /// as its own process here because the exit code is not observable anywhere else, and in a container + /// it must not sit on the setup server for ten minutes before getting there. + /// + /// A representing the asynchronous operation. + [Fact] + public async Task StoreDownPastTheDeadline_ExitsNonZero() + { + var root = Path.Combine(Path.GetTempPath(), "jellyfin-quickconnect-exit", Path.GetRandomFileName()); + var configDirectory = Path.Combine(root, "config"); + Directory.CreateDirectory(configDirectory); + Directory.CreateDirectory(Path.Combine(root, "cache")); + WriteNetworkConfiguration(configDirectory); + + var startInfo = new ProcessStartInfo(Environment.GetEnvironmentVariable("DOTNET_HOST_PATH") ?? "dotnet") + { + RedirectStandardOutput = true, + RedirectStandardError = true, + UseShellExecute = false, + WorkingDirectory = AppContext.BaseDirectory + }; + + foreach (var argument in new[] + { + "exec", + Path.Combine(AppContext.BaseDirectory, "jellyfin.dll"), + "--datadir", root, + "--cachedir", Path.Combine(root, "cache"), + "--nowebclient" + }) + { + startInfo.ArgumentList.Add(argument); + } + + startInfo.Environment[RedisConnectionStringVariable] = DeadStore; + startInfo.Environment[FfmpegNoValidationVariable] = "true"; + startInfo.Environment["DOTNET_RUNNING_IN_CONTAINER"] = "true"; + + try + { + var (exitCode, output) = await RunToCompletionAsync(startInfo, TimeSpan.FromMinutes(5)); + + Assert.Contains("UNREACHABLE", output, StringComparison.Ordinal); + Assert.Equal(1, exitCode); + } + finally + { + TryDelete(root); + } + } + /// /// A reachable store lets the server come up, and the quick connect it comes up with holds its /// requests in the shared store every instance reads. @@ -127,10 +215,79 @@ public sealed class QuickConnectStartupTests : IAsyncLifetime Version = "1.0.0" }; + // The setup server binds before anything else runs, so the spawned server gets a port of its own. + private static void WriteNetworkConfiguration(string configDirectory) + { + var listener = new TcpListener(IPAddress.Loopback, 0); + listener.Start(); + var port = ((IPEndPoint)listener.LocalEndpoint).Port; + listener.Stop(); + + var configuration = new NetworkConfiguration + { + InternalHttpPort = port, + PublicHttpPort = port, + EnableHttps = false, + AutoDiscovery = false + }; + + using var writer = new StreamWriter(Path.Combine(configDirectory, "network.xml")); + new XmlSerializer(typeof(NetworkConfiguration)).Serialize(writer, configuration); + } + + private static async Task<(int ExitCode, string Output)> RunToCompletionAsync(ProcessStartInfo startInfo, TimeSpan timeout) + { + var output = new StringBuilder(); + using var process = new Process { StartInfo = startInfo, EnableRaisingEvents = true }; + process.OutputDataReceived += (_, e) => Append(output, e.Data); + process.ErrorDataReceived += (_, e) => Append(output, e.Data); + + process.Start(); + process.BeginOutputReadLine(); + process.BeginErrorReadLine(); + + using var cancellation = new CancellationTokenSource(timeout); + try + { + await process.WaitForExitAsync(cancellation.Token).ConfigureAwait(false); + } + catch (OperationCanceledException) + { + process.Kill(true); + throw new TimeoutException($"The server did not exit within {timeout}. Output:\n{output}"); + } + + return (process.ExitCode, output.ToString()); + } + + private static void Append(StringBuilder output, string? line) + { + if (line is not null) + { + lock (output) + { + output.AppendLine(line); + } + } + } + + private static void TryDelete(string path) + { + try + { + Directory.Delete(path, true); + } + catch (IOException) + { + // A temporary directory left behind is not worth failing a test over. + } + } + private sealed class StartupHarness : WebApplicationFactory { private readonly ConcurrentBag _disposables = new(); - private readonly ConcurrentQueue _criticalEntries = new(); + private readonly ConcurrentQueue<(LogLevel Level, string Message)> _entries = new(); + private readonly Action? _beforeInitializeServices; private readonly string _root = Path.Combine( Path.GetTempPath(), "jellyfin-quickconnect-startup", @@ -141,14 +298,19 @@ public sealed class QuickConnectStartupTests : IAsyncLifetime StartupHelpers.PerformStaticInitialization(); } - public IReadOnlyCollection CriticalEntries => _criticalEntries.ToArray(); + public StartupHarness(Action? beforeInitializeServices = null) + { + _beforeInitializeServices = beforeInitializeServices; + } + + public IReadOnlyCollection CriticalEntries => Messages(LogLevel.Critical); + + public IReadOnlyCollection WarningEntries => Messages(LogLevel.Warning); protected override IHostBuilder CreateHostBuilder() => new HostBuilder(); protected override void ConfigureWebHost(IWebHostBuilder builder) { - Environment.SetEnvironmentVariable("JELLYFIN_FFMPEG__NOVALIDATION", "true"); - var commandLineOpts = new StartupOptions(); Directory.CreateDirectory(Path.Combine(_root, "logs")); Directory.CreateDirectory(Path.Combine(_root, "config")); @@ -166,8 +328,8 @@ public sealed class QuickConnectStartupTests : IAsyncLifetime var startupConfig = Program.CreateAppConfiguration(commandLineOpts, appPaths); ILoggerFactory loggerFactory = LoggerFactory.Create(logging => logging - .SetMinimumLevel(LogLevel.Critical) - .AddProvider(new CriticalEntryProvider(_criticalEntries))); + .SetMinimumLevel(LogLevel.Warning) + .AddProvider(new RecordingProvider(_entries))); _disposables.Add(loggerFactory); var appHost = new CoreAppHost(appPaths, loggerFactory, commandLineOpts, startupConfig); @@ -186,18 +348,29 @@ public sealed class QuickConnectStartupTests : IAsyncLifetime protected override IHost CreateHost(IHostBuilder builder) { var host = builder.Build(); - var appHost = (CoreAppHost)host.Services.GetRequiredService(); - appHost.ServiceProvider = host.Services; - var appPaths = (ServerApplicationPaths)host.Services.GetRequiredService(); - var configuration = host.Services.GetRequiredService(); + try + { + var appHost = (CoreAppHost)host.Services.GetRequiredService(); + appHost.ServiceProvider = host.Services; + var appPaths = (ServerApplicationPaths)host.Services.GetRequiredService(); + var configuration = host.Services.GetRequiredService(); - Program.ApplyStartupMigrationAsync(appPaths, configuration, new StartupOptions()).GetAwaiter().GetResult(); - Program.ApplyCoreMigrationsAsync(host.Services, JellyfinMigrationStageTypes.CoreInitialisation).GetAwaiter().GetResult(); - appHost.InitializeServices(configuration).GetAwaiter().GetResult(); - Program.ApplyCoreMigrationsAsync(host.Services, JellyfinMigrationStageTypes.AppInitialisation).GetAwaiter().GetResult(); - host.Start(); + Program.ApplyStartupMigrationAsync(appPaths, configuration, new StartupOptions()).GetAwaiter().GetResult(); + Program.ApplyCoreMigrationsAsync(host.Services, JellyfinMigrationStageTypes.CoreInitialisation).GetAwaiter().GetResult(); + _beforeInitializeServices?.Invoke(); + appHost.InitializeServices(configuration).GetAwaiter().GetResult(); + Program.ApplyCoreMigrationsAsync(host.Services, JellyfinMigrationStageTypes.AppInitialisation).GetAwaiter().GetResult(); + host.Start(); - return host; + return host; + } + catch + { + // The factory only owns the host once this returns, so a failed start disposes it here + // or the multiplexer it holds outlives the test. + host.Dispose(); + throw; + } } protected override void Dispose(bool disposing) @@ -211,36 +384,32 @@ public sealed class QuickConnectStartupTests : IAsyncLifetime _disposables.Clear(); - try - { - Directory.Delete(_root, true); - } - catch (IOException) - { - // A temporary directory left behind is not worth failing a test over. - } + TryDelete(_root); } - private sealed class CriticalEntryProvider : ILoggerProvider - { - private readonly ConcurrentQueue _entries; + private IReadOnlyCollection Messages(LogLevel level) + => _entries.Where(entry => entry.Level == level).Select(entry => entry.Message).ToArray(); - public CriticalEntryProvider(ConcurrentQueue entries) + private sealed class RecordingProvider : ILoggerProvider + { + private readonly ConcurrentQueue<(LogLevel Level, string Message)> _entries; + + public RecordingProvider(ConcurrentQueue<(LogLevel Level, string Message)> entries) { _entries = entries; } - public ILogger CreateLogger(string categoryName) => new CriticalEntryLogger(_entries); + public ILogger CreateLogger(string categoryName) => new RecordingLogger(_entries); public void Dispose() { } - private sealed class CriticalEntryLogger : ILogger + private sealed class RecordingLogger : ILogger { - private readonly ConcurrentQueue _entries; + private readonly ConcurrentQueue<(LogLevel Level, string Message)> _entries; - public CriticalEntryLogger(ConcurrentQueue entries) + public RecordingLogger(ConcurrentQueue<(LogLevel Level, string Message)> entries) { _entries = entries; } @@ -249,13 +418,13 @@ public sealed class QuickConnectStartupTests : IAsyncLifetime where TState : notnull => null; - public bool IsEnabled(LogLevel logLevel) => logLevel == LogLevel.Critical; + public bool IsEnabled(LogLevel logLevel) => logLevel >= LogLevel.Warning; public void Log(LogLevel logLevel, EventId eventId, TState state, Exception? exception, Func formatter) { - if (logLevel == LogLevel.Critical) + if (IsEnabled(logLevel)) { - _entries.Enqueue(formatter!(state, exception)); + _entries.Enqueue((logLevel, formatter!(state, exception))); } } } -- 2.47.3 From 33a5fbce9b467ec2f528298ed577ee3ba1240f5e Mon Sep 17 00:00:00 2001 From: unkin-agent Date: Sat, 26 Sep 2026 22:56:28 +1000 Subject: [PATCH 4/4] retry building the quick connect store in the startup probe, not just reading it --- .../ApplicationHost.cs | 43 ++++++++++++++----- Jellyfin.Server/Program.cs | 2 +- .../QuickConnect/QuickConnectStartupTests.cs | 38 +++++++++++++++- .../QuickConnectStoreWiringTests.cs | 9 ++-- 4 files changed, 76 insertions(+), 16 deletions(-) diff --git a/Emby.Server.Implementations/ApplicationHost.cs b/Emby.Server.Implementations/ApplicationHost.cs index 4826d7e6d6..83291a8ebb 100644 --- a/Emby.Server.Implementations/ApplicationHost.cs +++ b/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 } /// - /// Reads the quick connect store here so a store that cannot be reached stops startup, rather than - /// being discovered on the first request that needs it. A read rather than a resolve because a - /// shared store built with abortConnect=false constructs without touching the network, and - /// retried until so a starting instance rides out the blip - /// a running one already tolerates. + /// Builds and reads the quick connect store here so a store that cannot be reached stops startup, + /// rather than being discovered on the first request that needs it. Both halves sit inside the + /// retry: a store built with abortConnect=false constructs without touching the network and + /// only the read settles it, while one built without that option connects eagerly and fails at the + /// resolve. Retried until so a starting instance rides out + /// the blip a running one already tolerates. /// private async Task ProbeQuickConnectStoreAsync() { - var store = Resolve(); 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(); 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); - throw; + (int)elapsed.TotalSeconds); + + if (ex is ServiceUnavailableException) + { + throw; + } + + throw new ServiceUnavailableException("Quick connect store is unreachable.", ex); } } } diff --git a/Jellyfin.Server/Program.cs b/Jellyfin.Server/Program.cs index 01fc5cd2d6..0a850d56a0 100644 --- a/Jellyfin.Server/Program.cs +++ b/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); } diff --git a/tests/Jellyfin.Server.Tests/QuickConnect/QuickConnectStartupTests.cs b/tests/Jellyfin.Server.Tests/QuickConnect/QuickConnectStartupTests.cs index f6e29aa099..fe8fa82010 100644 --- a/tests/Jellyfin.Server.Tests/QuickConnect/QuickConnectStartupTests.cs +++ b/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)); } + /// + /// Three of the four connection strings the chart documents leave abortConnect 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 out. + /// + /// + /// The core initialisation migrations are skipped here because JellyfinMigrationService takes + /// an IBackupService 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. + /// + [Fact] + public void EagerlyConnectingStoreDownPastTheDeadline_StopsStartupWithTheSameMessage() + { + Environment.SetEnvironmentVariable(RedisConnectionStringVariable, EagerDeadStore); + + using var server = new StartupHarness(runCoreInitialisationMigrations: false); + + Assert.ThrowsAny(() => server.Services); + Assert.Contains( + server.CriticalEntries, + entry => entry.Contains("Quick connect", StringComparison.Ordinal) + && entry.Contains("UNREACHABLE", StringComparison.Ordinal) + && entry.Contains("valkey", StringComparison.Ordinal)); + } + /// /// 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 _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 CriticalEntries => Messages(LogLevel.Critical); @@ -356,7 +386,11 @@ public sealed class QuickConnectStartupTests : IAsyncLifetime var configuration = host.Services.GetRequiredService(); 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(); diff --git a/tests/Jellyfin.Server.Tests/QuickConnect/QuickConnectStoreWiringTests.cs b/tests/Jellyfin.Server.Tests/QuickConnect/QuickConnectStoreWiringTests.cs index f16fee5c40..73c33bedf5 100644 --- a/tests/Jellyfin.Server.Tests/QuickConnect/QuickConnectStoreWiringTests.cs +++ b/tests/Jellyfin.Server.Tests/QuickConnect/QuickConnectStoreWiringTests.cs @@ -108,9 +108,11 @@ public sealed class QuickConnectStoreWiringTests : IAsyncLifetime } /// - /// An unreachable connection string that connects eagerly, the default, cannot even build the store. - /// The lazily connecting form a deployment uses builds one, and the startup read in - /// is what stops the server coming up on that. + /// An unreachable connection string that connects eagerly, the default, throws while the store is + /// being built rather than on a read. A failed singleton factory is not cached, so every resolve + /// throws afresh, which is what lets the startup probe in + /// retry the build and report an eager store's outage as the same operator-facing failure it reports + /// for the lazily connecting form. /// [Fact] public void UnreachableRedisAtStartup_FailsClosed() @@ -120,6 +122,7 @@ public sealed class QuickConnectStoreWiringTests : IAsyncLifetime using var provider = BuildProvider(); Assert.ThrowsAny(() => provider.GetRequiredService()); + Assert.ThrowsAny(() => provider.GetRequiredService()); } private static QuickConnectResult NewRequest() => new QuickConnectResult( -- 2.47.3