Add a v16 managed-conf marker for a 15-minute checkpoint interval (#4246)

Darling/Darling.Tests/DarlingManagedPostgresTests.cs

@@ -427,8 +427,9 @@ public void EveryConfMarker_IsDistinct_AndNoneIsASubstringOfAnother()
            assertion below vacuously true, and a reflection filter that stopped matching is exactly the
            silent failure this shape invites. Eleven blocks as of #3175; twelve as of #3802 (v12 WAL sizing);
            thirteen as of #3899 (v13 statement statistics); fourteen as of #3909 (v14 PostgreSQL 17
-           maintenance_work_mem limit); fifteen as of #4246 (v15 WAL compression). */
-        Assert.Equal(15, markers.Length);
+           maintenance_work_mem limit); fifteen as of #4246 (v15 WAL compression); sixteen as of #4246 (v16
+           checkpoint interval). */
+        Assert.Equal(16, markers.Length);
 
         Assert.Equal(markers.Length, markers.Select(m => m.Value).Distinct(StringComparer.Ordinal).Count());
 
@@ -2267,6 +2268,75 @@ public void EnsureConfAppended_AppendsV15Once_AndNotAgainOnANextStart()
         }
     }
 
+    /* ===================== v16 checkpoint interval (#4246) ===================== */
+
+    /// <summary>
+    /// The v16 block (#4246): <c>checkpoint_timeout = 15min</c> only. See
+    /// <see cref="DarlingManagedPostgres.ConfMarkerV16"/> for the trial and the sync-bar risk, and why the
+    /// block deliberately says nothing about <c>max_wal_size</c>.
+    /// </summary>
+    [Fact]
+    public void CheckpointIntervalConfAppend_PinsV16Marker_AndSetsCheckpointTimeout()
+    {
+        var block = DarlingManagedPostgres.BuildCheckpointIntervalConfAppend();
+
+        Assert.Contains(DarlingManagedPostgres.ConfMarkerV16, block, StringComparison.Ordinal);
+        Assert.Equal("15min", LastSettingValue(block, "checkpoint_timeout"));
+
+        /* No fingerprint or stamp line, or the v8/v12 staleness checks would misread what they scan. */
+        Assert.DoesNotContain(DarlingManagedPostgres.ConfHardwareFingerprintPrefix, block, StringComparison.Ordinal);
+        Assert.DoesNotContain(DarlingManagedPostgres.ConfWalSizingStampPrefix, block, StringComparison.Ordinal);
+
+        /* v12 (#3802) is still the only thing that ever sets these: this block leaves the disk-derived
+           ceiling exactly where it is. */
+        Assert.Null(LastSettingValue(block, "max_wal_size"));
+        Assert.Null(LastSettingValue(block, "min_wal_size"));
+        Assert.Null(LastSettingValue(block, "checkpoint_completion_target"));
+    }
+
+    /// <summary>The generic per-setting source scan (#4214) walks every marker in this list; a block absent
+    /// from it would be invisible to that scan even though it is live.</summary>
+    [Fact]
+    public void ConfMarkerV16_IsInAllManagedConfMarkers()
+        => Assert.Contains(DarlingManagedPostgres.ConfMarkerV16, DarlingManagedPostgres.AllManagedConfMarkers);
+
+    /// <summary>
+    /// The heal on real files (#4246): a cluster whose conf carries every earlier marker but not v16 gains
+    /// exactly one v16 block, with <c>checkpoint_timeout</c> live in the file, and a second start appends
+    /// nothing more -- the same once-only shape v9-v11, v13 and v15 prove elsewhere (<see
+    /// cref="EnsureConfAppended_AppendsV15Once_AndNotAgainOnANextStart"/>), exercised here through the real
+    /// <see cref="DarlingManagedPostgres.EnsureConfAppended"/> rather than string concatenation.
+    /// </summary>
+    [Fact]
+    public void EnsureConfAppended_AppendsV16Once_AndNotAgainOnANextStart()
+    {
+        var root = Directory.CreateTempSubdirectory("darling-v16-");
+        try
+        {
+            var dataDirectory = Path.Combine(root.FullName, "pg");
+            Directory.CreateDirectory(dataDirectory);
+            var confPath = Path.Combine(dataDirectory, "postgresql.conf");
+            File.WriteAllText(confPath, DarlingManagedPostgres.BuildConfAppend(5994));
+
+            var pg = new DarlingManagedPostgres(
+                new PostgresConfig { Managed = true, Port = 5994, DataDirectory = dataDirectory }, NullLogger.Instance);
+
+            pg.EnsureConfAppended(dataDirectory);
+            var first = File.ReadAllText(confPath);
+            Assert.Equal(1, CountOccurrences(first, DarlingManagedPostgres.ConfMarkerV16));
+            Assert.Equal("15min", LastSettingValue(first, "checkpoint_timeout"));
+
+            pg.EnsureConfAppended(dataDirectory);
+            var second = File.ReadAllText(confPath);
+            Assert.Equal(1, CountOccurrences(second, DarlingManagedPostgres.ConfMarkerV16));
+            Assert.Equal("15min", LastSettingValue(second, "checkpoint_timeout"));
+        }
+        finally
+        {
+            root.Delete(recursive: true);
+        }
+    }
+
     /* ===================== v12 wal sizing (#3802) ===================== */
 
     private const long OneGb = 1024L * 1024 * 1024;

