diff --git a/Darling/Darling.Tests/DarlingManagedPostgresTests.cs b/Darling/Darling.Tests/DarlingManagedPostgresTests.cs
index 27c063d1b..f63d3c750 100644
--- a/Darling/Darling.Tests/DarlingManagedPostgresTests.cs
+++ b/Darling/Darling.Tests/DarlingManagedPostgresTests.cs
@@ -1635,15 +1635,19 @@ public void HardwareSizingConfAppend_NeverEmitsSharedBuffers(long ramGb)
}
///
- /// CONSTRAINT PIN 2 (#2845): the hardware block must never emit work_mem, at ANY host size.
- /// The formula would take it 31 -> 63 MB on the resized boxes, and the only measurements above 31 MB on
- /// this store's heaviest read are worse (PlanRegressionSql: default 26,565 ms, 31 MB 25,617 ms,
- /// 512 MB 59,323 ms). It is also the wrong KIND of setting for this block — a per-sort, per-connection
- /// ceiling that follows from the query mix, not from the machine.
+ /// PIN (#4207): the hardware block emits ALL FOUR
+ /// values, work_mem included, from the SAME
+ /// call the block already uses for the other three. Before #4207 this was the opposite pin
+ /// (HardwareSizingConfAppend_NeverEmitsWorkMem): #2845 measured a regression at 512 MB — 8x this
+ /// formula's own 64 MB ceiling — and concluded to exclude work_mem altogether, so the actual clamped
+ /// value the formula derives was never measured. #4207 measured what excluding it cost instead: three
+ /// field stores stuck at the v3 block's 31 MB after a resize to 31.5 GB, one of them having spilled 33 TB
+ /// to temp files since creation. See
+ /// for the full reversal.
///
- /// Note the theory covers 32 GB and above, where the formula clamps to the 64 MB ceiling: those
- /// are precisely the sizes where a naive "apply the formula to the new RAM" change would have doubled
- /// it.
+ /// Covers 32 GB and above, where the formula clamps to the 64 MB ceiling, on purpose: those are the
+ /// sizes where a resize can no longer move the value further, so a regression there would be the
+ /// permanent state of every sufficiently large store, not a transient one.
///
[Theory]
[InlineData(4)]
@@ -1651,22 +1655,48 @@ public void HardwareSizingConfAppend_NeverEmitsSharedBuffers(long ramGb)
[InlineData(32)]
[InlineData(64)]
[InlineData(512)]
- public void HardwareSizingConfAppend_NeverEmitsWorkMem(long ramGb)
+ public void HardwareSizingConfAppend_EmitsAllFourMemorySettings(long ramGb)
{
- var block = DarlingManagedPostgres.BuildHardwareSizingConfAppend(ramGb * 1024 * 1024 * 1024, 40);
+ var ramBytes = ramGb * 1024 * 1024 * 1024;
+ var block = DarlingManagedPostgres.BuildHardwareSizingConfAppend(ramBytes, 40);
+ var expected = DarlingManagedPostgres.DeriveMemorySettings(DarlingManagedPostgres.QuantizeRam(ramBytes));
- /* Anchored on the newline that starts every setting line. A bare "work_mem = " is a SUBSTRING of
- "maintenance_work_mem = ", so the unanchored form fails against a correct block — caught by the
- harness before this shipped, and the reason the positive assertion below is here as a guard. */
- Assert.DoesNotContain("\nwork_mem = ", block, StringComparison.Ordinal);
- Assert.Contains("\nmaintenance_work_mem = ", block, StringComparison.Ordinal);
+ /* Anchored on the newline that starts every setting line: a bare "work_mem = " is a SUBSTRING of
+ "maintenance_work_mem = ", so the unanchored form would pass even if this read the wrong line. */
+ Assert.Contains($"\neffective_cache_size = {expected.EffectiveCacheSizeMb}MB\n", block, StringComparison.Ordinal);
+ Assert.Contains($"\nmaintenance_work_mem = {expected.MaintenanceWorkMemMb}MB\n", block, StringComparison.Ordinal);
+ Assert.Contains($"\nwork_mem = {expected.WorkMemMb}MB\n", block, StringComparison.Ordinal);
+
+ /* shared_buffers stays excluded (CONSTRAINT PIN 1, #1559/#2845) — this pin is about the other three,
+ not a licence to re-derive every MemorySettings field. */
+ Assert.DoesNotContain("shared_buffers", block, StringComparison.Ordinal);
+ }
+
+ ///
+ /// The exact field measurement from #4207: the reported host RAM, 33,788,809,216 bytes (the issue's
+ /// "31.5 GiB" — actually 31.47 GiB, which rounds DOWN to
+ /// 31 GB, half a GB short of the 31.5 GB midpoint that would round up). At 31 GB, work_mem is
+ /// 62 MB, not the issue's rough "63 MB" (63 is 31.5 GiB's OWN raw RAM/512, i.e. what you get by
+ /// skipping the quantization step) — and not the 31 MB the stale v3 block left in force after the resize
+ /// either way. Either figure is roughly double the stale value, which is the point; this pins the one the
+ /// code actually derives from the reported bytes.
+ ///
+ [Fact]
+ public void HardwareSizingConfAppend_MeasuredFieldRam_EmitsWorkMem62Mb()
+ {
+ const long measuredFieldRamBytes = 33_788_809_216L;
+ var block = DarlingManagedPostgres.BuildHardwareSizingConfAppend(measuredFieldRamBytes, 40);
+
+ Assert.Contains("\nwork_mem = 62MB\n", block, StringComparison.Ordinal);
}
///
/// The block emits what it is for, at the values the resized fleet should have had. 31.5 GB is the
/// m7i.2xlarge reading; 32 GB is used here for a round assertion. effective_cache_size 24576MB is the
/// number #2845 was filed over — the boxes were sitting at 11.86 GB, which is 75% of the 16 GB they had
- /// before the resize.
+ /// before the resize. work_mem 64MB is the #4207 addition — the 32 GB round number lands exactly on the
+ /// formula's ceiling; covers the
+ /// field's actual reading, just under it.
///
[Fact]
public void HardwareSizingConfAppend_EmitsHostDerivedSettings()
@@ -1677,6 +1707,7 @@ public void HardwareSizingConfAppend_EmitsHostDerivedSettings()
Assert.Contains(DarlingManagedPostgres.ConfMarkerV8, block, StringComparison.Ordinal);
Assert.Contains("effective_cache_size = 24576MB", block, StringComparison.Ordinal); /* 75% of 32 GB (was 11.86 GB = 75% of 16 GB) */
Assert.Contains("maintenance_work_mem = 1638MB", block, StringComparison.Ordinal); /* 5% of 32 GB, past the 1536 floor, under the 2 GB cap */
+ Assert.Contains("work_mem = 64MB", block, StringComparison.Ordinal); /* RAM/512 = 64 MB, exactly the ceiling */
Assert.Contains("timescaledb.max_background_workers = 42", block, StringComparison.Ordinal); /* 40 hypertables + 2 */
Assert.Contains("max_worker_processes = 53", block, StringComparison.Ordinal); /* 3 + 42 + 8 */
}
@@ -1724,6 +1755,170 @@ public void HardwareFingerprint_NonPositiveRam_MatchesTheFallbackItDerivedUnder(
DarlingManagedPostgres.BuildHardwareFingerprint(0, 40));
}
+ /* ===================== #4207 v8 replace-in-place (was append-only) ===================== */
+
+ ///
+ /// PIN (#4207): two successive fingerprint changes — the field's actual pattern, RAM then hypertable
+ /// count — leave exactly ONE v8 block, not a growing pile. Before this fix each call to the equivalent
+ /// append appended a fresh block, which is how the field stores reached three copies from three
+ /// fingerprint changes since creation.
+ ///
+ [Fact]
+ public void ReplaceOrAppendHardwareSizingBlock_TwoSuccessiveFingerprintChanges_LeaveExactlyOneBlock()
+ {
+ const long sixteenGb = 16L * 1024 * 1024 * 1024;
+ const long thirtyTwoGb = 32L * 1024 * 1024 * 1024;
+
+ var conf = "shared_buffers = 1024MB\n";
+ conf = DarlingManagedPostgres.ReplaceOrAppendHardwareSizingBlock(
+ conf, DarlingManagedPostgres.BuildHardwareSizingConfAppend(sixteenGb, 40));
+ Assert.Equal(1, CountOccurrences(conf, DarlingManagedPostgres.ConfMarkerV8));
+
+ /* First change: a resize, 16 -> 32 GB. */
+ conf = DarlingManagedPostgres.ReplaceOrAppendHardwareSizingBlock(
+ conf, DarlingManagedPostgres.BuildHardwareSizingConfAppend(thirtyTwoGb, 40));
+ Assert.Equal(1, CountOccurrences(conf, DarlingManagedPostgres.ConfMarkerV8));
+
+ /* Second change: a hypertable count change with no resize, 32 GB stays but 40 -> 41. This is the
+ axis #2845 considered splitting into its own fingerprint; #4207 keeps it joined (see the
+ EnsureConfAppended v8 comment) because an in-place rewrite makes it exactly as cheap as a resize. */
+ conf = DarlingManagedPostgres.ReplaceOrAppendHardwareSizingBlock(
+ conf, DarlingManagedPostgres.BuildHardwareSizingConfAppend(thirtyTwoGb, 41));
+ Assert.Equal(1, CountOccurrences(conf, DarlingManagedPostgres.ConfMarkerV8));
+ Assert.True(DarlingManagedPostgres.ConfHasCurrentHardwareFingerprint(
+ conf, DarlingManagedPostgres.BuildHardwareFingerprint(thirtyTwoGb, 41)));
+
+ /* The unrelated line outside every block survived all three rewrites untouched. */
+ Assert.StartsWith("shared_buffers = 1024MB\n", conf, StringComparison.Ordinal);
+ }
+
+ ///
+ /// PIN (#4207): a conf carrying three v8 blocks — the exact shape #4207 measured on all three production
+ /// stores (72 -> 73 -> 74 background workers) — collapses to ONE on the next rewrite, at the
+ /// FIRST block's position. Rewriting the first position rather than the last is what keeps a manual
+ /// override positioned after the old last block still winning (see
+ /// ): collapsing can only move the
+ /// v8 lines EARLIER in the file, never later, so nothing that used to lose to the last block can start
+ /// winning, and nothing that used to beat it can start losing.
+ ///
+ [Fact]
+ public void ReplaceOrAppendHardwareSizingBlock_ThreeExistingBlocks_CollapseToOneAtTheFirstPosition()
+ {
+ const long sixteenGb = 16L * 1024 * 1024 * 1024;
+ const long thirtyTwoGb = 32L * 1024 * 1024 * 1024;
+ const long sixtyFourGb = 64L * 1024 * 1024 * 1024;
+
+ var block1 = DarlingManagedPostgres.BuildHardwareSizingConfAppend(sixteenGb, 40);
+ var block2 = DarlingManagedPostgres.BuildHardwareSizingConfAppend(thirtyTwoGb, 41);
+ var block3 = DarlingManagedPostgres.BuildHardwareSizingConfAppend(sixtyFourGb, 42);
+ var conf = "port = 5432\n" + block1 + block2 + block3;
+ var firstMarkerPosition = conf.IndexOf(DarlingManagedPostgres.ConfMarkerV8, StringComparison.Ordinal);
+ Assert.Equal(3, CountOccurrences(conf, DarlingManagedPostgres.ConfMarkerV8)); /* the broken shape, confirmed */
+
+ var newBlock = DarlingManagedPostgres.BuildHardwareSizingConfAppend(sixtyFourGb, 43);
+ var rewritten = DarlingManagedPostgres.ReplaceOrAppendHardwareSizingBlock(conf, newBlock);
+
+ Assert.Equal(1, CountOccurrences(rewritten, DarlingManagedPostgres.ConfMarkerV8));
+ Assert.Equal(
+ firstMarkerPosition,
+ rewritten.IndexOf(DarlingManagedPostgres.ConfMarkerV8, StringComparison.Ordinal));
+ Assert.True(DarlingManagedPostgres.ConfHasCurrentHardwareFingerprint(
+ rewritten, DarlingManagedPostgres.BuildHardwareFingerprint(sixtyFourGb, 43)));
+ }
+
+ ///
+ /// PIN (#4207): every byte outside a v8 span is byte-identical after a rewrite that both updates the
+ /// first block and removes a second one — INCLUDING an operator's own line sitting BETWEEN the two
+ /// blocks, which a naive "keep the first block's text, drop everything from the second marker on" splice
+ /// would lose even though it is not part of either block.
+ ///
+ [Fact]
+ public void ReplaceOrAppendHardwareSizingBlock_LinesOutsideBlocks_AreByteIdenticalAfterRewrite()
+ {
+ const long sixteenGb = 16L * 1024 * 1024 * 1024;
+ const long thirtyTwoGb = 32L * 1024 * 1024 * 1024;
+ const long sixtyFourGb = 64L * 1024 * 1024 * 1024;
+
+ const string before = "# operator header\nport = 5432\n";
+ /* Blank-line-led, like every block this file writes - see FindHardwareSizingBlockEnd's doc for why
+ an operator line with NO leading blank line is a known edge case this rule does not cover. */
+ const string between = "\nwork_mem = 999MB # an operator override sitting between two v8 blocks\n";
+ const string after = "\n# trailing operator block\nlisten_addresses = '*'\n";
+
+ var conf = before
+ + DarlingManagedPostgres.BuildHardwareSizingConfAppend(sixteenGb, 40)
+ + between
+ + DarlingManagedPostgres.BuildHardwareSizingConfAppend(thirtyTwoGb, 41)
+ + after;
+
+ var rewritten = DarlingManagedPostgres.ReplaceOrAppendHardwareSizingBlock(
+ conf, DarlingManagedPostgres.BuildHardwareSizingConfAppend(sixtyFourGb, 42));
+
+ Assert.StartsWith(before, rewritten, StringComparison.Ordinal);
+ Assert.Contains(between, rewritten, StringComparison.Ordinal);
+ Assert.EndsWith(after, rewritten, StringComparison.Ordinal);
+ Assert.Equal(1, CountOccurrences(rewritten, DarlingManagedPostgres.ConfMarkerV8));
+ }
+
+ ///
+ /// No existing v8 block falls back to a plain append — the v2-v7 shape — so a cluster's first-ever v8
+ /// write is unchanged by #4207.
+ ///
+ [Fact]
+ public void ReplaceOrAppendHardwareSizingBlock_NoExistingBlock_AppendsOne()
+ {
+ const long sixteenGb = 16L * 1024 * 1024 * 1024;
+ var conf = "port = 5432\n";
+ var block = DarlingManagedPostgres.BuildHardwareSizingConfAppend(sixteenGb, 40);
+
+ Assert.Equal(conf + block, DarlingManagedPostgres.ReplaceOrAppendHardwareSizingBlock(conf, block));
+ }
+
+ ///
+ /// The #4207 field defect end to end: a data directory whose postgresql.conf already carries three v8
+ /// blocks (the append-not-replace bug) collapses to one on the very next EnsureConfAppended call,
+ /// through the real heal path rather than the pure function directly, and the survivor carries
+ /// work_mem. A second heal is a no-op for v8: the fingerprint the first heal just wrote matches
+ /// this machine, so nothing is rewritten.
+ ///
+ [Fact]
+ public void EnsureConfAppended_ThreeExistingV8Blocks_CollapseToOneOnTheNextHeal()
+ {
+ var root = Directory.CreateTempSubdirectory("darling-v8collapse-");
+ try
+ {
+ var dataDirectory = Path.Combine(root.FullName, "pg");
+ Directory.CreateDirectory(dataDirectory);
+ var confPath = Path.Combine(dataDirectory, "postgresql.conf");
+
+ /* Fingerprints built from values no real test host will match (1-3 GB RAM, 1-3 hypertables), so
+ the v8 check is guaranteed to find the last one stale and act - exactly what the field stores
+ hit on every hypertable-count change once their RAM had already resized past the oldest
+ fingerprint. */
+ var stale1 = DarlingManagedPostgres.BuildHardwareSizingConfAppend(1L * 1024 * 1024 * 1024, 1);
+ var stale2 = DarlingManagedPostgres.BuildHardwareSizingConfAppend(2L * 1024 * 1024 * 1024, 2);
+ var stale3 = DarlingManagedPostgres.BuildHardwareSizingConfAppend(3L * 1024 * 1024 * 1024, 3);
+ File.WriteAllText(confPath, DarlingManagedPostgres.BuildConfAppend(5993) + stale1 + stale2 + stale3);
+
+ var pg = new DarlingManagedPostgres(
+ new PostgresConfig { Managed = true, Port = 5993, DataDirectory = dataDirectory }, NullLogger.Instance);
+
+ pg.EnsureConfAppended(dataDirectory);
+
+ var healed = File.ReadAllText(confPath);
+ Assert.Equal(1, CountOccurrences(healed, DarlingManagedPostgres.ConfMarkerV8));
+ Assert.Contains("\nwork_mem = ", healed, StringComparison.Ordinal);
+
+ pg.EnsureConfAppended(dataDirectory);
+ var healedAgain = File.ReadAllText(confPath);
+ Assert.Equal(1, CountOccurrences(healedAgain, DarlingManagedPostgres.ConfMarkerV8));
+ }
+ finally
+ {
+ root.Delete(recursive: true);
+ }
+ }
+
/* ===================== v12 wal sizing (#3802) ===================== */
private const long OneGb = 1024L * 1024 * 1024;
diff --git a/Darling/PerformanceMonitor.Darling.Service/DarlingManagedPostgres.cs b/Darling/PerformanceMonitor.Darling.Service/DarlingManagedPostgres.cs
index c84fa383a..418157423 100644
--- a/Darling/PerformanceMonitor.Darling.Service/DarlingManagedPostgres.cs
+++ b/Darling/PerformanceMonitor.Darling.Service/DarlingManagedPostgres.cs
@@ -1192,9 +1192,30 @@ internal static bool ShouldAppendHardwareSizing(string conf, bool ramReadingIsAu
/// was raised for — a planner hint with no allocation, found at 11.86 GB (75% of 16 GB) on hosts that
/// now have 31.5 GB, which biases the planner toward sequential scans on a store serving ~670k small
/// index lookups a day. maintenance_work_mem is a per-operation CEILING that PostgreSQL grows
- /// into rather than reserves, so re-deriving it cannot overcommit. The two worker settings are
- /// restart-only counts of background slots that only ever grow as collectors are added, and re-stating
- /// them is what finally makes the v2 block's "never goes stale" claim true.
+ /// into rather than reserves, so re-deriving it cannot overcommit. work_mem re-joined this list in
+ /// #4207: see the bullet below for why v3's original exclusion in #2845 does not hold up. The two worker
+ /// settings are restart-only counts of background slots that only ever grow as collectors are added, and
+ /// re-stating them is what finally makes the v2 block's "never goes stale" claim true.
+ ///
+ /// work_mem WAS excluded (#2845); #4207 measured why that was wrong. The original argument
+ /// was that the formula would take it 31 MB -> ~63 MB at 31.5 GB and the only measurements above 31 MB on
+ /// the heaviest read were WORSE: PlanRegressionSql at default 26,565 ms, at 31 MB 25,617 ms, and at
+ /// 512 MB 59,323 ms. That comparison never tested the value this formula actually derives — 512 MB
+ /// is 8x 's own 64 MB ceiling, a value nothing in this codebase would
+ /// ever write, so the regression it found says nothing about the ~63 MB case. What #2845 left unmeasured,
+ /// #4207 measured directly: three field stores stuck at the v3 block's 31 MB (16 GB-derived) after a
+ /// resize spilled 33 TB and 7 TB of pg_stat_database.temp_bytes to disk since creation, on
+ /// hosts reporting 33,788,809,216 bytes — nominally "31.5 GiB", actually 31.47 GiB, which
+ /// rounds DOWN to 31 GB (the 31.5 GB midpoint rounds up; this reading is half a
+ /// GB short of it) and turns into 62 MB, not the round "63 MB"
+ /// the issue's own back-of-envelope RAM/512 gave for a bare 31.5 GiB. Either figure is what nothing
+ /// re-applied — the exact staleness this whole block exists to heal, just for the one setting it skipped.
+ /// The claim that
+ /// work_mem is "not a property of the machine" is also narrower than it reads: the formula's own
+ /// ceiling (RAM/512, clamped 16-64 MB) is deliberately modest specifically BECAUSE it is a per-connection,
+ /// per-sort cost against a machine with a fixed amount of RAM (see ),
+ /// and a spill that costs disk I/O and wall-clock time is worse than the same query having had the RAM
+ /// its own host was sized to offer.
///
/// What it deliberately does NOT emit, and why the omissions are the load-bearing part.
///
@@ -1207,14 +1228,8 @@ internal static bool ShouldAppendHardwareSizing(string conf, bool ramReadingIsAu
/// any host above 4 GB, so a hardware change cannot move it and emitting it would buy nothing. The
/// reason to leave it out is the FUTURE one: if the cap is ever raised deliberately, that is a formula
/// change and belongs to a version-keyed block where it gets reviewed, not something a resize should
- /// silently propagate to production.
- /// - work_mem — EXCLUDED. The formula would take it 31 MB -> 63 MB at 31.5 GB, and the only
- /// measurements above 31 MB on this store's heaviest read are WORSE: PlanRegressionSql at default
- /// 26,565 ms, at 31 MB 25,617 ms, at 512 MB 59,323 ms (#2845). 63 MB is not 512 MB and no one has
- /// measured it, which is the point — the evidence that exists points the wrong way, so a resize is
- /// not the moment to move it. The deeper reason is that it does not belong to this block at all:
- /// everything here is a property of the MACHINE, while work_mem is a per-sort, per-connection ceiling
- /// whose right value follows from the QUERY MIX. The hardware changed; the sort behaviour did not.
+ /// silently propagate to production. work_mem has no such structural reason: nothing caps its formula
+ /// to a value a hardware change cannot move, which is exactly why letting it go stale had a cost.
/// - max_parallel_workers — not emitted because this class has never set it; it sits at the
/// PostgreSQL default of 8 regardless of core count. Deriving it from cores is a plausible want on a
/// 16-core host, but it is a behaviour change rather than a staleness fix, and it multiplies the
@@ -1225,10 +1240,10 @@ internal static bool ShouldAppendHardwareSizing(string conf, bool ramReadingIsAu
/// to do, and fingerprinting it would append a block of identical values on every resize.
///
///
- /// Reload semantics. effective_cache_size and maintenance_work_mem are
- /// SIGHUP-reloadable; the two worker settings are restart-only. The append runs before
- /// pg_ctl start on a service-owned start, so in practice the whole block takes effect on that
- /// very start — the same story as v3 and v7.
+ /// Reload semantics. effective_cache_size, maintenance_work_mem and
+ /// work_mem are all SIGHUP-reloadable; the two worker settings are restart-only. The append runs
+ /// before pg_ctl start on a service-owned start, so in practice the whole block takes effect on
+ /// that very start — the same story as v3 and v7.
///
internal static string BuildHardwareSizingConfAppend(long totalPhysicalMemoryBytes, int hypertableCount)
{
@@ -1241,11 +1256,154 @@ internal static string BuildHardwareSizingConfAppend(long totalPhysicalMemoryByt
builder.Append(BuildHardwareFingerprint(totalPhysicalMemoryBytes, hypertableCount)).Append('\n');
builder.Append("effective_cache_size = ").Append(settings.EffectiveCacheSizeMb).Append("MB\n");
builder.Append("maintenance_work_mem = ").Append(settings.MaintenanceWorkMemMb).Append("MB\n");
+ builder.Append("work_mem = ").Append(settings.WorkMemMb).Append("MB\n");
builder.Append("timescaledb.max_background_workers = ").Append(workers.MaxBackgroundWorkers).Append('\n');
builder.Append("max_worker_processes = ").Append(workers.MaxWorkerProcesses).Append('\n');
return builder.ToString();
}
+ ///
+ /// The [start, end) span of the v8 block whose marker begins at : from the
+ /// marker line through the last content line before the next blank line, or end of file (#4207).
+ ///
+ /// Why this is the rule, when the marker carries no end sentinel of its own (unlike the
+ /// begin/end pair replaces between). ,
+ /// like every Build*ConfAppend in this file, writes its block as ONE leading blank line — the
+ /// separator from whatever came before, itself OUTSIDE the block — followed by the marker and then
+ /// content lines with NO blank line between them. So the first blank line found after the marker is
+ /// always the start of what follows: either the next block's own leading separator, or trailing
+ /// whitespace at end of file. That holds for a v8 block written by ANY version of the builder, past or
+ /// future, not only today's line count — a setting added to or removed from the block moves where the
+ /// next blank line falls without this rule having to change.
+ ///
+ /// Known edge case. An operator line spliced in directly after a v8 block's last setting
+ /// line, with NO blank line of its own before it, reads as more content of that block rather than as
+ /// something outside it — every block this codebase writes is blank-line-separated from what follows
+ /// (each Build*ConfAppend begins with its own leading blank line), so this only bites a hand edit
+ /// that does not follow that convention. ALTER SYSTEM (postgresql.auto.conf) is unaffected either
+ /// way, since this function never reads that file.
+ ///
+ private static int FindHardwareSizingBlockEnd(string conf, int markerStart)
+ {
+ var cursor = conf.IndexOf('\n', markerStart);
+ if (cursor < 0)
+ {
+ return conf.Length;
+ }
+
+ cursor++;
+ while (cursor < conf.Length)
+ {
+ var lineEnd = conf.IndexOf('\n', cursor);
+ var line = lineEnd < 0 ? conf[cursor..] : conf[cursor..lineEnd];
+ if (line.TrimEnd('\r').Length == 0)
+ {
+ return cursor;
+ }
+
+ if (lineEnd < 0)
+ {
+ return conf.Length;
+ }
+
+ cursor = lineEnd + 1;
+ }
+
+ return cursor;
+ }
+
+ ///
+ /// Every v8 block's [start, end) span in , in file order — file order being
+ /// append order, so the first span is also the chronologically first block (#4207). On each of the three
+ /// field stores this returns three spans, one per fingerprint change since the store's creation, because
+ /// the prior code appended a fresh block on every change instead of replacing the one it superseded.
+ /// is what collapses them.
+ ///
+ internal static List<(int Start, int End)> FindHardwareSizingBlockSpans(string conf)
+ {
+ var spans = new List<(int Start, int End)>();
+ var searchFrom = 0;
+ while (true)
+ {
+ var markerStart = conf.IndexOf(ConfMarkerV8, searchFrom, StringComparison.Ordinal);
+ if (markerStart < 0)
+ {
+ break;
+ }
+
+ /* Defensive: the marker only means "a v8 block starts here" at the start of a line — it is
+ never written any other way — so a match that is not line-initial (impossible today, but
+ cheap to rule out) is skipped rather than treated as a block. */
+ if (markerStart > 0 && conf[markerStart - 1] != '\n')
+ {
+ searchFrom = markerStart + ConfMarkerV8.Length;
+ continue;
+ }
+
+ var end = FindHardwareSizingBlockEnd(conf, markerStart);
+ spans.Add((markerStart, end));
+ searchFrom = end;
+ }
+
+ return spans;
+ }
+
+ ///
+ /// Collapses however many v8 blocks carries into exactly one, at the position of
+ /// the FIRST (#4207). Every block after the first is a leftover from the append-not-replace bug — a
+ /// stale copy the code once left behind on every fingerprint change — and is removed outright; the first
+ /// is rewritten in place with 's content (the same string
+ /// returns for a plain append, leading blank line included).
+ /// No v8 block at all falls back to a plain append — the v2-v7 shape — so a cluster's first v8 write is
+ /// unchanged.
+ ///
+ /// Nothing outside a v8 span is touched, INCLUDING the blank line that separates one block from the
+ /// next: that separator is not part of either block under 's
+ /// rule, so removing a duplicate can leave a doubled blank line where three blocks once stood. That is
+ /// cosmetic — PostgreSQL ignores blank lines — and the alternative (also consuming the separator) would
+ /// touch a byte that is provably not part of any v8 block, which the pin on this function
+ /// (ReplaceOrAppendHardwareSizingBlock_LinesOutsideBlocks_AreByteIdenticalAfterRewrite) forbids.
+ ///
+ /// Why rewriting the FIRST block's position, not the last, keeps manual overrides winning
+ /// exactly as before. postgresql.conf takes the LAST occurrence of a setting, so what decides a
+ /// manual edit's fate is only ITS position relative to wherever the v8 lines end up — and collapsing can
+ /// only move that position EARLIER in the file (to the first block) or leave it unchanged (already one
+ /// block), never later. An edit that already sat after every v8 block still sits after the single
+ /// survivor; an edit that already lost to a later v8 block was losing before this function ever ran, for
+ /// the same reason. ALTER SYSTEM values in postgresql.auto.conf are unaffected either way —
+ /// that file is read after postgresql.conf in its entirety and outranks anything this function does.
+ ///
+ internal static string ReplaceOrAppendHardwareSizingBlock(string conf, string newBlockAppend)
+ {
+ var spans = FindHardwareSizingBlockSpans(conf);
+ if (spans.Count == 0)
+ {
+ return conf + newBlockAppend;
+ }
+
+ /* newBlockAppend carries the same leading blank line every Build*ConfAppend does; splicing it in at
+ an existing marker's position would double that separator, since the blank line already there
+ (untouched, being outside the span by definition) still precedes it. */
+ var content = newBlockAppend.StartsWith('\n') ? newBlockAppend[1..] : newBlockAppend;
+
+ var builder = new StringBuilder(conf.Length + content.Length);
+ var cursor = 0;
+ for (var i = 0; i < spans.Count; i++)
+ {
+ var (start, end) = spans[i];
+ builder.Append(conf, cursor, start - cursor);
+ if (i == 0)
+ {
+ builder.Append(content);
+ }
+
+ cursor = end;
+ }
+
+ builder.Append(conf, cursor, conf.Length - cursor);
+ return builder.ToString();
+ }
+
/* ===================== v9 session time zone ===================== */
///
@@ -2753,7 +2911,17 @@ it derived from has been replaced underneath it. This asks "was the sizing deriv
fresh initdb has just written v3 with identical values. The redundant first block is the price of
a simple invariant — after any start, the conf carries a fingerprint for the CURRENT host — and
without recording one on the first start there would be nothing for the second start to compare
- against. It converges immediately: the next start finds its own fingerprint and appends nothing. */
+ against. It converges immediately: the next start finds its own fingerprint and rewrites nothing.
+
+ REPLACES rather than appends (#4207). A fingerprint change used to append a fresh block, and
+ because the fingerprint includes the worker count, which moves with the hypertable count, each
+ field store had grown three copies by the time #4207 was filed. ReplaceOrAppendHardwareSizingBlock
+ rewrites the FIRST existing block in place and drops every other copy, so any fingerprint change —
+ a resize or a hypertable-count change alike — now costs one rewritten block, never a growing file.
+ That also removes the one remaining reason #2845 considered for splitting the worker count into
+ its own fingerprint (so a hypertable-count change would not re-trigger the memory lines): with an
+ in-place rewrite a worker-only change is exactly as cheap as a memory-only one, so the single
+ fingerprint stays single rather than gaining a second axis with nothing left to buy. */
/* INVARIANT this check depends on: `conf` was read ONCE at the top of this method, before v1-v7
may have appended. That is safe only because none of them emits a line carrying
ConfHardwareFingerprintPrefix, so nothing appended above can change this answer. A future version
@@ -2783,12 +2951,24 @@ written. Deriving the log line separately from the raw reading made them disagre
appears nowhere in the file. QuantizeRam is idempotent, so the call below still quantizes and
still gets the same answer. */
var v8QuantizedRam = QuantizeRam(v8RamBytes);
- File.AppendAllText(confPath, BuildHardwareSizingConfAppend(v8QuantizedRam, hypertableCount));
+ var v8Append = BuildHardwareSizingConfAppend(v8QuantizedRam, hypertableCount);
+
+ /* Re-read rather than reuse the `conf` snapshot from the top of this method: v1-v7 above may
+ have just appended their own healing blocks straight to disk (File.AppendAllText, bypassing
+ `conf` entirely), and rewriting the whole file from the stale snapshot would silently drop
+ them. None of v1-v7 can itself contain a v8 span, so this re-read cannot move or hide one. */
+ var v8CurrentConf = File.ReadAllText(confPath);
+ var v8PriorCopies = FindHardwareSizingBlockSpans(v8CurrentConf).Count;
+ File.WriteAllText(confPath, ReplaceOrAppendHardwareSizingBlock(v8CurrentConf, v8Append));
+
var v8Settings = DeriveMemorySettings(v8QuantizedRam);
var v8Workers = DeriveWorkerSettings(hypertableCount);
_logger.LogInformation(
- "Appended v8 hardware sizing to postgresql.conf (host RAM {RamMb} MB, {Hypertables} hypertables -> effective_cache_size {EffectiveCache}MB, maintenance_work_mem {Maintenance}MB, timescaledb.max_background_workers {BgWorkers}, max_worker_processes {WorkerProcesses}; shared_buffers and work_mem deliberately NOT re-derived, see #2845)",
- v8QuantizedRam / (1024L * 1024L), hypertableCount, v8Settings.EffectiveCacheSizeMb, v8Settings.MaintenanceWorkMemMb, v8Workers.MaxBackgroundWorkers, v8Workers.MaxWorkerProcesses);
+ "{Action} v8 hardware sizing in postgresql.conf (host RAM {RamMb} MB, {Hypertables} hypertables -> effective_cache_size {EffectiveCache}MB, maintenance_work_mem {Maintenance}MB, work_mem {WorkMem}MB, timescaledb.max_background_workers {BgWorkers}, max_worker_processes {WorkerProcesses}; shared_buffers deliberately NOT re-derived, see #2845){CollapseNote}",
+ v8PriorCopies == 0 ? "Appended" : "Rewrote", v8QuantizedRam / (1024L * 1024L), hypertableCount,
+ v8Settings.EffectiveCacheSizeMb, v8Settings.MaintenanceWorkMemMb, v8Settings.WorkMemMb,
+ v8Workers.MaxBackgroundWorkers, v8Workers.MaxWorkerProcesses,
+ v8PriorCopies > 1 ? $" (collapsed {v8PriorCopies} copies into 1, #4207)" : string.Empty);
}
/* Checked independently of v1-v8, and placed AFTER v8 on purpose: v8 keys on the last fingerprint