From 97d3e782510d31a5488d3588e819923f5a0e074f Mon Sep 17 00:00:00 2001 From: sebastian-ederer Date: Mon, 4 May 2026 21:19:29 +0200 Subject: [PATCH] refactor: Remove drop_created_before alter_job workaround TimescaleDB 2.26.3 (timescale/timescaledb#9577) fixed the policy_retention_check bug that caused alter_job to fail on drop_created_before retention policies. The workaround in RetentionPolicyOperationGenerator that short-circuited BuildAlterJobClauses for these policies is no longer needed. --- docs/data-annotations/retention-policies.md | 2 +- docs/fluent-api/retention-policies.md | 2 +- .../RetentionPolicyOperationGenerator.cs | 20 -- .../RetentionPolicyOperationGeneratorTests.cs | 45 ++-- .../RetentionPolicyIntegrationTests.cs | 231 +++++++++++++++++- ...etentionPolicyScaffoldingExtractorTests.cs | 68 ++++++ 6 files changed, 326 insertions(+), 42 deletions(-) diff --git a/docs/data-annotations/retention-policies.md b/docs/data-annotations/retention-policies.md index 6576b78..0b22853 100644 --- a/docs/data-annotations/retention-policies.md +++ b/docs/data-annotations/retention-policies.md @@ -54,7 +54,7 @@ public class ApiRequestLog } ``` -> :warning: **Note:** Due to a known bug in TimescaleDB ([#9446](https://github.com/timescale/timescaledb/issues/9446)), `alter_job` fails when used with `DropCreatedBefore` policies. The library works around this by skipping the `alter_job` call for `DropCreatedBefore` policies. As a result, job scheduling parameters (`ScheduleInterval`, `MaxRuntime`, `MaxRetries`, `RetryPeriod`) are accepted by the API but have no effect at the database level when `DropCreatedBefore` is used. +> :warning: **Note:** Customizing job scheduling parameters (`ScheduleInterval`, `MaxRuntime`, `MaxRetries`, `RetryPeriod`) on a `DropCreatedBefore` policy requires **TimescaleDB 2.26.3 or later**. Earlier versions contain a bug in `alter_job` that prevents these settings from being applied for `drop_created_before` policies. Policies using `DropAfter` are not affected and work on all supported TimescaleDB versions. ## Complete Example diff --git a/docs/fluent-api/retention-policies.md b/docs/fluent-api/retention-policies.md index 8aa517e..9282a83 100644 --- a/docs/fluent-api/retention-policies.md +++ b/docs/fluent-api/retention-policies.md @@ -59,7 +59,7 @@ public class ApiRequestLogConfiguration : IEntityTypeConfiguration :warning: **Note:** Due to a known bug in TimescaleDB ([#9446](https://github.com/timescale/timescaledb/issues/9446)), `alter_job` fails when used with `drop_created_before` policies. The library works around this by skipping the `alter_job` call for `drop_created_before` policies. As a result, job scheduling parameters (`scheduleInterval`, `maxRuntime`, `maxRetries`, `retryPeriod`) are accepted by the API but have no effect at the database level when `dropCreatedBefore` is used. +> :warning: **Note:** Customizing job scheduling parameters (`scheduleInterval`, `maxRuntime`, `maxRetries`, `retryPeriod`) on a `dropCreatedBefore` policy requires **TimescaleDB 2.26.3 or later**. Earlier versions contain a bug in `alter_job` that prevents these settings from being applied for `drop_created_before` policies. Policies using `dropAfter` are not affected and work on all supported TimescaleDB versions. ## Complete Example diff --git a/src/Eftdb/Generators/RetentionPolicyOperationGenerator.cs b/src/Eftdb/Generators/RetentionPolicyOperationGenerator.cs index d74da86..ffb54fd 100644 --- a/src/Eftdb/Generators/RetentionPolicyOperationGenerator.cs +++ b/src/Eftdb/Generators/RetentionPolicyOperationGenerator.cs @@ -96,16 +96,6 @@ private static List BuildAlterJobClauses(AddRetentionPolicyOperation ope { List clauses = []; - // alter_job fails for drop_created_before retention policies. - // TimescaleDB's policy_retention_check expects drop_after in the job config JSONB - // but finds drop_created_before instead. Workaround: avoid alter_job for - // drop_created_before policies, or recreate the policy entirely. - // TODO: Remove this when a fix has been applied to TimescaleDB. - if (!string.IsNullOrEmpty(operation.DropCreatedBefore)) - { - return clauses; - } - if (!string.IsNullOrWhiteSpace(operation.ScheduleInterval)) clauses.Add($"schedule_interval => INTERVAL '{operation.ScheduleInterval}'"); @@ -125,16 +115,6 @@ private static List BuildAlterJobClauses(AlterRetentionPolicyOperation o { List clauses = []; - // alter_job fails for drop_created_before retention policies. - // TimescaleDB's policy_retention_check expects drop_after in the job config JSONB - // but finds drop_created_before instead. Workaround: avoid alter_job for - // drop_created_before policies, or recreate the policy entirely. - // TODO: Remove this when a fix has been applied to TimescaleDB. - if (!string.IsNullOrEmpty(operation.DropCreatedBefore)) - { - return clauses; - } - if (!string.IsNullOrWhiteSpace(operation.ScheduleInterval) && operation.ScheduleInterval != operation.OldScheduleInterval) clauses.Add($"schedule_interval => INTERVAL '{operation.ScheduleInterval}'"); diff --git a/tests/Eftdb.Tests/Generators/RetentionPolicyOperationGeneratorTests.cs b/tests/Eftdb.Tests/Generators/RetentionPolicyOperationGeneratorTests.cs index ffe29e8..90815ac 100644 --- a/tests/Eftdb.Tests/Generators/RetentionPolicyOperationGeneratorTests.cs +++ b/tests/Eftdb.Tests/Generators/RetentionPolicyOperationGeneratorTests.cs @@ -47,10 +47,10 @@ public void Generate_Add_DropAfter_with_minimal_config_creates_only_add_policy_s #endregion - #region Generate_Add_DropCreatedBefore_creates_add_policy_without_alter_job + #region Generate_Add_DropCreatedBefore_with_job_settings_creates_add_and_alter_job_sql [Fact] - public void Generate_Add_DropCreatedBefore_creates_add_policy_without_alter_job() + public void Generate_Add_DropCreatedBefore_with_job_settings_creates_add_and_alter_job_sql() { // Arrange AddRetentionPolicyOperation operation = new() @@ -62,10 +62,11 @@ public void Generate_Add_DropCreatedBefore_creates_add_policy_without_alter_job( MaxRetries = 5 }; - // DropCreatedBefore policies must not emit alter_job due to TimescaleDB bug #9446. - // Job settings are intentionally ignored. string expected = @".Sql(@"" SELECT add_retention_policy('public.""""TestTable""""', drop_created_before => INTERVAL '30 days'); + SELECT alter_job(job_id, schedule_interval => INTERVAL '1 day', max_retries => 5) + FROM timescaledb_information.jobs + WHERE proc_name = 'policy_retention' AND hypertable_schema = 'public' AND hypertable_name = 'TestTable'; "")"; // Act @@ -138,10 +139,10 @@ FROM timescaledb_information.jobs #endregion - #region Generate_Add_DropCreatedBefore_with_job_settings_still_omits_alter_job + #region Generate_Add_DropCreatedBefore_with_all_job_settings_creates_add_and_alter_job_sql [Fact] - public void Generate_Add_DropCreatedBefore_with_job_settings_still_omits_alter_job() + public void Generate_Add_DropCreatedBefore_with_all_job_settings_creates_add_and_alter_job_sql() { // Arrange AddRetentionPolicyOperation operation = new() @@ -155,10 +156,11 @@ public void Generate_Add_DropCreatedBefore_with_job_settings_still_omits_alter_j RetryPeriod = "10 minutes" }; - // Even with all job settings specified, alter_job is omitted for DropCreatedBefore - // due to TimescaleDB bug #9446 workaround. string expected = @".Sql(@"" SELECT add_retention_policy('public.""""TestTable""""', drop_created_before => INTERVAL '30 days'); + SELECT alter_job(job_id, schedule_interval => INTERVAL '2 days', max_runtime => INTERVAL '1 hour', max_retries => 5, retry_period => INTERVAL '10 minutes') + FROM timescaledb_information.jobs + WHERE proc_name = 'policy_retention' AND hypertable_schema = 'public' AND hypertable_name = 'TestTable'; "")"; // Act @@ -240,10 +242,10 @@ FROM timescaledb_information.jobs #endregion - #region Generate_Alter_changed_to_DropCreatedBefore_creates_remove_and_add_without_alter_job + #region Generate_Alter_changed_to_DropCreatedBefore_creates_remove_add_and_alter_job_sql [Fact] - public void Generate_Alter_changed_to_DropCreatedBefore_creates_remove_and_add_without_alter_job() + public void Generate_Alter_changed_to_DropCreatedBefore_creates_remove_add_and_alter_job_sql() { // Arrange AlterRetentionPolicyOperation operation = new() @@ -258,11 +260,13 @@ public void Generate_Alter_changed_to_DropCreatedBefore_creates_remove_and_add_w OldScheduleInterval = "1 day" }; - // During Alter recreation, alter_job is still emitted to reapply existing job settings. - // The DropCreatedBefore workaround only applies to the Add path. + // During recreation, alter_job is emitted to reapply the final-state job settings string expected = @".Sql(@"" SELECT remove_retention_policy('public.""""TestTable""""', if_exists => true); SELECT add_retention_policy('public.""""TestTable""""', drop_created_before => INTERVAL '30 days'); + SELECT alter_job(job_id, schedule_interval => INTERVAL '1 day') + FROM timescaledb_information.jobs + WHERE proc_name = 'policy_retention' AND hypertable_schema = 'public' AND hypertable_name = 'TestTable'; "")"; // Act @@ -400,10 +404,10 @@ FROM timescaledb_information.jobs #endregion - #region Generate_Alter_DropCreatedBefore_job_settings_change_skips_alter_job + #region Generate_Alter_DropCreatedBefore_only_job_settings_change_emits_alter_job [Fact] - public void Generate_Alter_DropCreatedBefore_job_settings_change_skips_alter_job() + public void Generate_Alter_DropCreatedBefore_only_job_settings_change_emits_alter_job() { // Arrange AlterRetentionPolicyOperation operation = new() @@ -420,12 +424,17 @@ public void Generate_Alter_DropCreatedBefore_job_settings_change_skips_alter_job OldScheduleInterval = "1 day" }; - // No recreation needed, but alter_job is skipped for DropCreatedBefore due to TimescaleDB bug #9446. - RetentionPolicyOperationGenerator generator = new(true); - List result = generator.Generate(operation); + string expected = @".Sql(@"" + SELECT alter_job(job_id, schedule_interval => INTERVAL '2 days') + FROM timescaledb_information.jobs + WHERE proc_name = 'policy_retention' AND hypertable_schema = 'public' AND hypertable_name = 'TestTable'; + "")"; + + // Act + string result = GetGeneratedCode(operation); // Assert - Assert.Empty(result); + Assert.Equal(SqlHelper.NormalizeSql(expected), SqlHelper.NormalizeSql(result)); } #endregion diff --git a/tests/Eftdb.Tests/Integration/RetentionPolicyIntegrationTests.cs b/tests/Eftdb.Tests/Integration/RetentionPolicyIntegrationTests.cs index 6ff6677..ec9a468 100644 --- a/tests/Eftdb.Tests/Integration/RetentionPolicyIntegrationTests.cs +++ b/tests/Eftdb.Tests/Integration/RetentionPolicyIntegrationTests.cs @@ -599,8 +599,6 @@ protected override void OnModelCreating(ModelBuilder modelBuilder) } } - //[Fact(Skip = "TimescaleDB bug #9446: alter_job fails when config contains drop_created_before instead of drop_after. " + - // "The generator's Alter recreation path emits alter_job to reapply job settings, which hits this bug.")] [Fact] public async Task Should_Alter_RetentionPolicy_DropAfter_To_DropCreatedBefore() { @@ -775,4 +773,233 @@ public async Task Should_Create_RetentionPolicy_ViaEnsureCreated() } #endregion + + #region Should_Alter_RetentionPolicy_DropAfter_To_DropCreatedBefore_PreservesJobSettings + + private class PreserveJobSettingsMetric + { + public int Id { get; set; } + public DateTime Time { get; set; } + public double Value { get; set; } + } + + private class InitialPreserveJobSettingsContext(string connectionString) : DbContext + { + public DbSet Metrics { get; set; } = null!; + + protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder) + => optionsBuilder.UseNpgsql(connectionString).UseTimescaleDb(); + + protected override void OnModelCreating(ModelBuilder modelBuilder) + { + modelBuilder.Entity(entity => + { + entity.ToTable("retention_preserve_job_settings"); + entity.HasKey(e => new { e.Time, e.Id }); + entity.IsHypertable(e => e.Time); + entity.WithRetentionPolicy( + dropAfter: "7 days", + scheduleInterval: "6 hours", + maxRetries: 5 + ); + }); + } + } + + private class ModifiedPreserveJobSettingsContext(string connectionString) : DbContext + { + public DbSet Metrics { get; set; } = null!; + + protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder) + => optionsBuilder.UseNpgsql(connectionString).UseTimescaleDb(); + + protected override void OnModelCreating(ModelBuilder modelBuilder) + { + modelBuilder.Entity(entity => + { + entity.ToTable("retention_preserve_job_settings"); + entity.HasKey(e => new { e.Time, e.Id }); + entity.IsHypertable(e => e.Time); + entity.WithRetentionPolicy( + dropCreatedBefore: "30 days", // <-- Changed from dropAfter: "7 days" + scheduleInterval: "6 hours", + maxRetries: 5 + ); + }); + } + } + + [Fact] + public async Task Should_Alter_RetentionPolicy_DropAfter_To_DropCreatedBefore_PreservesJobSettings() + { + await using InitialPreserveJobSettingsContext initialContext = new(_connectionString!); + await CreateDatabaseViaMigrationAsync(initialContext); + + await using ModifiedPreserveJobSettingsContext modifiedContext = new(_connectionString!); + await AlterDatabaseViaMigrationAsync(initialContext, modifiedContext); + + // The recreation path must reapply non-default job settings on the new policy + // (this is exactly the flow the pre-2.26.3 alter_job bug used to break). + int jobId = await GetRetentionPolicyJobIdAsync(modifiedContext, "retention_preserve_job_settings"); + Assert.True(jobId > 0); + + TimeSpan scheduleInterval = await GetScheduleIntervalAsync(modifiedContext, jobId); + Assert.Equal(TimeSpan.FromHours(6), scheduleInterval); + + int maxRetries = await GetJobMaxRetriesAsync(modifiedContext, jobId); + Assert.Equal(5, maxRetries); + + string? config = await GetRetentionPolicyConfigAsync(modifiedContext, "retention_preserve_job_settings"); + Assert.NotNull(config); + Assert.Contains("drop_created_before", config); + Assert.DoesNotContain("drop_after", config); + } + + #endregion + + #region Should_Alter_DropCreatedBefore_RetentionPolicy_ScheduleInterval + + private class AlterDcbScheduleMetric + { + public int Id { get; set; } + public DateTime Time { get; set; } + public double Value { get; set; } + } + + private class InitialAlterDcbScheduleContext(string connectionString) : DbContext + { + public DbSet Metrics { get; set; } = null!; + + protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder) + => optionsBuilder.UseNpgsql(connectionString).UseTimescaleDb(); + + protected override void OnModelCreating(ModelBuilder modelBuilder) + { + modelBuilder.Entity(entity => + { + entity.ToTable("retention_alter_dcb_schedule"); + entity.HasKey(e => new { e.Time, e.Id }); + entity.IsHypertable(e => e.Time); + entity.WithRetentionPolicy( + dropCreatedBefore: "30 days", + scheduleInterval: "1 day" + ); + }); + } + } + + private class ModifiedAlterDcbScheduleContext(string connectionString) : DbContext + { + public DbSet Metrics { get; set; } = null!; + + protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder) + => optionsBuilder.UseNpgsql(connectionString).UseTimescaleDb(); + + protected override void OnModelCreating(ModelBuilder modelBuilder) + { + modelBuilder.Entity(entity => + { + entity.ToTable("retention_alter_dcb_schedule"); + entity.HasKey(e => new { e.Time, e.Id }); + entity.IsHypertable(e => e.Time); + entity.WithRetentionPolicy( + dropCreatedBefore: "30 days", + scheduleInterval: "12 hours" // <-- Changed from "1 day" + ); + }); + } + } + + [Fact] + public async Task Should_Alter_DropCreatedBefore_RetentionPolicy_ScheduleInterval() + { + await using InitialAlterDcbScheduleContext initialContext = new(_connectionString!); + await CreateDatabaseViaMigrationAsync(initialContext); + + int jobId = await GetRetentionPolicyJobIdAsync(initialContext, "retention_alter_dcb_schedule"); + TimeSpan initialSchedule = await GetScheduleIntervalAsync(initialContext, jobId); + Assert.Equal(TimeSpan.FromDays(1), initialSchedule); + + await using ModifiedAlterDcbScheduleContext modifiedContext = new(_connectionString!); + await AlterDatabaseViaMigrationAsync(initialContext, modifiedContext); + + // Pre-2.26.3 this alter_job call would fail because the policy config carries + // drop_created_before instead of drop_after. + TimeSpan newSchedule = await GetScheduleIntervalAsync(modifiedContext, jobId); + Assert.Equal(TimeSpan.FromHours(12), newSchedule); + } + + #endregion + + #region Should_Alter_RetentionPolicy_InitialStart + + private class AlterInitialStartMetric + { + public int Id { get; set; } + public DateTime Time { get; set; } + public double Value { get; set; } + } + + private class InitialAlterInitialStartContext(string connectionString) : DbContext + { + public DbSet Metrics { get; set; } = null!; + + protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder) + => optionsBuilder.UseNpgsql(connectionString).UseTimescaleDb(); + + protected override void OnModelCreating(ModelBuilder modelBuilder) + { + modelBuilder.Entity(entity => + { + entity.ToTable("retention_alter_initial_start"); + entity.HasKey(e => new { e.Time, e.Id }); + entity.IsHypertable(e => e.Time); + entity.WithRetentionPolicy( + dropAfter: "7 days", + initialStart: new DateTime(2025, 1, 1, 0, 0, 0, DateTimeKind.Utc) + ); + }); + } + } + + private class ModifiedAlterInitialStartContext(string connectionString) : DbContext + { + public DbSet Metrics { get; set; } = null!; + + protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder) + => optionsBuilder.UseNpgsql(connectionString).UseTimescaleDb(); + + protected override void OnModelCreating(ModelBuilder modelBuilder) + { + modelBuilder.Entity(entity => + { + entity.ToTable("retention_alter_initial_start"); + entity.HasKey(e => new { e.Time, e.Id }); + entity.IsHypertable(e => e.Time); + entity.WithRetentionPolicy( + dropAfter: "7 days", + initialStart: new DateTime(2026, 6, 15, 12, 0, 0, DateTimeKind.Utc) // <-- Changed from 2025-01-01 + ); + }); + } + } + + [Fact] + public async Task Should_Alter_RetentionPolicy_InitialStart() + { + await using InitialAlterInitialStartContext initialContext = new(_connectionString!); + await CreateDatabaseViaMigrationAsync(initialContext); + + await using ModifiedAlterInitialStartContext modifiedContext = new(_connectionString!); + await AlterDatabaseViaMigrationAsync(initialContext, modifiedContext); + + int jobId = await GetRetentionPolicyJobIdAsync(modifiedContext, "retention_alter_initial_start"); + Assert.True(jobId > 0); + + DateTime? initialStart = await GetJobInitialStartAsync(modifiedContext, jobId); + Assert.NotNull(initialStart); + Assert.Equal(new DateTime(2026, 6, 15, 12, 0, 0, DateTimeKind.Utc), initialStart.Value.ToUniversalTime()); + } + + #endregion } diff --git a/tests/Eftdb.Tests/Integration/RetentionPolicyScaffoldingExtractorTests.cs b/tests/Eftdb.Tests/Integration/RetentionPolicyScaffoldingExtractorTests.cs index adbd820..f6e6b29 100644 --- a/tests/Eftdb.Tests/Integration/RetentionPolicyScaffoldingExtractorTests.cs +++ b/tests/Eftdb.Tests/Integration/RetentionPolicyScaffoldingExtractorTests.cs @@ -2,6 +2,7 @@ using CmdScale.EntityFrameworkCore.TimescaleDB.Configuration.RetentionPolicy; using CmdScale.EntityFrameworkCore.TimescaleDB.Design.Scaffolding; using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Scaffolding.Metadata; using Npgsql; using Testcontainers.PostgreSql; @@ -484,4 +485,71 @@ public async Task Should_Extract_RetentionPolicy_With_Custom_Schema() } #endregion + + #region Should_RoundTrip_RetentionPolicy_With_All_Job_Settings_And_InitialStart + + private class RoundTripMetric + { + public DateTime Timestamp { get; set; } + public double Value { get; set; } + } + + private class RoundTripContext(string connectionString) : DbContext + { + public DbSet Metrics => Set(); + + protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder) + => optionsBuilder.UseNpgsql(connectionString).UseTimescaleDb(); + + protected override void OnModelCreating(ModelBuilder modelBuilder) + { + modelBuilder.Entity(entity => + { + entity.HasNoKey(); + entity.ToTable("scaff_retention_roundtrip"); + entity.IsHypertable(x => x.Timestamp) + .WithRetentionPolicy( + dropAfter: "14 days", + scheduleInterval: "12 hours", + maxRuntime: "01:00:00", + maxRetries: 3, + retryPeriod: "00:15:00", + initialStart: new DateTime(2026, 6, 1, 0, 0, 0, DateTimeKind.Utc)); + }); + } + } + + [Fact] + public async Task Should_RoundTrip_RetentionPolicy_With_All_Job_Settings_And_InitialStart() + { + await using RoundTripContext context = new(_connectionString!); + await CreateDatabaseViaMigrationAsync(context); + + RetentionPolicyScaffoldingExtractor extractor = new(); + await using NpgsqlConnection connection = new(_connectionString); + Dictionary<(string Schema, string TableName), object> result = extractor.Extract(connection); + + Assert.True(result.ContainsKey(("public", "scaff_retention_roundtrip"))); + RetentionPolicyScaffoldingExtractor.RetentionPolicyInfo info = + (RetentionPolicyScaffoldingExtractor.RetentionPolicyInfo)result[("public", "scaff_retention_roundtrip")]; + + DatabaseTable table = new() { Name = "scaff_retention_roundtrip", Schema = "public" }; + RetentionPolicyAnnotationApplier applier = new(); + applier.ApplyAnnotations(table, info); + + Assert.Equal(true, table[RetentionPolicyAnnotations.HasRetentionPolicy]); + Assert.Equal("14 days", table[RetentionPolicyAnnotations.DropAfter]); + Assert.Null(table[RetentionPolicyAnnotations.DropCreatedBefore]); + // PostgreSQL normalizes sub-day intervals to HH:MM:SS form on the round-trip. + Assert.Equal("12:00:00", table[RetentionPolicyAnnotations.ScheduleInterval]); + Assert.Equal("01:00:00", table[RetentionPolicyAnnotations.MaxRuntime]); + Assert.Equal(3, table[RetentionPolicyAnnotations.MaxRetries]); + Assert.Equal("00:15:00", table[RetentionPolicyAnnotations.RetryPeriod]); + + DateTime expectedInitialStart = new(2026, 6, 1, 0, 0, 0, DateTimeKind.Utc); + DateTime actualInitialStart = Assert.IsType(table[RetentionPolicyAnnotations.InitialStart]); + Assert.Equal(expectedInitialStart, actualInitialStart.ToUniversalTime()); + } + + #endregion }