Darling/Darling.Tests/ManagedConfFileTests.cs

@@ -323,6 +323,160 @@ public void RenderWithValues_ExtraKey_AppendedAfterOwnedKeys_ResultIsNotHandEdit
         Assert.False(ManagedConfFile.IsHandEdited(rendered));
     }
 
+    /// <summary>#4246: v16's checkpoint interval is one of the builders <see cref="ManagedConfFile.RenderBody"/>
+    /// calls, so the managed body itself carries <c>checkpoint_timeout</c> — not just the legacy v1-v16
+    /// appenders <c>EnsureConfAppended</c> runs against a self-hosted conf.</summary>
+    [Fact]
+    public void RenderBody_ContainsCheckpointTimeout()
+    {
+        var body = ManagedConfFile.RenderBody(SampleInputs());
+
+        Assert.Contains("checkpoint_timeout = '15min'", body, StringComparison.Ordinal);
+    }
+
+    /// <summary>
+    /// #4246 (claude-desktop's parity pin, 14:47Z): for EVERY marker in
+    /// <see cref="DarlingManagedPostgres.AllManagedConfMarkers"/>, every key that marker's OWN legacy builder
+    /// emits must show up in <see cref="ManagedConfFile.RenderBody"/>'s output too. The expected keys are DERIVED
+    /// by parsing each builder's actual output (<see cref="DarlingManagedPostgres.ParseConfText"/>) rather than
+    /// hand-typed here, so a future vN whose marker is added to <c>AllManagedConfMarkers</c> and whose builder is
+    /// written, but whose builder call is never added to <c>RenderBody</c> (exactly this PR's own bug, for v16),
+    /// fails this test on its own keys — with no hand roster to forget to update alongside it.
+    ///
+    /// <para>v14 (the legacy <c>maintenance_work_mem</c> cap) is the one documented skip. <c>RenderBody</c>
+    /// decides v14 purely from its own inputs: it reduces the blocks above v14,
+    /// reads whatever <c>maintenance_work_mem</c> is in force from that reduction, and calls the SAME
+    /// <see cref="DarlingManagedPostgres.NeedsLegacyMaintenanceWorkMemCap"/> predicate
+    /// <c>EnsureConfAppended</c>'s own heal uses. The skip is arithmetic, not a missing input: every
+    /// <c>maintenance_work_mem</c> value this fixture's memory-sizing builders can ever produce is
+    /// <see cref="DarlingManagedPostgres.DeriveMemorySettings"/>'s own value, which is capped at
+    /// <see cref="DarlingManagedPostgres.MaintenanceWorkMemCapMb"/> (2047 MB = 2,096,128 kB) for ANY RAM size —
+    /// the cap is a MIN in the formula, so raising RAM without bound never raises the derived value past it. That
+    /// cap sits 1,023 kB UNDER <see cref="DarlingManagedPostgres.LegacyMaintenanceWorkMemMaxKb"/> (2,097,151 kB) —
+    /// deliberately, so a fresh derivation never crosses the very limit v14 exists to cap. So no RAM figure this
+    /// fixture could supply (there is no upper bound to check: the derivation's cap makes every RAM equally
+    /// incapable) ever drives <c>RenderBody</c> to append v14 on its own. v14 only fires for a value ALREADY on
+    /// disk from before this cap existed (a pre-#3909 store) or set directly by <c>ALTER SYSTEM</c> — inputs
+    /// <c>RenderBody</c>'s formula-only inputs cannot produce. It is exercised directly by
+    /// <c>NeedsLegacyMaintenanceWorkMemCap_OnlyForAnOverLimitValueOnPostgres17OrEarlier</c> in
+    /// <c>DarlingManagedPostgresTests</c>, not here.</para>
+    /// </summary>
+    [Fact]
+    public void RenderBody_CarriesEveryManagedConfMarkersOwnedKeys()
+    {
+        var inputs = SampleInputs();
+        var body = ManagedConfFile.RenderBody(inputs);
+        var (_, bodyValues) = ExtractValues(body);
+
+        foreach (var marker in DarlingManagedPostgres.AllManagedConfMarkers)
+        {
+            var representativeBlock = BuildRepresentativeBlockOrNull(marker, inputs);
+            if (representativeBlock is null)
+            {
+                continue; // v14: documented skip above, exercised elsewhere.
+            }
+
+            foreach (var (_, key, _) in DarlingManagedPostgres.ParseConfText(representativeBlock))
+            {
+                Assert.True(
+                    bodyValues.ContainsKey(key),
+                    $"marker '{marker}' owns key '{key}' (from its own builder's output) but RenderBody's rendered body does not carry it.");
+            }
+        }
+    }
+
+    /// <summary>Maps one <see cref="DarlingManagedPostgres.AllManagedConfMarkers"/> entry to its OWN legacy
+    /// builder's output, called with inputs this fixture's <paramref name="inputs"/> can drive (RAM/disk
+    /// authoritative, PostgreSQL 18) — the keys asserted against <c>RenderBody</c> are parsed back out of this
+    /// text, never hand-typed. Throws for a marker with no entry here, so a marker added to
+    /// <c>AllManagedConfMarkers</c> without a corresponding line here fails loudly instead of being silently
+    /// skipped.</summary>
+    private static string? BuildRepresentativeBlockOrNull(string marker, ManagedConfFile.RenderInputs inputs)
+    {
+        if (marker == DarlingManagedPostgres.ConfMarker)
+        {
+            return DarlingManagedPostgres.BuildConfAppend(inputs.Port);
+        }
+
+        if (marker == DarlingManagedPostgres.ConfMarkerV2)
+        {
+            return DarlingManagedPostgres.BuildWorkerSizingConfAppend();
+        }
+
+        if (marker == DarlingManagedPostgres.ConfMarkerV3)
+        {
+            return DarlingManagedPostgres.BuildMemorySizingConfAppend(inputs.RamBytes);
+        }
+
+        if (marker == DarlingManagedPostgres.ConfMarkerV4)
+        {
+            return DarlingManagedPostgres.BuildWriteThroughputConfAppend();
+        }
+
+        if (marker == DarlingManagedPostgres.ConfMarkerV5)
+        {
+            return DarlingManagedPostgres.BuildColocatedSizingConfAppend(inputs.RamBytes);
+        }
+
+        if (marker == DarlingManagedPostgres.ConfMarkerV6)
+        {
+            return DarlingManagedPostgres.BuildLogRotationConfAppend();
+        }
+
+        if (marker == DarlingManagedPostgres.ConfMarkerV7)
+        {
+            return DarlingManagedPostgres.BuildCompressionMemoryConfAppend(inputs.RamBytes);
+        }
+
+        if (marker == DarlingManagedPostgres.ConfMarkerV8)
+        {
+            return DarlingManagedPostgres.BuildHardwareSizingConfAppend(inputs.RamBytes, inputs.HypertableCount);
+        }
+
+        if (marker == DarlingManagedPostgres.ConfMarkerV9)
+        {
+            return DarlingManagedPostgres.BuildTimeZoneConfAppend();
+        }
+
+        if (marker == DarlingManagedPostgres.ConfMarkerV10)
+        {
+            return DarlingManagedPostgres.BuildMessageLocaleConfAppend();
+        }
+
+        if (marker == DarlingManagedPostgres.ConfMarkerV11)
+        {
+            return DarlingManagedPostgres.BuildJobExecutionLoggingConfAppend();
+        }
+
+        if (marker == DarlingManagedPostgres.ConfMarkerV12)
+        {
+            return DarlingManagedPostgres.BuildWalSizingConfAppend(inputs.DataVolumeFreeBytes, inputs.DataVolumeTotalBytes, inputs.PostgresMajor);
+        }
+
+        if (marker == DarlingManagedPostgres.ConfMarkerV13)
+        {
+            return DarlingManagedPostgres.BuildStatementStatisticsConfAppend(inputs.EffectivePreloadList);
+        }
+
+        if (marker == DarlingManagedPostgres.ConfMarkerV14)
+        {
+            return null; // documented skip -- no RAM ever drives RenderBody's own inputs past the cap; see the class doc comment on RenderBody_CarriesEveryManagedConfMarkersOwnedKeys
+        }
+
+        if (marker == DarlingManagedPostgres.ConfMarkerV15)
+        {
+            return DarlingManagedPostgres.BuildWalVolumeConfAppend();
+        }
+
+        if (marker == DarlingManagedPostgres.ConfMarkerV16)
+        {
+            return DarlingManagedPostgres.BuildCheckpointIntervalConfAppend();
+        }
+
+        throw new InvalidOperationException(
+            $"marker '{marker}' is in AllManagedConfMarkers but has no representative-block mapping in this parity test -- add one.");
+    }
+
     private static (List<string> Order, Dictionary<string, string> Values) ExtractValues(string body)
     {
         var order = new List<string>();

Darling/Darling.Tests/ManagedConfMigrationTests.cs

@@ -46,6 +46,17 @@ public void ClassifyLines_UntouchedV15Block_IsOurs()
         Assert.Equal(ConfLineClassification.Ours, wal.Classification);
     }
 
