diff --git a/Darling/Darling.Tests/DarlingManagedPostgresTests.cs b/Darling/Darling.Tests/DarlingManagedPostgresTests.cs index a7390c834..c5939347e 100644 --- a/Darling/Darling.Tests/DarlingManagedPostgresTests.cs +++ b/Darling/Darling.Tests/DarlingManagedPostgresTests.cs @@ -1524,24 +1524,34 @@ public void ShouldAppendHardwareSizing_NonAuthoritativeRamReading_AppendsNothing const int hypertables = 40; var conf = DarlingManagedPostgres.BuildHardwareSizingConfAppend(sixteenGb, hypertables); + var thirtyTwoGbBlock = DarlingManagedPostgres.BuildHardwareSizingConfAppend(thirtyTwoGb, hypertables); /* A genuine 16 -> 32 GB resize DOES append, but only with an authoritative reading behind it. */ Assert.True(DarlingManagedPostgres.ShouldAppendHardwareSizing( conf, ramReadingIsAuthoritative: true, - DarlingManagedPostgres.BuildHardwareFingerprint(thirtyTwoGb, hypertables))); + DarlingManagedPostgres.BuildHardwareFingerprint(thirtyTwoGb, hypertables), thirtyTwoGbBlock)); /* The same apparent change, from a reading we could not trust, must do nothing at all. */ Assert.False(DarlingManagedPostgres.ShouldAppendHardwareSizing( conf, ramReadingIsAuthoritative: false, - DarlingManagedPostgres.BuildHardwareFingerprint(thirtyTwoGb, hypertables))); + DarlingManagedPostgres.BuildHardwareFingerprint(thirtyTwoGb, hypertables), thirtyTwoGbBlock)); /* And it stays inert for ANY value the GC fallback might invent, which is the append-loop case. */ foreach (var guessGb in new long[] { 3, 7, 12, 29 }) { + var guessBlock = DarlingManagedPostgres.BuildHardwareSizingConfAppend(guessGb * 1024 * 1024 * 1024, hypertables); Assert.False(DarlingManagedPostgres.ShouldAppendHardwareSizing( conf, ramReadingIsAuthoritative: false, - DarlingManagedPostgres.BuildHardwareFingerprint(guessGb * 1024 * 1024 * 1024, hypertables))); + DarlingManagedPostgres.BuildHardwareFingerprint(guessGb * 1024 * 1024 * 1024, hypertables), guessBlock)); } + + /* #4207: a non-authoritative reading must heal nothing even when BOTH of the new conditions are + also true — duplicate blocks present AND the newest one's content stale. Authoritativeness gates + the whole decision, not just the original fingerprint leg of it. */ + var duplicatesConf = conf + thirtyTwoGbBlock; + Assert.False(DarlingManagedPostgres.ShouldAppendHardwareSizing( + duplicatesConf, ramReadingIsAuthoritative: false, + DarlingManagedPostgres.BuildHardwareFingerprint(thirtyTwoGb, hypertables), thirtyTwoGbBlock)); } @@ -1919,6 +1929,238 @@ hit on every hypertable-count change once their RAM had already resized past the } } + /// + /// The v8 block AS AN OLDER BUILD WROTE IT, before work_mem rejoined the formula in #4207: every + /// line emits today, with the + /// work_mem line removed. This is the literal on-disk shape of a field store's stale v8 blocks — + /// its fingerprint line is untouched and therefore fully current, only the FORMULA that turned the + /// fingerprint's own inputs into settings gained a line since this block was written. + /// + private static string BuildLegacyV8BlockWithoutWorkMem(long totalPhysicalMemoryBytes, int hypertableCount) + { + var current = DarlingManagedPostgres.BuildHardwareSizingConfAppend(totalPhysicalMemoryBytes, hypertableCount); + return string.Join('\n', current.Split('\n').Where(line => !line.StartsWith("work_mem = ", StringComparison.Ordinal))); + } + + /// + /// THE FIELD DEFECT #4207 WAS REOPENED FOR (issuecomment-5837990283): a store whose v8 blocks were ALL + /// written before #4225 shipped has no fingerprint change left to trigger #2845's original check — its + /// RAM and hypertable count have not moved since its LAST v8 write, so + /// already says "current" — yet + /// the surviving blocks are still missing work_mem, and there are three of them (the + /// append-not-replace bug's own leftovers, #4225 fixed going forward but never retroactively). The v3 + /// block, frozen since the store's original initdb, is the field's own report of a stale + /// work_mem = 31MB line that nothing ever re-applies. + /// + [Fact] + public void ShouldAppendHardwareSizing_FieldCase_CurrentFingerprintButStaleContent_Heals() + { + const long currentRam = 33_788_809_216L; /* the fleet's actual reading (#4207): nominally "31.5 GiB" */ + const int hypertables = 40; + + var fingerprint = DarlingManagedPostgres.BuildHardwareFingerprint(currentRam, hypertables); + var expectedAppend = DarlingManagedPostgres.BuildHardwareSizingConfAppend(currentRam, hypertables); + var expectedWorkMemMb = DarlingManagedPostgres.DeriveMemorySettings( + DarlingManagedPostgres.QuantizeRam(currentRam)).WorkMemMb; + Assert.Equal(62, expectedWorkMemMb); /* the doc comment's own figure - sanity-checks the fixture */ + + var legacyBlock = BuildLegacyV8BlockWithoutWorkMem(currentRam, hypertables); + /* Leading "\n" boundary: a bare "work_mem" substring search would also match inside + "maintenance_work_mem", which the legacy block still legitimately carries. */ + Assert.DoesNotContain("\nwork_mem = ", legacyBlock, StringComparison.Ordinal); + + /* The v3 block: marker-guarded, so it is written ONCE at initdb and never rewritten again - frozen + at whatever the very first start derived, exactly the field's own report. */ + const string v3Block = + "\n" + DarlingManagedPostgres.ConfMarkerV3 + "\n" + + "shared_buffers = 1024MB\n" + + "effective_cache_size = 12288MB\n" + + "maintenance_work_mem = 2047MB\n" + + "work_mem = 31MB\n"; + + var conf = "port = 5432\n" + v3Block + legacyBlock + legacyBlock + legacyBlock; + Assert.Equal(3, CountOccurrences(conf, DarlingManagedPostgres.ConfMarkerV8)); + Assert.True( + DarlingManagedPostgres.ConfHasCurrentHardwareFingerprint(conf, fingerprint), + "the fixture's premise: the newest v8 block's fingerprint already matches today's inputs"); + + /* THE FIX: heals anyway, because of the duplicate-block and stale-content conditions #4207 added - + see ShouldAppendHardwareSizing_FieldCase_OldFingerprintOnlyPredicate_WouldWronglySkip for what the + original single-condition check does with this exact fixture. */ + Assert.True(DarlingManagedPostgres.ShouldAppendHardwareSizing( + conf, ramReadingIsAuthoritative: true, fingerprint, expectedAppend)); + + var healed = DarlingManagedPostgres.ReplaceOrAppendHardwareSizingBlock(conf, expectedAppend); + Assert.Equal(1, CountOccurrences(healed, DarlingManagedPostgres.ConfMarkerV8)); + + /* The v3 line is untouched - still there, still 31MB - but the v8 line lands LATER in the file, so + postgresql.conf's last-occurrence-wins rule makes IT the value in force. */ + Assert.Contains("work_mem = 31MB", healed, StringComparison.Ordinal); + Assert.Equal($"{expectedWorkMemMb}MB", LastSettingValue(healed, "work_mem")); + Assert.True( + healed.IndexOf(DarlingManagedPostgres.ConfMarkerV8, StringComparison.Ordinal) + > healed.IndexOf(DarlingManagedPostgres.ConfMarkerV3, StringComparison.Ordinal)); + + /* IDEMPOTENT: a second pass over the healed text changes nothing, byte for byte. */ + Assert.False(DarlingManagedPostgres.ShouldAppendHardwareSizing( + healed, ramReadingIsAuthoritative: true, fingerprint, expectedAppend)); + Assert.Equal(healed, DarlingManagedPostgres.ReplaceOrAppendHardwareSizingBlock(healed, expectedAppend)); + } + + /// + /// PIN: the OLD, #2845-only predicate — authoritative-and-fingerprint-mismatch, with neither of #4207's + /// two new conditions — says "nothing to do" on the exact fixture + /// proves + /// needs healing. Reproduced literally rather than called, since the function no longer exposes that + /// expression alone: this IS the regression #4207 was reopened to describe, and reverting + /// to just this line is what #4225 + /// shipped and left three field stores unhealed. + /// + [Fact] + public void ShouldAppendHardwareSizing_FieldCase_OldFingerprintOnlyPredicate_WouldWronglySkip() + { + const long currentRam = 33_788_809_216L; + const int hypertables = 40; + var fingerprint = DarlingManagedPostgres.BuildHardwareFingerprint(currentRam, hypertables); + var legacyBlock = BuildLegacyV8BlockWithoutWorkMem(currentRam, hypertables); + var conf = "port = 5432\n" + legacyBlock + legacyBlock + legacyBlock; + + var oldPredicateResult = true && !DarlingManagedPostgres.ConfHasCurrentHardwareFingerprint(conf, fingerprint); + + Assert.False(oldPredicateResult, "the old predicate misses this store entirely - its fingerprint is already current"); + } + + /// + /// A conf that is ALREADY the block this build would write is not rewritten — #4207's two new + /// conditions must not turn every start into a rewrite of a perfectly healthy store. + /// + [Fact] + public void ShouldAppendHardwareSizing_AlreadyCurrentSingleBlock_IsNotRewritten() + { + const long ram = 32L * 1024 * 1024 * 1024; + const int hypertables = 40; + var fingerprint = DarlingManagedPostgres.BuildHardwareFingerprint(ram, hypertables); + var block = DarlingManagedPostgres.BuildHardwareSizingConfAppend(ram, hypertables); + var conf = "port = 5432\n" + block; + + Assert.False(DarlingManagedPostgres.ShouldAppendHardwareSizing( + conf, ramReadingIsAuthoritative: true, fingerprint, block)); + + /* Same property with a CRLF FILE, since a Windows-edited conf can carry them (#4207: the content + comparison must normalise, not just the fingerprint-line comparison #2845 already did). Only the + FILE is CRLF'd, not the freshly-rendered append: BuildHardwareSizingConfAppend, like every + Build*ConfAppend in this class, only ever emits LF, so that is the shape production always passes + as expectedBlockAppend regardless of what is already on disk. */ + var crlfConf = conf.Replace("\n", "\r\n", StringComparison.Ordinal); + Assert.False(DarlingManagedPostgres.ShouldAppendHardwareSizing( + crlfConf, ramReadingIsAuthoritative: true, fingerprint, block)); + } + + /// + /// #4207 (reopened) END TO END, proven against a real server: a store whose v8 blocks were ALL written + /// before work_mem rejoined the formula, with a fingerprint that already matches this host, + /// adopts the derived work_mem on its very next service-owned start — the live half of + /// . + /// + /// The BEFORE conf is built from a REAL fresh initdb's own v8 block (so the fingerprint is + /// genuinely current for whatever RAM and hypertable count this runner actually has), with its + /// work_mem line stripped and the single block tripled — reproducing the exact field shape + /// without needing a specific RAM figure to land on. + /// + [Fact] + public async Task ExistingStore_HealsPreFixV8Blocks_OnNextStart_Gated() + { + var runtimeRoot = Environment.GetEnvironmentVariable("DARLING_TEST_PGRUNTIME"); + Assert.SkipWhen(string.IsNullOrWhiteSpace(runtimeRoot), + "Set DARLING_TEST_PGRUNTIME to an assembled pg-runtime directory (the folder containing pgsql\\bin\\pg_ctl.exe; " + + "Darling\\tools\\fetch-pg-runtime.ps1 -KeepWork leaves one under artifacts\\pg-runtime-work\\assemble\\pg-runtime) " + + "to run the #4207 conf-heal E2E."); + Assert.SkipUnless(OperatingSystem.IsWindows(), "The bundled runtime is Windows-only."); + Assert.SkipUnless(File.Exists(Path.Combine(runtimeRoot!, "pgsql", "bin", "pg_ctl.exe")), + $"DARLING_TEST_PGRUNTIME={runtimeRoot} does not contain pgsql\\bin\\pg_ctl.exe."); + + var root = Directory.CreateTempSubdirectory("darling-pgv8heal-"); + var dataDirectory = Path.Combine(root.FullName, "pg"); + var config = new PostgresConfig + { + Managed = true, + Port = FindFreeTcpPort(), + DataDirectory = dataDirectory, + }; + var confPath = Path.Combine(dataDirectory, "postgresql.conf"); + + var owner = new DarlingManagedPostgres(config, NullLogger.Instance, runtimeRoot); + try + { + using var timeout = new CancellationTokenSource(TimeSpan.FromMinutes(8)); + + /* A real store, provisioned the normal way: exactly one CURRENT v8 block, with work_mem. */ + await owner.EnsureRunningAsync(timeout.Token); + await owner.StopIfStartedByThisProcessAsync(); + + var fresh = await File.ReadAllTextAsync(confPath, timeout.Token); + var spans = DarlingManagedPostgres.FindHardwareSizingBlockSpans(fresh); + Assert.Single(spans); + var (v8Start, v8End) = spans[0]; + var freshBlock = fresh[v8Start..v8End]; + var derivedWorkMem = LastSettingValue(fresh, "work_mem"); + Assert.NotNull(derivedWorkMem); + Assert.Contains("work_mem = " + derivedWorkMem, freshBlock, StringComparison.Ordinal); + + /* Reproduce the field shape: the same block, minus work_mem, three times over - the + append-not-replace leftovers (#4225 fixed the bug; this store's blocks predate the fix) with a + fingerprint that is ALREADY current, since it came straight from this run's own real start. */ + var legacyBlock = string.Join( + '\n', + freshBlock.Split('\n').Where(line => !line.StartsWith("work_mem = ", StringComparison.Ordinal))); + /* Checked on the single copy, before tripling and splicing: the v3 block elsewhere in this real + conf can legitimately carry the SAME derived value on a host whose RAM clamps both formulas to + the same ceiling (64MB), so asserting against the whole file would be a false failure on such + a host rather than a check of what THIS splice removed. */ + Assert.DoesNotContain("\nwork_mem = ", legacyBlock, StringComparison.Ordinal); + + /* A blank line between each copy - the separator every Build*ConfAppend leads with, and so the + shape every REAL append-not-replace duplicate carried. fresh[..v8Start] already supplies the + separator before the first copy, and fresh[v8End..] already supplies one after the last, so + only the two seams IN BETWEEN need one inserted; without it FindHardwareSizingBlockSpans reads + all three copies as a single span (nothing blank to stop it at), and the fixture would not be + the three-block shape #4207 describes. */ + var legacyConf = fresh[..v8Start] + legacyBlock + '\n' + legacyBlock + '\n' + legacyBlock + fresh[v8End..]; + await File.WriteAllTextAsync(confPath, legacyConf, timeout.Token); + + Assert.Equal(3, DarlingManagedPostgres.FindHardwareSizingBlockSpans(legacyConf).Count); + + /* The service-owned start: EnsureConfAppended heals BEFORE pg_ctl start, so the derived value + is live on this very start rather than one restart later. */ + var healedOwner = new DarlingManagedPostgres(config, NullLogger.Instance, runtimeRoot); + var healedConnectionString = await healedOwner.EnsureRunningAsync(timeout.Token); + try + { + var healedConf = await File.ReadAllTextAsync(confPath, timeout.Token); + Assert.Equal(1, CountOccurrences(healedConf, DarlingManagedPostgres.ConfMarkerV8)); + Assert.Equal(derivedWorkMem, LastSettingValue(healedConf, "work_mem")); + + var (live, expected) = await ReadSettingAndLiteralBytesAsync( + healedConnectionString, "work_mem", derivedWorkMem!, timeout.Token); + Assert.Equal(expected, live); + } + finally + { + await healedOwner.StopIfStartedByThisProcessAsync(); + } + + /* A third start must not append a second v8 block - already current, nothing left to heal. */ + Assert.Equal( + 1, + CountOccurrences(await File.ReadAllTextAsync(confPath, timeout.Token), DarlingManagedPostgres.ConfMarkerV8)); + } + finally + { + await owner.StopIfStartedByThisProcessAsync(); + TryDeleteRecursive(root.FullName); + } + } + /* ===================== v15 WAL compression (#4246) ===================== */ /// diff --git a/Darling/PerformanceMonitor.Darling.Service/DarlingManagedPostgres.cs b/Darling/PerformanceMonitor.Darling.Service/DarlingManagedPostgres.cs index 985e62d17..392736863 100644 --- a/Darling/PerformanceMonitor.Darling.Service/DarlingManagedPostgres.cs +++ b/Darling/PerformanceMonitor.Darling.Service/DarlingManagedPostgres.cs @@ -1234,19 +1234,80 @@ private static bool LastLineWithPrefixEquals(string conf, string prefix, string } /// - /// Whether the v8 block should be appended on this start (#2845) — the whole decision as one pure - /// function so the property can be pinned without a data directory. + /// Whether the v8 block should be appended on this start (#2845; the second and third conditions are + /// #4207's heal for a store resized BEFORE that fix shipped) — the whole decision as one pure function + /// so the property can be pinned without a data directory. /// - /// Two conditions, and the FIRST is the one that is easy to get wrong: the RAM reading must be - /// authoritative. A non-authoritative reading is not evidence that the hardware is unchanged, it is the - /// absence of evidence either way — and re-deriving production sizing from a number we could not read is - /// worse than leaving the last good block in force. It also stops a flapping Win32 call from minting a - /// novel fingerprint on every blip and appending a block each time, which a value-only guard cannot do - /// because the fallback it would guard against is a live, varying quantity rather than a fixed - /// sentinel. + /// The RAM reading must be authoritative for ANY of the three conditions below to act — checked + /// first, and short-circuiting the rest. A non-authoritative reading is not evidence that the hardware + /// is unchanged, it is the absence of evidence either way — and re-deriving production sizing from a + /// number we could not read is worse than leaving the last good block in force. It also stops a + /// flapping Win32 call from minting a novel fingerprint on every blip and appending a block each time, + /// which a value-only guard cannot do because the fallback it would guard against is a live, varying + /// quantity rather than a fixed sentinel. + /// + /// Given an authoritative reading, any of three conditions triggers a heal: + /// + /// the newest fingerprint in the conf does not match today's inputs (#2845's original condition — + /// a genuine hardware or hypertable-count change). + /// the conf holds MORE THAN ONE v8 block (). #4225 made + /// a fingerprint change collapse to a single rewritten block, but a store that had already accumulated + /// duplicates before that fix shipped has no fingerprint change left to trigger on — its newest + /// fingerprint already matches, so the first condition alone would leave the duplicates in place + /// forever. + /// the newest block's content does not match what THIS BUILD would write for those same inputs + /// (), even though its fingerprint line matches. A block + /// written by an older build — before work_mem rejoined this list in #4207 — fingerprints as + /// "current" for its RAM and hypertable count, because the fingerprint encodes only those two inputs, + /// never the formula version that turned them into settings. Without this condition, that store would + /// never re-derive: nothing about its hardware ever changes again, so the first condition never fires + /// either. + /// + /// + internal static bool ShouldAppendHardwareSizing( + string conf, bool ramReadingIsAuthoritative, string expectedFingerprint, string expectedBlockAppend) + => ramReadingIsAuthoritative + && (!ConfHasCurrentHardwareFingerprint(conf, expectedFingerprint) + || FindHardwareSizingBlockSpans(conf).Count > 1 + || !NewestHardwareSizingBlockIsCurrent(conf, expectedBlockAppend)); + + /// + /// True when the LAST v8 block in is, line for line, the text + /// would write for the current inputs (#4207) — the + /// stale-CONTENT half of 's decision, checked even when the + /// fingerprint line itself already matches (see that method's remarks for why fingerprint-only misses a + /// block written by an older formula). + /// + /// False when there is no v8 block at all: that reads as "not current" and defers to + /// 's plain-append fallback, the same v2-v7 shape as + /// before. + /// + /// Line endings are normalised before comparing. can be CRLF — this file + /// is written and hand-edited on Windows — while every Build*ConfAppend in this class emits LF + /// only, so a byte comparison would read every CRLF conf as permanently stale and rewrite it on every + /// single start. /// - internal static bool ShouldAppendHardwareSizing(string conf, bool ramReadingIsAuthoritative, string expectedFingerprint) - => ramReadingIsAuthoritative && !ConfHasCurrentHardwareFingerprint(conf, expectedFingerprint); + internal static bool NewestHardwareSizingBlockIsCurrent(string conf, string expectedBlockAppend) + { + var spans = FindHardwareSizingBlockSpans(conf); + if (spans.Count == 0) + { + return false; + } + + var (start, end) = spans[^1]; + var actual = conf[start..end]; + + /* expectedBlockAppend carries the same leading blank-line separator BuildHardwareSizingConfAppend + always does; a block SPAN never includes that separator (see FindHardwareSizingBlockEnd), so it + is stripped here to compare like with like. */ + var expected = expectedBlockAppend.StartsWith('\n') ? expectedBlockAppend[1..] : expectedBlockAppend; + + return string.Equals( + actual.Replace("\r\n", "\n", StringComparison.Ordinal), + expected.Replace("\r\n", "\n", StringComparison.Ordinal), + StringComparison.Ordinal); + } /// /// The v8 hardware-sizing block (#2845): re-states the settings that are a pure function of the @@ -3029,32 +3090,47 @@ which is not a substring of this prefix (pinned), so it neither disturbs this re _logger.LogWarning( "Skipped the v8 hardware-sizing check: total physical memory could not be read authoritatively, so a hardware change cannot be distinguished from a failed reading. The existing sizing block stays in force."); } - else if (ShouldAppendHardwareSizing(conf, v8Authoritative, v8Fingerprint)) + else { /* Quantize ONCE here and pass the result down, so the values logged are necessarily the values written. Deriving the log line separately from the raw reading made them disagree near a GB boundary — a 31.5 GB host writes effective_cache_size 24576MB but logged 24192MB, a number that appears nowhere in the file. QuantizeRam is idempotent, so the call below still quantizes and - still gets the same answer. */ + still gets the same answer. Built unconditionally (not only once ShouldAppendHardwareSizing + says yes): #4207's stale-content condition needs the text THIS build would write to compare + against what is already there, so the decision itself depends on this value. */ var v8QuantizedRam = QuantizeRam(v8RamBytes); 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( - "{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); + if (ShouldAppendHardwareSizing(conf, v8Authoritative, v8Fingerprint, v8Append)) + { + /* Which of #4207's three conditions fired, for the log line below. Classified against the + same `conf` snapshot ShouldAppendHardwareSizing just decided on, in the same priority + order that function checks them in — not against the re-read below, though the answer is + identical either way (see the INVARIANT comment above: none of v1-v7 can introduce, remove + or move a v8 span). */ + var v8Reason = + !ConfHasCurrentHardwareFingerprint(conf, v8Fingerprint) ? "hardware fingerprint changed" + : FindHardwareSizingBlockSpans(conf).Count > 1 ? "duplicate v8 blocks found, #4207" + : "existing block content is stale, #4207"; + + /* 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( + "{Action} v8 hardware sizing in postgresql.conf ({Reason}; 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", v8Reason, 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