From 91655fcb0a3f208b24b91f96d8ab46abe45a4a7e Mon Sep 17 00:00:00 2001 From: unkin-agent Date: Sun, 13 Sep 2026 13:19:48 +1000 Subject: [PATCH] fix(ha): gate library tasks on the scan leader by default Jellyfin:ScanLeader:Enabled defaults to false and nothing sets it, so leader election never runs and every replica executes the timer-driven library tasks concurrently - the exact behaviour the lease prevents. - Enable gating by default when a Redis connection string is configured - Honour an explicit Enabled setting either way - Carry the effective decision onto the bound options the task worker reads - Log at startup whether gating is active - Warn when gating is enabled but no Redis connection string is configured --- Jellyfin.Server/CoreAppHost.cs | 16 +- .../ScanLeaderServiceCollectionExtensions.cs | 71 +++++++++ .../ScheduledTasks/ScanLeaderOptions.cs | 13 +- docs/FORK-DIFF.md | 13 +- .../ScanLeader/ScanLeaderRegistrationTests.cs | 144 ++++++++++++++++++ 5 files changed, 242 insertions(+), 15 deletions(-) create mode 100644 Jellyfin.Server/Extensions/ScanLeaderServiceCollectionExtensions.cs create mode 100644 tests/Jellyfin.Server.Tests/ScanLeader/ScanLeaderRegistrationTests.cs diff --git a/Jellyfin.Server/CoreAppHost.cs b/Jellyfin.Server/CoreAppHost.cs index 0077b52bf1..8e037bf742 100644 --- a/Jellyfin.Server/CoreAppHost.cs +++ b/Jellyfin.Server/CoreAppHost.cs @@ -3,13 +3,13 @@ using System.Collections.Generic; using System.Reflection; using Emby.Server.Implementations; using Emby.Server.Implementations.MediaEncoding; -using Emby.Server.Implementations.ScheduledTasks; using Emby.Server.Implementations.Session; using Jellyfin.Api.WebSocketListeners; using Jellyfin.Database.Implementations; using Jellyfin.Drawing; using Jellyfin.Drawing.Skia; using Jellyfin.LiveTv; +using Jellyfin.Server.Extensions; using Jellyfin.Server.Implementations.Activity; using Jellyfin.Server.Implementations.Devices; using Jellyfin.Server.Implementations.Events; @@ -27,7 +27,6 @@ using MediaBrowser.Controller.Library; using MediaBrowser.Controller.Lyrics; using MediaBrowser.Controller.MediaEncoding; using MediaBrowser.Controller.Net; -using MediaBrowser.Controller.ScheduledTasks; using MediaBrowser.Controller.Security; using MediaBrowser.Controller.Trickplay; using MediaBrowser.Model.Activity; @@ -132,17 +131,8 @@ namespace Jellyfin.Server } // Scan-leader lease: gates periodic library-mutating scheduled tasks to a single leader - // instance. Redis-backed when enabled and a Redis connection is configured, no-op otherwise. - serviceCollection.Configure(_startupConfig.GetSection("Jellyfin:ScanLeader")); - var scanLeaderEnabled = bool.TryParse(_startupConfig["Jellyfin:ScanLeader:Enabled"], out var enabled) && enabled; - if (scanLeaderEnabled && !string.IsNullOrEmpty(redisConnectionString)) - { - serviceCollection.AddSingleton(); - } - else - { - serviceCollection.AddSingleton(); - } + // instance. Active by default once a Redis connection is configured, no-op otherwise. + serviceCollection.AddScanLeaderLease(_startupConfig, Logger); foreach (var type in GetExportTypes()) { diff --git a/Jellyfin.Server/Extensions/ScanLeaderServiceCollectionExtensions.cs b/Jellyfin.Server/Extensions/ScanLeaderServiceCollectionExtensions.cs new file mode 100644 index 0000000000..4b7be151e0 --- /dev/null +++ b/Jellyfin.Server/Extensions/ScanLeaderServiceCollectionExtensions.cs @@ -0,0 +1,71 @@ +using System; +using Emby.Server.Implementations.ScheduledTasks; +using MediaBrowser.Controller.ScheduledTasks; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; + +namespace Jellyfin.Server.Extensions; + +/// +/// Extensions for registering the scan-leader lease. +/// +public static class ScanLeaderServiceCollectionExtensions +{ + private const string RedisConnectionStringKey = "Jellyfin:TranscodeStore:RedisConnectionString"; + + /// + /// Registers the scan-leader lease and reports at whether + /// timer-driven library tasks are gated to a single instance. + /// + /// + /// Gating is on by default once a Redis connection string is configured: that is only set for a + /// multi-instance deployment, which is the only shape where running library scans on every + /// instance is wrong. Single-instance installs have no Redis and keep running every task locally. + /// + /// The service collection. + /// The configuration to read Jellyfin:ScanLeader from. + /// The logger to report the gating decision on. + /// The updated service collection. + public static IServiceCollection AddScanLeaderLease( + this IServiceCollection serviceCollection, + IConfiguration configuration, + ILogger logger) + { + ArgumentNullException.ThrowIfNull(configuration); + ArgumentNullException.ThrowIfNull(logger); + + serviceCollection.Configure(configuration.GetSection(ScanLeaderOptions.ConfigurationSection)); + + var redisConfigured = !string.IsNullOrEmpty(configuration[RedisConnectionStringKey]); + var requested = bool.TryParse(configuration[ScanLeaderOptions.EnabledKey], out var explicitChoice) + ? explicitChoice + : redisConfigured; + var active = requested && redisConfigured; + + // The task worker reads Enabled from the bound options, so the effective decision has to land + // there as well as on the lease registration below. + serviceCollection.PostConfigure(options => options.Enabled = active); + + if (active) + { + logger.LogInformation( + "Scan-leader gating is active: timer-driven library tasks run only on the instance holding the Redis scan-leader lease."); + + return serviceCollection.AddSingleton(); + } + + if (requested) + { + logger.LogWarning( + "Scan-leader gating is enabled but no Redis connection string is configured ({Key}), so it cannot run: timer-driven library tasks run on every instance.", + RedisConnectionStringKey); + } + else + { + logger.LogInformation("Scan-leader gating is off: timer-driven library tasks run on every instance."); + } + + return serviceCollection.AddSingleton(); + } +} diff --git a/MediaBrowser.Controller/ScheduledTasks/ScanLeaderOptions.cs b/MediaBrowser.Controller/ScheduledTasks/ScanLeaderOptions.cs index 80064245fa..9818898181 100644 --- a/MediaBrowser.Controller/ScheduledTasks/ScanLeaderOptions.cs +++ b/MediaBrowser.Controller/ScheduledTasks/ScanLeaderOptions.cs @@ -6,9 +6,20 @@ namespace MediaBrowser.Controller.ScheduledTasks; /// public sealed class ScanLeaderOptions { + /// + /// The configuration section these options bind from. + /// + public const string ConfigurationSection = "Jellyfin:ScanLeader"; + + /// + /// The configuration key that overrides the default enablement. + /// + public const string EnabledKey = ConfigurationSection + ":Enabled"; + /// /// Gets or sets a value indicating whether scan-leader election is enabled. When disabled, - /// every instance runs its periodic tasks as before. + /// every instance runs its periodic tasks as before. Left unset, election is enabled whenever a + /// Redis connection string is configured, because that is the only deployment shape that needs it. /// public bool Enabled { get; set; } diff --git a/docs/FORK-DIFF.md b/docs/FORK-DIFF.md index 0483ed27ff..0528338071 100644 --- a/docs/FORK-DIFF.md +++ b/docs/FORK-DIFF.md @@ -60,7 +60,7 @@ return 1 | File | Purpose | |------|---------| | `MediaBrowser.Controller/ScheduledTasks/IScanLeaderLease.cs` | DI contract for the leader lease | -| `MediaBrowser.Controller/ScheduledTasks/ScanLeaderOptions.cs` | `Enabled`, `LeaseDurationSeconds`, `GatedTaskKeys` | +| `MediaBrowser.Controller/ScheduledTasks/ScanLeaderOptions.cs` | `Enabled` (defaults to on when Redis is configured), `LeaseDurationSeconds`, `GatedTaskKeys` | | `MediaBrowser.Controller/ScheduledTasks/NullScanLeaderLease.cs` | Always-leader default, preserving single-instance behaviour | | `Emby.Server.Implementations/ScheduledTasks/RedisScanLeaderLease.cs` | Redis TTL lease; an unreachable Redis is treated as holding the lease | @@ -69,6 +69,17 @@ Gated by default: `RefreshLibrary`, `RefreshPeople`, `RefreshChapterImages`, `CleanupUserDataTask`, `OptimizeDatabaseTask`. Only timer-driven runs are gated; manual and API-triggered runs always execute locally. +Gating is on whenever `Jellyfin:TranscodeStore:RedisConnectionString` is set, because that is only +set for a multi-instance deployment. Set `Jellyfin:ScanLeader:Enabled=false` to opt out. +Startup logs which way it went: + +``` +Scan-leader gating is active: timer-driven library tasks run only on the instance holding the Redis scan-leader lease. +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. + ### PostgreSQL provider `src/Jellyfin.Database/Jellyfin.Database.Providers.PostgreSQL/` is an EF Core diff --git a/tests/Jellyfin.Server.Tests/ScanLeader/ScanLeaderRegistrationTests.cs b/tests/Jellyfin.Server.Tests/ScanLeader/ScanLeaderRegistrationTests.cs new file mode 100644 index 0000000000..58503090fb --- /dev/null +++ b/tests/Jellyfin.Server.Tests/ScanLeader/ScanLeaderRegistrationTests.cs @@ -0,0 +1,144 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using Emby.Server.Implementations.ScheduledTasks; +using Jellyfin.Server.Extensions; +using MediaBrowser.Controller.ScheduledTasks; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; +using Xunit; + +namespace Jellyfin.Server.Tests.ScanLeader; + +/// +/// Without gating every replica runs the timer-driven library tasks, which is what the lease exists +/// to prevent, and nothing in the running server reveals which way the switch went. Both the default +/// and the startup log line are pinned here. +/// +public sealed class ScanLeaderRegistrationTests +{ + private const string RedisKey = "Jellyfin:TranscodeStore:RedisConnectionString"; + + [Fact] + public void AddScanLeaderLease_Should_Gate_By_Default_When_Redis_Is_Configured() + { + var services = new ServiceCollection(); + var logger = new LogRecorder(); + + services.AddScanLeaderLease(Configuration(redis: "valkey-cheeztv-valkey:6379,abortConnect=false"), logger); + + Assert.Equal(typeof(RedisScanLeaderLease), LeaseImplementation(services)); + Assert.True(EffectiveOptions(services).Enabled); + Assert.True(logger.Has(LogLevel.Information, "active")); + } + + [Fact] + public void AddScanLeaderLease_Should_Not_Gate_Without_Redis() + { + var services = new ServiceCollection(); + var logger = new LogRecorder(); + + services.AddScanLeaderLease(Configuration(redis: null), logger); + + Assert.Equal(typeof(NullScanLeaderLease), LeaseImplementation(services)); + Assert.False(EffectiveOptions(services).Enabled); + Assert.True(logger.Has(LogLevel.Information, "off")); + } + + [Fact] + public void AddScanLeaderLease_Should_Honour_Explicit_Opt_Out() + { + var services = new ServiceCollection(); + var logger = new LogRecorder(); + + services.AddScanLeaderLease(Configuration(redis: "valkey:6379", enabled: "false"), logger); + + Assert.Equal(typeof(NullScanLeaderLease), LeaseImplementation(services)); + Assert.False(EffectiveOptions(services).Enabled); + Assert.DoesNotContain(logger.Entries, entry => entry.Level >= LogLevel.Warning); + } + + [Fact] + public void AddScanLeaderLease_Should_Warn_When_Enabled_Without_Redis() + { + var services = new ServiceCollection(); + var logger = new LogRecorder(); + + services.AddScanLeaderLease(Configuration(redis: null, enabled: "true"), logger); + + Assert.Equal(typeof(NullScanLeaderLease), LeaseImplementation(services)); + Assert.False(EffectiveOptions(services).Enabled); + Assert.True(logger.Has(LogLevel.Warning, "cannot run")); + } + + [Fact] + public void AddScanLeaderLease_Should_Keep_Bound_Options() + { + var services = new ServiceCollection(); + var configuration = new ConfigurationBuilder() + .AddInMemoryCollection(new Dictionary + { + [RedisKey] = "valkey:6379", + ["Jellyfin:ScanLeader:LeaseDurationSeconds"] = "90" + }) + .Build(); + + services.AddScanLeaderLease(configuration, new LogRecorder()); + + var options = EffectiveOptions(services); + + Assert.Equal(90, options.LeaseDurationSeconds); + Assert.Contains("RefreshLibrary", options.GatedTaskKeys); + } + + private static IConfiguration Configuration(string? redis, string? enabled = null) + => new ConfigurationBuilder() + .AddInMemoryCollection(new Dictionary + { + [RedisKey] = redis, + [ScanLeaderOptions.EnabledKey] = enabled + }) + .Build(); + + private static Type? LeaseImplementation(IServiceCollection services) + => services.Single(descriptor => descriptor.ServiceType == typeof(IScanLeaderLease)).ImplementationType; + + private static ScanLeaderOptions EffectiveOptions(IServiceCollection services) + { + using var provider = services.BuildServiceProvider(); + return provider.GetRequiredService>().Value; + } + + private sealed class LogRecorder : ILogger + { + private readonly List<(LogLevel Level, string Message)> _entries = new(); + + public IReadOnlyList<(LogLevel Level, string Message)> Entries => _entries; + + public IDisposable BeginScope(TState state) + where TState : notnull + => NoopScope.Instance; + + public bool IsEnabled(LogLevel logLevel) => true; + + public void Log(LogLevel logLevel, EventId eventId, TState state, Exception? exception, Func formatter) + { + ArgumentNullException.ThrowIfNull(formatter); + _entries.Add((logLevel, formatter(state, exception))); + } + + public bool Has(LogLevel level, string substring) + => _entries.Any(entry => entry.Level == level && entry.Message.Contains(substring, StringComparison.Ordinal)); + + private sealed class NoopScope : IDisposable + { + public static readonly NoopScope Instance = new(); + + public void Dispose() + { + } + } + } +} -- 2.47.3