+    [Fact]
+    public void ClassifyLines_UntouchedV16Block_IsOurs()
+    {
+        var conf = DarlingManagedPostgres.BuildCheckpointIntervalConfAppend();
+        var lines = ClassifyLines(conf);
+
+        var checkpoint = FindByText(lines, "checkpoint_timeout");
+        Assert.Equal(ConfLineClassification.Ours, checkpoint.Classification);
+        Assert.Equal(DarlingManagedPostgres.ConfMarkerV16, checkpoint.BlockMarker);
+    }
+
     [Fact]
     public void ClassifyLines_UntouchedV13PreloadLine_IsOurs()
     {

Darling/Darling.Tests/ManagedConfRehearsalTests.cs

@@ -27,17 +27,19 @@ namespace Darling.Tests;
 /// stay exactly where it is); and <c>log_timezone = 'UTC'</c> at the end (a key no managed block owns, so it
 /// stays exactly where it is too).
 ///
-/// <para><b>This fixture's numbers</b> are 69 lines removed / 24 managed keys / 1 operator line moved.
+/// <para><b>This fixture's numbers</b> are 69 lines removed / 25 managed keys / 1 operator line moved.
 /// The "1 operator line moved" is exactly what the rewrite logic requires. The counts below explain why they
-/// are 69/24 rather than some other plausible pair, given how today's builders emit their blocks:
+/// are 69/25 rather than some other plausible pair, given how today's builders emit their blocks:
 /// <list type="bullet">
-/// <item><b>managed keys 24 vs 23.</b> <see cref="ManagedConfFile.RenderBody"/> emits one line per key
+/// <item><b>managed keys 25 vs 23.</b> <see cref="ManagedConfFile.RenderBody"/> emits one line per key
 /// <see cref="DarlingManagedPostgres.ParseConfText"/> reads back from calling every <c>Build*ConfAppend</c> in
-/// order with THIS PR's inputs (RAM authoritative, disk authoritative, PostgreSQL 18) — today that is 24 keys.
-/// The extra key is <c>min_wal_size</c>: v12's WAL-sizing block
+/// order with THIS PR's inputs (RAM authoritative, disk authoritative, PostgreSQL 18) — today that is 25 keys.
+/// One extra key is <c>min_wal_size</c>: v12's WAL-sizing block
 /// (<see cref="DarlingManagedPostgres.BuildWalSizingConfAppend"/>) writes both <c>max_wal_size</c> AND
 /// <c>min_wal_size</c>, and nothing later in the v1-v14 order overwrites the second one, so it survives the
-/// reduction as its own key.</item>
+/// reduction as its own key. The other extra key is <c>checkpoint_timeout</c>: v16 (#4246)
+/// (<see cref="DarlingManagedPostgres.BuildCheckpointIntervalConfAppend"/>) is now appended in
+/// <see cref="ManagedConfFile.RenderBody"/> too, and nothing else in this fixture's inputs sets that key.</item>
 /// <item><b>lines removed 69.</b> The initdb stock conf, v1-v14
 /// each appended once except v8 four times, the three hand edits — this fixture reproduces, so the shapes
 /// match. The absolute REMOVAL count is a function of how many total OURS lines the classifier drops (three
@@ -122,7 +124,7 @@ public void Rewrite_RehearsalFixture_LinesRemovedManagedKeysAndOperatorLinesMove
         var afterLineCount = result.NewConfText.Split('\n').Length;
 
         Assert.Equal(69, beforeLineCount - afterLineCount);
-        Assert.Equal(24, managedValues.Count);
+        Assert.Equal(25, managedValues.Count);
         Assert.Single(result.ExcludedKeys);
         Assert.Contains("max_connections", result.ExcludedKeys);
     }