From ae1e902dca00175f0088446be265b99cce41255b Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Thu, 1 Oct 2026 23:10:21 -0400 Subject: [PATCH 01/11] Tests: a runtime rescue that hits a brief file lock retries before it defers the update --- .../Darling.Tests/DarlingStoreUpgradeTests.cs | 286 +++++++++++++++++- .../DarlingStoreUpgrade.cs | 13 +- 2 files changed, 296 insertions(+), 3 deletions(-) diff --git a/Darling/Darling.Tests/DarlingStoreUpgradeTests.cs b/Darling/Darling.Tests/DarlingStoreUpgradeTests.cs index 25113e1a0..6849d156e 100644 --- a/Darling/Darling.Tests/DarlingStoreUpgradeTests.cs +++ b/Darling/Darling.Tests/DarlingStoreUpgradeTests.cs @@ -2678,7 +2678,8 @@ public async Task RuntimeAdvance_APreviousRuntimeItCannotDelete_DefersTheSwapIns using var hold = new FileStream(heldFile, FileMode.Open, FileAccess.Read, FileShare.None); var log = new CapturingLogger(); - var advance = await new DarlingStoreUpgrade(log).TryAdvanceRuntimeAsync( + /* The lock is held throughout, so the whole retry budget is spent; do not sleep through it. */ + var advance = await new DarlingStoreUpgrade(log) { RetryDelay = (_, _) => Task.CompletedTask }.TryAdvanceRuntimeAsync( host.RuntimeRoot, host.Package, host.DataDirectory, /* nothing is running in this fixture */ (_, _) => Task.FromResult(false), TestContext.Current.CancellationToken); @@ -2773,6 +2774,289 @@ public async Task RuntimeAdvance_APreviousRuntimeItCanClear_IsReplacedAndTheSwap } } + /* ================================================================================== + The runtime rescue's bounded retry. Just after the store stops, an antivirus scan or the exiting + server can hold the runtime folder for a moment, and the first lock used to defer the whole update + to the next service start. The portable pins stand in for the lock through the seams, because a + file lock blocks rename and delete on Windows only; the real-lock pins skip elsewhere. + ================================================================================== */ + + private static readonly TimeSpan[] s_expectedRetryDelays = + [ + TimeSpan.FromSeconds(0.5), TimeSpan.FromSeconds(1), TimeSpan.FromSeconds(2), TimeSpan.FromSeconds(2), + ]; + + private static int CountRetryLines(CapturingLogger log) + => log.ToString().Split(Environment.NewLine) + .Count(line => line.StartsWith("[Information]", StringComparison.Ordinal) + && line.Contains("Retrying", StringComparison.Ordinal)); + + private static bool HasWarning(CapturingLogger log) + => log.ToString().Split(Environment.NewLine) + .Any(line => line.StartsWith("[Warning]", StringComparison.Ordinal) + && (line.Contains("Could not rescue the current runtime", StringComparison.Ordinal) + || line.Contains("Could not clear the previous runtime", StringComparison.Ordinal))); + + [Fact] + public async Task RuntimeAdvance_AMoveThatIsLockedTwice_RetriesAndThenSwaps() + { + var root = Directory.CreateTempSubdirectory("darling-move-retry-"); + try + { + var host = PlantHostAwaitingARuntimeSwap(root.FullName); + var log = new CapturingLogger(); + var delays = new List(); + var attempts = 0; + var upgrade = new DarlingStoreUpgrade(log) + { + RetryDelay = (delay, _) => { delays.Add(delay); return Task.CompletedTask; }, + MoveRuntimeDirectory = (from, to) => + { + if (++attempts <= 2) + { + throw new IOException("The process cannot access the file because it is being used by another process."); + } + + Directory.Move(from, to); + }, + }; + + var advance = await upgrade.TryAdvanceRuntimeAsync( + host.RuntimeRoot, host.Package, host.DataDirectory, + (_, _) => Task.FromResult(false), + TestContext.Current.CancellationToken); + + Assert.True(advance.Swapped); + Assert.Equal(3, attempts); + Assert.Equal(2, CountRetryLines(log)); + Assert.False(HasWarning(log)); + Assert.Equal(HostAwaitingARuntimeSwap.PackageRuntime, File.ReadAllText(host.PgCtl)); + Assert.Equal(DarlingStoreUpgrade.ComputeFileHash(host.Package), File.ReadAllText(host.StampPath).Trim()); + Assert.Equal(s_expectedRetryDelays.Take(2), delays); + } + finally + { + TryDeleteTree(root.FullName); + } + } + + [Fact] + public async Task RuntimeAdvance_AMoveThatStaysLocked_RetriesFourTimesThenDefers() + { + var root = Directory.CreateTempSubdirectory("darling-move-stuck-"); + try + { + var host = PlantHostAwaitingARuntimeSwap(root.FullName); + var log = new CapturingLogger(); + var delays = new List(); + var attempts = 0; + var upgrade = new DarlingStoreUpgrade(log) + { + RetryDelay = (delay, _) => { delays.Add(delay); return Task.CompletedTask; }, + MoveRuntimeDirectory = (_, _) => + { + attempts++; + throw new UnauthorizedAccessException("Access to the path is denied."); + }, + }; + + var advance = await upgrade.TryAdvanceRuntimeAsync( + host.RuntimeRoot, host.Package, host.DataDirectory, + (_, _) => Task.FromResult(false), + TestContext.Current.CancellationToken); + + Assert.False(advance.Swapped); + Assert.Null(advance.PreviousBinDirectory); + Assert.Equal(5, attempts); + Assert.Equal(4, CountRetryLines(log)); + Assert.Equal(s_expectedRetryDelays, delays); + Assert.Equal(HostAwaitingARuntimeSwap.LiveRuntime, File.ReadAllText(host.PgCtl)); + Assert.Equal(HostAwaitingARuntimeSwap.PriorStamp, File.ReadAllText(host.StampPath).Trim()); + var warning = Assert.Single( + log.ToString().Split(Environment.NewLine), + line => line.Contains("Could not rescue the current runtime", StringComparison.Ordinal)); + Assert.StartsWith("[Warning]", warning, StringComparison.Ordinal); + } + finally + { + TryDeleteTree(root.FullName); + } + } + + [Fact] + public async Task RuntimeAdvance_AClearThatIsLockedTwice_RetriesAndThenSwaps() + { + var root = Directory.CreateTempSubdirectory("darling-clear-retry-"); + try + { + var host = PlantHostAwaitingARuntimeSwap(root.FullName); + var log = new CapturingLogger(); + var delays = new List(); + var attempts = 0; + var upgrade = new DarlingStoreUpgrade(log) + { + RetryDelay = (delay, _) => { delays.Add(delay); return Task.CompletedTask; }, + ClearPreviousRuntime = path => + { + if (++attempts <= 2) + { + throw new IOException("The process cannot access the file because it is being used by another process."); + } + + DarlingStoreUpgrade.EmptyDirectory(path); + }, + }; + + var advance = await upgrade.TryAdvanceRuntimeAsync( + host.RuntimeRoot, host.Package, host.DataDirectory, + (_, _) => Task.FromResult(false), + TestContext.Current.CancellationToken); + + Assert.True(advance.Swapped); + Assert.Equal(3, attempts); + Assert.Equal(2, CountRetryLines(log)); + Assert.False(HasWarning(log)); + Assert.Equal(HostAwaitingARuntimeSwap.PackageRuntime, File.ReadAllText(host.PgCtl)); + Assert.Equal(DarlingStoreUpgrade.ComputeFileHash(host.Package), File.ReadAllText(host.StampPath).Trim()); + Assert.Equal(s_expectedRetryDelays.Take(2), delays); + } + finally + { + TryDeleteTree(root.FullName); + } + } + + [Fact] + public async Task RuntimeAdvance_AClearThatStaysLocked_RetriesFourTimesThenDefers() + { + var root = Directory.CreateTempSubdirectory("darling-clear-stuck-"); + try + { + var host = PlantHostAwaitingARuntimeSwap(root.FullName); + var log = new CapturingLogger(); + var delays = new List(); + var attempts = 0; + var upgrade = new DarlingStoreUpgrade(log) + { + RetryDelay = (delay, _) => { delays.Add(delay); return Task.CompletedTask; }, + ClearPreviousRuntime = _ => + { + attempts++; + throw new IOException("The process cannot access the file because it is being used by another process."); + }, + }; + + var advance = await upgrade.TryAdvanceRuntimeAsync( + host.RuntimeRoot, host.Package, host.DataDirectory, + (_, _) => Task.FromResult(false), + TestContext.Current.CancellationToken); + + AssertSwapDeferred(advance, host, log); + Assert.Equal(5, attempts); + Assert.Equal(4, CountRetryLines(log)); + Assert.Equal(s_expectedRetryDelays, delays); + } + finally + { + TryDeleteTree(root.FullName); + } + } + + [Fact] + public async Task RuntimeAdvance_ACancellationDuringARetryDelay_Propagates_NotADeferral() + { + var root = Directory.CreateTempSubdirectory("darling-retry-cancel-"); + try + { + var host = PlantHostAwaitingARuntimeSwap(root.FullName); + var log = new CapturingLogger(); + using var cts = CancellationTokenSource.CreateLinkedTokenSource(TestContext.Current.CancellationToken); + var upgrade = new DarlingStoreUpgrade(log) + { + /* The caller's token must reach the delay: cancelling it there ends the wait. */ + RetryDelay = (delay, token) => + { + cts.Cancel(); + return Task.Delay(delay, token); + }, + MoveRuntimeDirectory = (_, _) => throw new IOException("held"), + }; + + await Assert.ThrowsAnyAsync(() => upgrade.TryAdvanceRuntimeAsync( + host.RuntimeRoot, host.Package, host.DataDirectory, + (_, _) => Task.FromResult(false), + cts.Token)); + + Assert.False(HasWarning(log)); + Assert.Equal(HostAwaitingARuntimeSwap.PriorStamp, File.ReadAllText(host.StampPath).Trim()); + } + finally + { + TryDeleteTree(root.FullName); + } + } + + /// + /// The real lock, on the platform where one blocks a rename: a file under the live runtime is held for + /// about a second, which is longer than the first attempt and shorter than the retry budget. + /// + [Fact] + public async Task RuntimeAdvance_ALiveRuntimeHeldForAMoment_IsRescuedOnceTheLockClears() + { + Assert.SkipUnless(OperatingSystem.IsWindows(), "File locks block rename and delete on Windows only."); + var root = Directory.CreateTempSubdirectory("darling-move-held-"); + try + { + var host = PlantHostAwaitingARuntimeSwap(root.FullName); + var hold = new FileStream(host.PgCtl, FileMode.Open, FileAccess.Read, FileShare.None); + using var release = new Timer(_ => hold.Dispose(), null, TimeSpan.FromSeconds(1), Timeout.InfiniteTimeSpan); + + var advance = await new DarlingStoreUpgrade(new CapturingLogger()).TryAdvanceRuntimeAsync( + host.RuntimeRoot, host.Package, host.DataDirectory, + (_, _) => Task.FromResult(false), + TestContext.Current.CancellationToken); + + Assert.True(advance.Swapped); + Assert.Equal(HostAwaitingARuntimeSwap.PackageRuntime, File.ReadAllText(host.PgCtl)); + Assert.Equal(DarlingStoreUpgrade.ComputeFileHash(host.Package), File.ReadAllText(host.StampPath).Trim()); + } + finally + { + TryDeleteTree(root.FullName); + } + } + + /// The same real lock, on a file under the last update's rescued runtime. + [Fact] + public async Task RuntimeAdvance_APreviousRuntimeHeldForAMoment_IsClearedOnceTheLockClears() + { + Assert.SkipUnless(OperatingSystem.IsWindows(), "File locks block rename and delete on Windows only."); + var root = Directory.CreateTempSubdirectory("darling-prev-held-"); + try + { + var host = PlantHostAwaitingARuntimeSwap(root.FullName); + var previousRoot = DarlingStoreUpgrade.PreviousRuntimeRootFor(host.RuntimeRoot); + var heldFile = Path.Combine(previousRoot, "pgsql", "bin", "postgres.exe"); + Directory.CreateDirectory(Path.GetDirectoryName(heldFile)!); + File.WriteAllText(heldFile, "previous runtime"); + var hold = new FileStream(heldFile, FileMode.Open, FileAccess.Read, FileShare.None); + using var release = new Timer(_ => hold.Dispose(), null, TimeSpan.FromSeconds(1), Timeout.InfiniteTimeSpan); + + var advance = await new DarlingStoreUpgrade(new CapturingLogger()).TryAdvanceRuntimeAsync( + host.RuntimeRoot, host.Package, host.DataDirectory, + (_, _) => Task.FromResult(false), + TestContext.Current.CancellationToken); + + Assert.True(advance.Swapped); + Assert.False(File.Exists(heldFile)); + Assert.Equal(HostAwaitingARuntimeSwap.PackageRuntime, File.ReadAllText(host.PgCtl)); + } + finally + { + TryDeleteTree(root.FullName); + } + } + /// /// #4052: the narrowed install-root grant leaves the service Modify on pg-runtime-prev itself but /// only Read & Execute on the root above it, so a delete-then-recreate of the folder can delete and then diff --git a/Darling/PerformanceMonitor.Darling.Service/DarlingStoreUpgrade.cs b/Darling/PerformanceMonitor.Darling.Service/DarlingStoreUpgrade.cs index 75b57a25c..76536cb58 100644 --- a/Darling/PerformanceMonitor.Darling.Service/DarlingStoreUpgrade.cs +++ b/Darling/PerformanceMonitor.Darling.Service/DarlingStoreUpgrade.cs @@ -183,6 +183,15 @@ can lie. DarlingStoreUpgradeSiblingNamesTests pins the pair against the source. public DarlingStoreUpgrade(ILogger logger) => _logger = logger ?? throw new ArgumentNullException(nameof(logger)); + /// The wait between attempts of a runtime-rescue step. Tests replace it so a retry does not sleep. + internal Func RetryDelay { get; set; } = Task.Delay; + + /// The rename that rescues the live runtime. Tests replace it to stand in for a file lock. + internal Action MoveRuntimeDirectory { get; set; } = Directory.Move; + + /// The step that clears the last update's rescued runtime. Tests replace it to stand in for a file lock. + internal Action ClearPreviousRuntime { get; set; } = EmptyDirectory; + /// /// The pg_ctl --version probe behind every runtime-major read that decides whether a runtime /// directory is kept, swapped or used for an upgrade. An instance member so a test with no binaries to @@ -1268,7 +1277,7 @@ just as surely as an unstamped one did on DARLING01. */ try { - EmptyDirectory(previousRoot); + ClearPreviousRuntime(previousRoot); } catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) { @@ -1287,7 +1296,7 @@ just as surely as an unstamped one did on DARLING01. */ try { - Directory.Move(pgsqlDirectory, previousPgsql); + MoveRuntimeDirectory(pgsqlDirectory, previousPgsql); } catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) { From 935522c9be086853ce6bb82fe0514dc299ed9714 Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Thu, 1 Oct 2026 23:11:04 -0400 Subject: [PATCH 02/11] Darling: the runtime rescue retries a briefly locked folder for a few seconds before it defers the update --- .../DarlingStoreUpgrade.cs | 47 ++++++++++++++++++- 1 file changed, 45 insertions(+), 2 deletions(-) diff --git a/Darling/PerformanceMonitor.Darling.Service/DarlingStoreUpgrade.cs b/Darling/PerformanceMonitor.Darling.Service/DarlingStoreUpgrade.cs index 76536cb58..e26ebe84e 100644 --- a/Darling/PerformanceMonitor.Darling.Service/DarlingStoreUpgrade.cs +++ b/Darling/PerformanceMonitor.Darling.Service/DarlingStoreUpgrade.cs @@ -192,6 +192,39 @@ public DarlingStoreUpgrade(ILogger logger) /// The step that clears the last update's rescued runtime. Tests replace it to stand in for a file lock. internal Action ClearPreviousRuntime { get; set; } = EmptyDirectory; + /// + /// Just after the store stops, an antivirus scan or the exiting server can hold the runtime folder for a + /// moment. Runs up to .Length + 1 + /// times, waiting between attempts, and retries only and + /// . The last failure is rethrown for the caller's existing + /// "defer the update" handling; a cancelled wait propagates as the cancellation. + /// + private async Task RetryTransientIoAsync(Action operation, string what, CancellationToken cancellationToken) + { + for (var attempt = 1; ; attempt++) + { + try + { + operation(); + return; + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException + && attempt <= s_runtimeRescueRetryDelays.Length) + { + var delay = s_runtimeRescueRetryDelays[attempt - 1]; + _logger.LogInformation( + "Retrying {What} in {Delay} after attempt {Attempt} failed ({Message}).", + what, delay, attempt, ex.Message); + await RetryDelay(delay, cancellationToken); + } + } + } + + private static readonly TimeSpan[] s_runtimeRescueRetryDelays = + [ + TimeSpan.FromSeconds(0.5), TimeSpan.FromSeconds(1), TimeSpan.FromSeconds(2), TimeSpan.FromSeconds(2), + ]; + /// /// The pg_ctl --version probe behind every runtime-major read that decides whether a runtime /// directory is kept, swapped or used for an upgrade. An instance member so a test with no binaries to @@ -1277,7 +1310,12 @@ just as surely as an unstamped one did on DARLING01. */ try { - ClearPreviousRuntime(previousRoot); + /* EmptyDirectory only runs here once the rescued-runtime guard above has decided the folder may be + cleared, and clearing is idempotent, so running it again after a partial pass is safe. */ + await RetryTransientIoAsync( + () => ClearPreviousRuntime(previousRoot), + $"clearing the previous runtime at {previousRoot}", + cancellationToken); } catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) { @@ -1296,7 +1334,12 @@ just as surely as an unstamped one did on DARLING01. */ try { - MoveRuntimeDirectory(pgsqlDirectory, previousPgsql); + /* A failed Directory.Move leaves the source intact (a same-volume rename is one operation), so + trying it again after a lock clears is safe. */ + await RetryTransientIoAsync( + () => MoveRuntimeDirectory(pgsqlDirectory, previousPgsql), + $"rescuing the current runtime to {previousPgsql}", + cancellationToken); } catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) { From 6837f1e98048df9287e2626858f4f391f1847dc7 Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Thu, 1 Oct 2026 23:12:48 -0400 Subject: [PATCH 03/11] Tests: the downgrade-guard ordering pin follows the rescue's move seam --- Darling/Darling.Tests/DarlingStoreUpgradeTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Darling/Darling.Tests/DarlingStoreUpgradeTests.cs b/Darling/Darling.Tests/DarlingStoreUpgradeTests.cs index 6849d156e..a6534a6ec 100644 --- a/Darling/Darling.Tests/DarlingStoreUpgradeTests.cs +++ b/Darling/Darling.Tests/DarlingStoreUpgradeTests.cs @@ -2491,7 +2491,7 @@ which after this release is all of them. */ var call = source.IndexOf("IsDowngradeAgainstStore(dataDirectory, runtimeZipPath)", StringComparison.Ordinal); Assert.True(call >= 0, "nothing calls IsDowngradeAgainstStore — a correct downgrade check that is never invoked is what #1738 already was"); - var rescue = source.IndexOf("Directory.Move(pgsqlDirectory, previousPgsql)", StringComparison.Ordinal); + var rescue = source.IndexOf("MoveRuntimeDirectory(pgsqlDirectory, previousPgsql)", StringComparison.Ordinal); Assert.True(rescue > call, "the downgrade guard must run BEFORE the runtime is rescued and replaced"); var noStampBranch = source.IndexOf("if (stamp is null)", StringComparison.Ordinal); From 43c4ce2049c45011675d989cb647650cb91d46ad Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Thu, 1 Oct 2026 23:12:48 -0400 Subject: [PATCH 04/11] Tests: the downgrade-guard ordering pin follows the rescue's move seam --- Darling/Darling.Tests/DarlingStoreUpgradeTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Darling/Darling.Tests/DarlingStoreUpgradeTests.cs b/Darling/Darling.Tests/DarlingStoreUpgradeTests.cs index 6849d156e..a6534a6ec 100644 --- a/Darling/Darling.Tests/DarlingStoreUpgradeTests.cs +++ b/Darling/Darling.Tests/DarlingStoreUpgradeTests.cs @@ -2491,7 +2491,7 @@ which after this release is all of them. */ var call = source.IndexOf("IsDowngradeAgainstStore(dataDirectory, runtimeZipPath)", StringComparison.Ordinal); Assert.True(call >= 0, "nothing calls IsDowngradeAgainstStore — a correct downgrade check that is never invoked is what #1738 already was"); - var rescue = source.IndexOf("Directory.Move(pgsqlDirectory, previousPgsql)", StringComparison.Ordinal); + var rescue = source.IndexOf("MoveRuntimeDirectory(pgsqlDirectory, previousPgsql)", StringComparison.Ordinal); Assert.True(rescue > call, "the downgrade guard must run BEFORE the runtime is rescued and replaced"); var noStampBranch = source.IndexOf("if (stamp is null)", StringComparison.Ordinal); From 2f5f449e8e2575bb5a2f062d654e627c2922023a Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Thu, 1 Oct 2026 23:37:24 -0400 Subject: [PATCH 05/11] Tests and logging: the runtime rescue's retry line reads plainly, and the deferral pins don't sleep --- .../Darling.Tests/DarlingStoreUpgradeTests.cs | 17 ++++++++++++----- .../DarlingStoreUpgrade.cs | 8 ++++---- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/Darling/Darling.Tests/DarlingStoreUpgradeTests.cs b/Darling/Darling.Tests/DarlingStoreUpgradeTests.cs index a6534a6ec..4b8bd6f31 100644 --- a/Darling/Darling.Tests/DarlingStoreUpgradeTests.cs +++ b/Darling/Darling.Tests/DarlingStoreUpgradeTests.cs @@ -2714,13 +2714,15 @@ public async Task RuntimeAdvance_APreviousRuntimeFolderItCannotCreate_DefersTheS File.WriteAllText(previousRoot, "a file where the folder should be"); var log = new CapturingLogger(); - var advance = await new DarlingStoreUpgrade(log).TryAdvanceRuntimeAsync( + /* The folder never becomes creatable, so the whole retry budget is spent; do not sleep through it. */ + var advance = await new DarlingStoreUpgrade(log) { RetryDelay = (_, _) => Task.CompletedTask }.TryAdvanceRuntimeAsync( host.RuntimeRoot, host.Package, host.DataDirectory, (_, _) => Task.FromResult(false), TestContext.Current.CancellationToken); var warning = AssertSwapDeferred(advance, host, log); Assert.Contains(previousRoot, warning, StringComparison.Ordinal); + Assert.Equal(4, CountRetryLines(log)); /* Nothing is deleted to make room: the file is not the service's to remove. */ Assert.Equal("a file where the folder should be", File.ReadAllText(previousRoot)); @@ -2988,6 +2990,7 @@ await Assert.ThrowsAnyAsync(() => upgrade.TryAdvance cts.Token)); Assert.False(HasWarning(log)); + Assert.Equal(HostAwaitingARuntimeSwap.LiveRuntime, File.ReadAllText(host.PgCtl)); Assert.Equal(HostAwaitingARuntimeSwap.PriorStamp, File.ReadAllText(host.StampPath).Trim()); } finally @@ -3005,11 +3008,12 @@ public async Task RuntimeAdvance_ALiveRuntimeHeldForAMoment_IsRescuedOnceTheLock { Assert.SkipUnless(OperatingSystem.IsWindows(), "File locks block rename and delete on Windows only."); var root = Directory.CreateTempSubdirectory("darling-move-held-"); + FileStream? hold = null; try { var host = PlantHostAwaitingARuntimeSwap(root.FullName); - var hold = new FileStream(host.PgCtl, FileMode.Open, FileAccess.Read, FileShare.None); - using var release = new Timer(_ => hold.Dispose(), null, TimeSpan.FromSeconds(1), Timeout.InfiniteTimeSpan); + var held = hold = new FileStream(host.PgCtl, FileMode.Open, FileAccess.Read, FileShare.None); + using var release = new Timer(_ => held.Dispose(), null, TimeSpan.FromSeconds(1), Timeout.InfiniteTimeSpan); var advance = await new DarlingStoreUpgrade(new CapturingLogger()).TryAdvanceRuntimeAsync( host.RuntimeRoot, host.Package, host.DataDirectory, @@ -3022,6 +3026,7 @@ public async Task RuntimeAdvance_ALiveRuntimeHeldForAMoment_IsRescuedOnceTheLock } finally { + hold?.Dispose(); TryDeleteTree(root.FullName); } } @@ -3032,6 +3037,7 @@ public async Task RuntimeAdvance_APreviousRuntimeHeldForAMoment_IsClearedOnceThe { Assert.SkipUnless(OperatingSystem.IsWindows(), "File locks block rename and delete on Windows only."); var root = Directory.CreateTempSubdirectory("darling-prev-held-"); + FileStream? hold = null; try { var host = PlantHostAwaitingARuntimeSwap(root.FullName); @@ -3039,8 +3045,8 @@ public async Task RuntimeAdvance_APreviousRuntimeHeldForAMoment_IsClearedOnceThe var heldFile = Path.Combine(previousRoot, "pgsql", "bin", "postgres.exe"); Directory.CreateDirectory(Path.GetDirectoryName(heldFile)!); File.WriteAllText(heldFile, "previous runtime"); - var hold = new FileStream(heldFile, FileMode.Open, FileAccess.Read, FileShare.None); - using var release = new Timer(_ => hold.Dispose(), null, TimeSpan.FromSeconds(1), Timeout.InfiniteTimeSpan); + var held = hold = new FileStream(heldFile, FileMode.Open, FileAccess.Read, FileShare.None); + using var release = new Timer(_ => held.Dispose(), null, TimeSpan.FromSeconds(1), Timeout.InfiniteTimeSpan); var advance = await new DarlingStoreUpgrade(new CapturingLogger()).TryAdvanceRuntimeAsync( host.RuntimeRoot, host.Package, host.DataDirectory, @@ -3053,6 +3059,7 @@ public async Task RuntimeAdvance_APreviousRuntimeHeldForAMoment_IsClearedOnceThe } finally { + hold?.Dispose(); TryDeleteTree(root.FullName); } } diff --git a/Darling/PerformanceMonitor.Darling.Service/DarlingStoreUpgrade.cs b/Darling/PerformanceMonitor.Darling.Service/DarlingStoreUpgrade.cs index e26ebe84e..445721db9 100644 --- a/Darling/PerformanceMonitor.Darling.Service/DarlingStoreUpgrade.cs +++ b/Darling/PerformanceMonitor.Darling.Service/DarlingStoreUpgrade.cs @@ -213,8 +213,8 @@ private async Task RetryTransientIoAsync(Action operation, string what, Cancella { var delay = s_runtimeRescueRetryDelays[attempt - 1]; _logger.LogInformation( - "Retrying {What} in {Delay} after attempt {Attempt} failed ({Message}).", - what, delay, attempt, ex.Message); + "Retrying {What} in {DelaySeconds} s after attempt {Attempt} failed ({Message}).", + what, delay.TotalSeconds, attempt, ex.Message); await RetryDelay(delay, cancellationToken); } } @@ -1314,7 +1314,7 @@ just as surely as an unstamped one did on DARLING01. */ cleared, and clearing is idempotent, so running it again after a partial pass is safe. */ await RetryTransientIoAsync( () => ClearPreviousRuntime(previousRoot), - $"clearing the previous runtime at {previousRoot}", + $"the clear of the previous runtime at {previousRoot}", cancellationToken); } catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) @@ -1338,7 +1338,7 @@ await RetryTransientIoAsync( trying it again after a lock clears is safe. */ await RetryTransientIoAsync( () => MoveRuntimeDirectory(pgsqlDirectory, previousPgsql), - $"rescuing the current runtime to {previousPgsql}", + $"the rescue of the current runtime to {previousPgsql}", cancellationToken); } catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) From 7d4296b26099921a67c660be8cebcceb1f1a6241 Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Thu, 1 Oct 2026 23:39:25 -0400 Subject: [PATCH 06/11] Darling: the runtime update's revert after a failed extract retries a briefly locked folder too --- .../Darling.Tests/DarlingStoreUpgradeTests.cs | 52 +++++++++++++++++++ .../DarlingStoreUpgrade.cs | 16 +++++- 2 files changed, 66 insertions(+), 2 deletions(-) diff --git a/Darling/Darling.Tests/DarlingStoreUpgradeTests.cs b/Darling/Darling.Tests/DarlingStoreUpgradeTests.cs index 4b8bd6f31..022e4a5c7 100644 --- a/Darling/Darling.Tests/DarlingStoreUpgradeTests.cs +++ b/Darling/Darling.Tests/DarlingStoreUpgradeTests.cs @@ -2999,6 +2999,58 @@ await Assert.ThrowsAnyAsync(() => upgrade.TryAdvance } } + /// + /// A package whose runtime fails to extract (no pgsql\bin\pg_ctl.exe in it) sends the update to + /// its revert. The restore of the previous runtime hits a briefly locked folder twice, and the revert + /// retries it: the original extract failure is what surfaces, with the live runtime back at pgsql. + /// + [Fact] + public async Task RuntimeAdvance_AFailedExtractWhoseRestoreIsLockedTwice_RetriesAndRestoresTheRuntime() + { + var root = Directory.CreateTempSubdirectory("darling-revert-retry-"); + try + { + var host = PlantHostAwaitingARuntimeSwap(root.FullName); + + var brokenSource = Path.Combine(root.FullName, "broken", "pgsql"); + Directory.CreateDirectory(Path.Combine(brokenSource, "bin")); + File.WriteAllText(Path.Combine(brokenSource, "bin", "postgres.exe"), "a package with no pg_ctl"); + File.Delete(host.Package); + ZipFile.CreateFromDirectory(brokenSource, host.Package, CompressionLevel.NoCompression, includeBaseDirectory: true); + + var previousPgsql = Path.Combine(DarlingStoreUpgrade.PreviousRuntimeRootFor(host.RuntimeRoot), "pgsql"); + var log = new CapturingLogger(); + var restoreAttempts = 0; + var upgrade = new DarlingStoreUpgrade(log) + { + RetryDelay = (_, _) => Task.CompletedTask, + MoveRuntimeDirectory = (from, to) => + { + if (string.Equals(from, previousPgsql, StringComparison.OrdinalIgnoreCase) && ++restoreAttempts <= 2) + { + throw new IOException("The process cannot access the file because it is being used by another process."); + } + + Directory.Move(from, to); + }, + }; + + await Assert.ThrowsAsync(() => upgrade.TryAdvanceRuntimeAsync( + host.RuntimeRoot, host.Package, host.DataDirectory, + (_, _) => Task.FromResult(false), + TestContext.Current.CancellationToken)); + + Assert.Equal(3, restoreAttempts); + Assert.Equal(2, CountRetryLines(log)); + Assert.Equal(HostAwaitingARuntimeSwap.LiveRuntime, File.ReadAllText(host.PgCtl)); + Assert.Equal(HostAwaitingARuntimeSwap.PriorStamp, File.ReadAllText(host.StampPath).Trim()); + } + finally + { + TryDeleteTree(root.FullName); + } + } + /// /// The real lock, on the platform where one blocks a rename: a file under the live runtime is held for /// about a second, which is longer than the first attempt and shorter than the retry budget. diff --git a/Darling/PerformanceMonitor.Darling.Service/DarlingStoreUpgrade.cs b/Darling/PerformanceMonitor.Darling.Service/DarlingStoreUpgrade.cs index 445721db9..457a39569 100644 --- a/Darling/PerformanceMonitor.Darling.Service/DarlingStoreUpgrade.cs +++ b/Darling/PerformanceMonitor.Darling.Service/DarlingStoreUpgrade.cs @@ -1375,12 +1375,24 @@ await Task.Run( would leave an unbootable runtime behind. */ var failedExtract = pgsqlDirectory + ".failed"; TryDeleteDirectory(failedExtract); + + /* Both moves retry a briefly locked folder like the rescue does: a same-volume rename is atomic, + so trying again is safe, and a lock here would otherwise leave no runtime at pgsql. The waits + use CancellationToken.None: if the extract failed because the update was cancelled, the revert + still has to finish to leave a bootable runtime, and a cancelled wait would throw a new + OperationCanceledException in place of the original failure. */ if (Directory.Exists(pgsqlDirectory)) { - Directory.Move(pgsqlDirectory, failedExtract); + await RetryTransientIoAsync( + () => MoveRuntimeDirectory(pgsqlDirectory, failedExtract), + $"the move aside of the failed extract at {pgsqlDirectory}", + CancellationToken.None); } - Directory.Move(previousPgsql, pgsqlDirectory); + await RetryTransientIoAsync( + () => MoveRuntimeDirectory(previousPgsql, pgsqlDirectory), + $"the restore of the previous runtime to {pgsqlDirectory}", + CancellationToken.None); TryDeleteDirectory(failedExtract); TryEmptyDirectory(previousRoot); throw; From 0795b2b1da0b022ba192150c3ee6adade1775b6f Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Thu, 1 Oct 2026 23:45:49 -0400 Subject: [PATCH 07/11] Darling: a start that finds the runtime missing after an interrupted update puts back the runtime that last opened the store --- .../Darling.Tests/DarlingStoreUpgradeTests.cs | 230 ++++++++++++++++++ .../DarlingManagedPostgres.cs | 6 + .../DarlingStoreUpgrade.cs | 58 +++++ 3 files changed, 294 insertions(+) diff --git a/Darling/Darling.Tests/DarlingStoreUpgradeTests.cs b/Darling/Darling.Tests/DarlingStoreUpgradeTests.cs index 022e4a5c7..fd66981c4 100644 --- a/Darling/Darling.Tests/DarlingStoreUpgradeTests.cs +++ b/Darling/Darling.Tests/DarlingStoreUpgradeTests.cs @@ -3051,6 +3051,236 @@ await Assert.ThrowsAsync(() => upgrade.TryAdvanceRunt } } + /* ================================================================================== + #4934: a start that finds no runtime at pgsql, but the store's own runtime rescued, puts it back. + ================================================================================== */ + + private sealed record RestoreHost( + string Root, string RuntimeRoot, string Pgsql, string PreviousPgsql, string PreviousBin, string DataDirectory); + + /// A deploy folder with a store on (null: no store) and nothing else yet. + private static RestoreHost PlantRestoreHost(string root, string? storeMajor) + { + var runtimeRoot = Path.Combine(root, "deploy", "pg-runtime"); + var previousPgsql = Path.Combine(DarlingStoreUpgrade.PreviousRuntimeRootFor(runtimeRoot), "pgsql"); + var dataDirectory = Path.Combine(root, "pg"); + Directory.CreateDirectory(runtimeRoot); + Directory.CreateDirectory(dataDirectory); + if (storeMajor is not null) + { + File.WriteAllText(Path.Combine(dataDirectory, "PG_VERSION"), storeMajor + "\n"); + } + + return new RestoreHost( + root, runtimeRoot, Path.Combine(runtimeRoot, "pgsql"), previousPgsql, + Path.Combine(previousPgsql, "bin"), dataDirectory); + } + + private static int CountWarnings(CapturingLogger log) + => log.ToString().Split(Environment.NewLine).Count(line => line.StartsWith("[Warning]", StringComparison.Ordinal)); + + [Fact] + public async Task RestoreRescuedRuntime_AnEmptyPgsqlAndTheStoresRescuedRuntime_PutsTheRuntimeBack() + { + var root = Directory.CreateTempSubdirectory("darling-restore-empty-"); + try + { + var host = PlantRestoreHost(root.FullName, "17"); + Directory.CreateDirectory(host.Pgsql); + PlantRuntime(host.PreviousPgsql, "the-stores-own"); + var log = new CapturingLogger(); + var upgrade = new DarlingStoreUpgrade(log) + { + ReadRuntimeVersionLine = VersionsByBin((host.PreviousBin, "pg_ctl (PostgreSQL) 17.6")), + }; + + Assert.True(await upgrade.TryRestoreRescuedRuntimeAsync(host.RuntimeRoot, host.DataDirectory, TestContext.Current.CancellationToken)); + + Assert.Equal("the-stores-own", File.ReadAllText(Path.Combine(host.Pgsql, "bin", "runtime.txt"))); + Assert.True(File.Exists(Path.Combine(host.Pgsql, "bin", "pg_ctl.exe"))); + Assert.False(Directory.Exists(host.PreviousPgsql)); + Assert.Equal(1, CountWarnings(log)); + } + finally + { + TryDeleteTree(root.FullName); + } + } + + [Fact] + public async Task RestoreRescuedRuntime_APartialPgsql_IsMovedAsideAndDeleted_AndTheRuntimeComesBack() + { + var root = Directory.CreateTempSubdirectory("darling-restore-partial-"); + try + { + var host = PlantRestoreHost(root.FullName, "17"); + Directory.CreateDirectory(Path.Combine(host.Pgsql, "lib")); + File.WriteAllText(Path.Combine(host.Pgsql, "lib", "half-extracted.dll"), "partial"); + PlantRuntime(host.PreviousPgsql, "the-stores-own"); + var upgrade = new DarlingStoreUpgrade(new CapturingLogger()) + { + ReadRuntimeVersionLine = VersionsByBin((host.PreviousBin, "pg_ctl (PostgreSQL) 17.6")), + }; + + Assert.True(await upgrade.TryRestoreRescuedRuntimeAsync(host.RuntimeRoot, host.DataDirectory, TestContext.Current.CancellationToken)); + + Assert.Equal("the-stores-own", File.ReadAllText(Path.Combine(host.Pgsql, "bin", "runtime.txt"))); + Assert.False(File.Exists(Path.Combine(host.Pgsql, "lib", "half-extracted.dll"))); + Assert.False(Directory.Exists(host.Pgsql + ".failed")); + } + finally + { + TryDeleteTree(root.FullName); + } + } + + [Fact] + public async Task RestoreRescuedRuntime_NoStore_MovesNothing() + { + var root = Directory.CreateTempSubdirectory("darling-restore-nostore-"); + try + { + var host = PlantRestoreHost(root.FullName, null); + PlantRuntime(host.PreviousPgsql, "rescued"); + var upgrade = new DarlingStoreUpgrade(new CapturingLogger()) + { + ReadRuntimeVersionLine = VersionsByBin((host.PreviousBin, "pg_ctl (PostgreSQL) 17.6")), + }; + + Assert.False(await upgrade.TryRestoreRescuedRuntimeAsync(host.RuntimeRoot, host.DataDirectory, TestContext.Current.CancellationToken)); + + Assert.False(Directory.Exists(host.Pgsql)); + Assert.True(File.Exists(Path.Combine(host.PreviousBin, "pg_ctl.exe"))); + } + finally + { + TryDeleteTree(root.FullName); + } + } + + [Fact] + public async Task RestoreRescuedRuntime_AStoreWithNoRescuedRuntime_MovesNothing() + { + var root = Directory.CreateTempSubdirectory("darling-restore-noprev-"); + try + { + var host = PlantRestoreHost(root.FullName, "17"); + var upgrade = new DarlingStoreUpgrade(new CapturingLogger()) + { + ReadRuntimeVersionLine = VersionsByBin((host.PreviousBin, "pg_ctl (PostgreSQL) 17.6")), + }; + + Assert.False(await upgrade.TryRestoreRescuedRuntimeAsync(host.RuntimeRoot, host.DataDirectory, TestContext.Current.CancellationToken)); + + Assert.False(Directory.Exists(host.Pgsql)); + Assert.False(Directory.Exists(host.PreviousPgsql)); + } + finally + { + TryDeleteTree(root.FullName); + } + } + + [Fact] + public async Task RestoreRescuedRuntime_ARescuedRuntimeOfAnotherMajor_MovesNothing() + { + var root = Directory.CreateTempSubdirectory("darling-restore-major-"); + try + { + var host = PlantRestoreHost(root.FullName, "17"); + PlantRuntime(host.PreviousPgsql, "an-eighteen"); + var upgrade = new DarlingStoreUpgrade(new CapturingLogger()) + { + ReadRuntimeVersionLine = VersionsByBin((host.PreviousBin, "pg_ctl (PostgreSQL) 18.4")), + }; + + Assert.False(await upgrade.TryRestoreRescuedRuntimeAsync(host.RuntimeRoot, host.DataDirectory, TestContext.Current.CancellationToken)); + + Assert.False(Directory.Exists(host.Pgsql)); + Assert.True(File.Exists(Path.Combine(host.PreviousBin, "pg_ctl.exe"))); + } + finally + { + TryDeleteTree(root.FullName); + } + } + + [Fact] + public async Task RestoreRescuedRuntime_ALiveRuntimeInPlace_MovesNothing() + { + var root = Directory.CreateTempSubdirectory("darling-restore-live-"); + try + { + var host = PlantRestoreHost(root.FullName, "17"); + PlantRuntime(host.Pgsql, "live"); + PlantRuntime(host.PreviousPgsql, "rescued"); + var upgrade = new DarlingStoreUpgrade(new CapturingLogger()) + { + ReadRuntimeVersionLine = VersionsByBin((host.PreviousBin, "pg_ctl (PostgreSQL) 17.6")), + }; + + Assert.False(await upgrade.TryRestoreRescuedRuntimeAsync(host.RuntimeRoot, host.DataDirectory, TestContext.Current.CancellationToken)); + + Assert.Equal("live", File.ReadAllText(Path.Combine(host.Pgsql, "bin", "runtime.txt"))); + Assert.Equal("rescued", File.ReadAllText(Path.Combine(host.PreviousBin, "runtime.txt"))); + } + finally + { + TryDeleteTree(root.FullName); + } + } + + [Fact] + public async Task RestoreRescuedRuntime_AMoveLockedTwice_RetriesAndRestores() + { + var root = Directory.CreateTempSubdirectory("darling-restore-retry-"); + try + { + var host = PlantRestoreHost(root.FullName, "17"); + PlantRuntime(host.PreviousPgsql, "the-stores-own"); + var log = new CapturingLogger(); + var attempts = 0; + var upgrade = new DarlingStoreUpgrade(log) + { + RetryDelay = (_, _) => Task.CompletedTask, + ReadRuntimeVersionLine = VersionsByBin((host.PreviousBin, "pg_ctl (PostgreSQL) 17.6")), + MoveRuntimeDirectory = (from, to) => + { + if (string.Equals(from, host.PreviousPgsql, StringComparison.OrdinalIgnoreCase) && ++attempts <= 2) + { + throw new IOException("The process cannot access the file because it is being used by another process."); + } + + Directory.Move(from, to); + }, + }; + + Assert.True(await upgrade.TryRestoreRescuedRuntimeAsync(host.RuntimeRoot, host.DataDirectory, TestContext.Current.CancellationToken)); + + Assert.Equal(3, attempts); + Assert.Equal(2, CountRetryLines(log)); + Assert.Equal("the-stores-own", File.ReadAllText(Path.Combine(host.Pgsql, "bin", "runtime.txt"))); + } + finally + { + TryDeleteTree(root.FullName); + } + } + + [Fact] + public void EnsureRuntime_RestoresTheRescuedRuntime_BeforeItLooksForPgCtl() + { + var source = RepoFile.ReadRepoFile("Darling", "PerformanceMonitor.Darling.Service", "DarlingManagedPostgres.cs"); + var start = source.IndexOf("private async Task EnsureRuntimeAsync(", StringComparison.Ordinal); + Assert.True(start >= 0, "EnsureRuntimeAsync must exist"); + var body = source[start..]; + var restore = body.IndexOf("_storeUpgrade.TryRestoreRescuedRuntimeAsync(_runtimeRoot, _dataDirectory, cancellationToken)", StringComparison.Ordinal); + var existsCheck = body.IndexOf("if (File.Exists(pgCtl))", StringComparison.Ordinal); + Assert.True(restore >= 0, "EnsureRuntimeAsync must try to restore the rescued runtime"); + Assert.True(existsCheck >= 0, "EnsureRuntimeAsync must test for pg_ctl.exe"); + Assert.True(restore < existsCheck, "the restore must run BEFORE the pg_ctl.exe test, so a restored runtime takes the normal path and not the first-run extract"); + } + /// /// The real lock, on the platform where one blocks a rename: a file under the live runtime is held for /// about a second, which is longer than the first attempt and shorter than the retry budget. diff --git a/Darling/PerformanceMonitor.Darling.Service/DarlingManagedPostgres.cs b/Darling/PerformanceMonitor.Darling.Service/DarlingManagedPostgres.cs index 9bd4fa2a9..b3249d7d7 100644 --- a/Darling/PerformanceMonitor.Darling.Service/DarlingManagedPostgres.cs +++ b/Darling/PerformanceMonitor.Darling.Service/DarlingManagedPostgres.cs @@ -3335,6 +3335,12 @@ private async Task EnsureRuntimeAsync(CancellationToken cancellationToke var pgsqlDirectory = Path.Combine(_runtimeRoot, "pgsql"); var binDirectory = Path.Combine(pgsqlDirectory, "bin"); var pgCtl = Path.Combine(binDirectory, "pg_ctl.exe"); + + /* #4934: a runtime update that died between the rescue and a good extract leaves no pg_ctl.exe here + and the store's own runtime in pg-runtime-prev. Put it back first, so the branch below takes the + normal path (and retries the update) instead of the first-run extract. */ + await _storeUpgrade.TryRestoreRescuedRuntimeAsync(_runtimeRoot, _dataDirectory, cancellationToken); + if (File.Exists(pgCtl)) { /* #1706: an extracted runtime is NOT refreshed by a deploy — this early return is exactly why a diff --git a/Darling/PerformanceMonitor.Darling.Service/DarlingStoreUpgrade.cs b/Darling/PerformanceMonitor.Darling.Service/DarlingStoreUpgrade.cs index 457a39569..5c9f7f2e0 100644 --- a/Darling/PerformanceMonitor.Darling.Service/DarlingStoreUpgrade.cs +++ b/Darling/PerformanceMonitor.Darling.Service/DarlingStoreUpgrade.cs @@ -1068,6 +1068,64 @@ internal sealed record RuntimeAdvance(bool Swapped, string? PreviousBinDirectory return previousMajor == storeMajor ? previousBin : null; } + /// + /// Puts the store's own rescued runtime back at pgsql when the live runtime folder holds no + /// bin\pg_ctl.exe and the rescued copy under is the one that + /// last opened the store. Returns true when it moved the runtime back; false, having changed nothing, + /// in every other case (a live runtime is there, there is no store, no rescued copy, or the rescued copy + /// is another major than the store's, which already refuses). + /// + /// The shape: a runtime update moved the live runtime aside and its extract never finished (the + /// process died, or an antivirus scan held the folder), leaving an empty or partial pgsql with the + /// good runtime in pg-runtime-prev. Without this, the next start saw no pg_ctl.exe, took the + /// first-run branch and extracted the package as if there were no store. + /// + /// A move that still fails after its retries throws, as the first-run branch does: there is no + /// runtime to start on, and no fallback state is invented for it. + /// + internal async Task TryRestoreRescuedRuntimeAsync(string runtimeRoot, string dataDirectory, CancellationToken cancellationToken) + { + var pgsqlDirectory = Path.Combine(runtimeRoot, "pgsql"); + if (File.Exists(Path.Combine(pgsqlDirectory, "bin", "pg_ctl.exe"))) + { + return false; + } + + if (await FindRescuedRuntimeBinAsync(runtimeRoot, dataDirectory, cancellationToken) is null) + { + return false; + } + + var previousPgsql = Path.Combine(PreviousRuntimeRootFor(runtimeRoot), "pgsql"); + + /* A partial pgsql goes aside before the restore, exactly as the extract-failure revert does it: a + move is one operation, a recursive delete is not, and a half-deleted folder is no runtime. */ + var failedExtract = pgsqlDirectory + ".failed"; + TryDeleteDirectory(failedExtract); + if (Directory.Exists(pgsqlDirectory)) + { + await RetryTransientIoAsync( + () => MoveRuntimeDirectory(pgsqlDirectory, failedExtract), + $"the move aside of the incomplete runtime at {pgsqlDirectory}", + cancellationToken); + } + + await RetryTransientIoAsync( + () => MoveRuntimeDirectory(previousPgsql, pgsqlDirectory), + $"the restore of the rescued runtime to {pgsqlDirectory}", + cancellationToken); + TryDeleteDirectory(failedExtract); + + /* The stamp still names the runtime that was live before the interrupted update: the advance writes it + only after a good extract (File.WriteAllText(stampPath, zipHash) in TryAdvanceRuntimeAsync). So the + normal path that follows compares the package against that stamp, sees the difference, and retries + the update. */ + _logger.LogWarning( + "The Postgres runtime at {Runtime} had no pg_ctl.exe, and the runtime that last opened the store at {DataDirectory} was found at {Previous}. It was put back, and the runtime update is retried on this start.", + pgsqlDirectory, dataDirectory, previousPgsql); + return true; + } + /// /// Whether an unidentifiable runtime must STOP the service rather than be waved through, PURE so the /// decision is pinned without needing a broken runtime to reproduce (deleting the guard inline left the From 1a35b7868ee009f8d6d9e511dd9a24dc9b603644 Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Fri, 2 Oct 2026 00:10:13 -0400 Subject: [PATCH 08/11] Darling: a failed runtime extract is logged before its revert, and the revert's move aside is pinned --- .../Darling.Tests/DarlingStoreUpgradeTests.cs | 54 +++++++++++++++++++ .../DarlingStoreUpgrade.cs | 8 ++- 2 files changed, 60 insertions(+), 2 deletions(-) diff --git a/Darling/Darling.Tests/DarlingStoreUpgradeTests.cs b/Darling/Darling.Tests/DarlingStoreUpgradeTests.cs index fd66981c4..97f5aa392 100644 --- a/Darling/Darling.Tests/DarlingStoreUpgradeTests.cs +++ b/Darling/Darling.Tests/DarlingStoreUpgradeTests.cs @@ -3051,6 +3051,60 @@ await Assert.ThrowsAsync(() => upgrade.TryAdvanceRunt } } + /// + /// The same failed extract, with the MOVE ASIDE of the partial runtime locked twice instead: the revert + /// retries it, and the previous runtime is back at pgsql afterwards. + /// + [Fact] + public async Task RuntimeAdvance_AFailedExtractWhoseMoveAsideIsLockedTwice_RetriesAndRestoresTheRuntime() + { + var root = Directory.CreateTempSubdirectory("darling-revert-aside-"); + try + { + var host = PlantHostAwaitingARuntimeSwap(root.FullName); + + var brokenSource = Path.Combine(root.FullName, "broken", "pgsql"); + Directory.CreateDirectory(Path.Combine(brokenSource, "bin")); + File.WriteAllText(Path.Combine(brokenSource, "bin", "postgres.exe"), "a package with no pg_ctl"); + File.Delete(host.Package); + ZipFile.CreateFromDirectory(brokenSource, host.Package, CompressionLevel.NoCompression, includeBaseDirectory: true); + + var pgsqlDirectory = Path.Combine(host.RuntimeRoot, "pgsql"); + var log = new CapturingLogger(); + var moveAsideAttempts = 0; + var upgrade = new DarlingStoreUpgrade(log) + { + RetryDelay = (_, _) => Task.CompletedTask, + MoveRuntimeDirectory = (from, to) => + { + if (string.Equals(from, pgsqlDirectory, StringComparison.OrdinalIgnoreCase) + && to.EndsWith(".failed", StringComparison.OrdinalIgnoreCase) + && ++moveAsideAttempts <= 2) + { + throw new IOException("The process cannot access the file because it is being used by another process."); + } + + Directory.Move(from, to); + }, + }; + + await Assert.ThrowsAsync(() => upgrade.TryAdvanceRuntimeAsync( + host.RuntimeRoot, host.Package, host.DataDirectory, + (_, _) => Task.FromResult(false), + TestContext.Current.CancellationToken)); + + Assert.Equal(3, moveAsideAttempts); + Assert.Equal(2, CountRetryLines(log)); + Assert.Contains("could not be extracted", log.ToString()); + Assert.Equal(HostAwaitingARuntimeSwap.LiveRuntime, File.ReadAllText(host.PgCtl)); + Assert.Equal(HostAwaitingARuntimeSwap.PriorStamp, File.ReadAllText(host.StampPath).Trim()); + } + finally + { + TryDeleteTree(root.FullName); + } + } + /* ================================================================================== #4934: a start that finds no runtime at pgsql, but the store's own runtime rescued, puts it back. ================================================================================== */ diff --git a/Darling/PerformanceMonitor.Darling.Service/DarlingStoreUpgrade.cs b/Darling/PerformanceMonitor.Darling.Service/DarlingStoreUpgrade.cs index 5c9f7f2e0..c92067372 100644 --- a/Darling/PerformanceMonitor.Darling.Service/DarlingStoreUpgrade.cs +++ b/Darling/PerformanceMonitor.Darling.Service/DarlingStoreUpgrade.cs @@ -1425,12 +1425,16 @@ await Task.Run( $"Extracted {runtimeZipPath} but {binDirectory}\\pg_ctl.exe is missing — the archive does not contain pgsql\\bin."); } } - catch (Exception) + catch (Exception extractFailure) { /* The new runtime is not usable; put the old one back so the store still boots, and let the caller's existing error path report. Nothing has touched the data directory yet. Move-aside rather than delete-first, for the reason spelled out in RevertRuntime: a partial delete - would leave an unbootable runtime behind. */ + would leave an unbootable runtime behind. The cause is logged first: a revert move that + still fails after its retries throws in its place, and would hide why the extract failed. */ + _logger.LogWarning( + "The new Postgres runtime from {Package} could not be extracted to {Runtime}: {Reason}. The previous runtime is being put back.", + runtimeZipPath, pgsqlDirectory, extractFailure.Message); var failedExtract = pgsqlDirectory + ".failed"; TryDeleteDirectory(failedExtract); From 795becaeb2d88c975c5f49c1c4a4424602379427 Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Fri, 2 Oct 2026 00:13:30 -0400 Subject: [PATCH 09/11] Darling: the start-time runtime restore fires only for an interrupted update with no server running --- .../Darling.Tests/DarlingStoreUpgradeTests.cs | 211 +++++++++++++++++- .../DarlingManagedPostgres.cs | 2 +- .../DarlingStoreUpgrade.cs | 87 ++++++-- 3 files changed, 271 insertions(+), 29 deletions(-) diff --git a/Darling/Darling.Tests/DarlingStoreUpgradeTests.cs b/Darling/Darling.Tests/DarlingStoreUpgradeTests.cs index 97f5aa392..4342f77ef 100644 --- a/Darling/Darling.Tests/DarlingStoreUpgradeTests.cs +++ b/Darling/Darling.Tests/DarlingStoreUpgradeTests.cs @@ -3110,9 +3110,17 @@ await Assert.ThrowsAsync(() => upgrade.TryAdvanceRunt ================================================================================== */ private sealed record RestoreHost( - string Root, string RuntimeRoot, string Pgsql, string PreviousPgsql, string PreviousBin, string DataDirectory); + string Root, string RuntimeRoot, string Pgsql, string PreviousPgsql, string PreviousBin, string DataDirectory, + string Zip, string StampPath) + { + /// The stamp an interrupted update leaves behind: the package that was live before it, not the shipped one. + public const string InterruptedStamp = "0000000000000000000000000000000000000000000000000000000000000000"; + } - /// A deploy folder with a store on (null: no store) and nothing else yet. + /// + /// A deploy folder with a store on (null: no store), a shipped package, and + /// the stamp of an interrupted update (it differs from the package's hash). + /// private static RestoreHost PlantRestoreHost(string root, string? storeMajor) { var runtimeRoot = Path.Combine(root, "deploy", "pg-runtime"); @@ -3125,9 +3133,14 @@ private static RestoreHost PlantRestoreHost(string root, string? storeMajor) File.WriteAllText(Path.Combine(dataDirectory, "PG_VERSION"), storeMajor + "\n"); } + var zip = Path.Combine(root, "deploy", "pg-runtime.zip"); + File.WriteAllText(zip, "the shipped package"); + var stampPath = Path.Combine(runtimeRoot, DarlingStoreUpgrade.RuntimeStampFileName); + File.WriteAllText(stampPath, RestoreHost.InterruptedStamp); + return new RestoreHost( root, runtimeRoot, Path.Combine(runtimeRoot, "pgsql"), previousPgsql, - Path.Combine(previousPgsql, "bin"), dataDirectory); + Path.Combine(previousPgsql, "bin"), dataDirectory, zip, stampPath); } private static int CountWarnings(CapturingLogger log) @@ -3148,7 +3161,7 @@ public async Task RestoreRescuedRuntime_AnEmptyPgsqlAndTheStoresRescuedRuntime_P ReadRuntimeVersionLine = VersionsByBin((host.PreviousBin, "pg_ctl (PostgreSQL) 17.6")), }; - Assert.True(await upgrade.TryRestoreRescuedRuntimeAsync(host.RuntimeRoot, host.DataDirectory, TestContext.Current.CancellationToken)); + Assert.True(await upgrade.TryRestoreRescuedRuntimeAsync(host.RuntimeRoot, host.Zip, host.DataDirectory, TestContext.Current.CancellationToken)); Assert.Equal("the-stores-own", File.ReadAllText(Path.Combine(host.Pgsql, "bin", "runtime.txt"))); Assert.True(File.Exists(Path.Combine(host.Pgsql, "bin", "pg_ctl.exe"))); @@ -3176,7 +3189,7 @@ public async Task RestoreRescuedRuntime_APartialPgsql_IsMovedAsideAndDeleted_And ReadRuntimeVersionLine = VersionsByBin((host.PreviousBin, "pg_ctl (PostgreSQL) 17.6")), }; - Assert.True(await upgrade.TryRestoreRescuedRuntimeAsync(host.RuntimeRoot, host.DataDirectory, TestContext.Current.CancellationToken)); + Assert.True(await upgrade.TryRestoreRescuedRuntimeAsync(host.RuntimeRoot, host.Zip, host.DataDirectory, TestContext.Current.CancellationToken)); Assert.Equal("the-stores-own", File.ReadAllText(Path.Combine(host.Pgsql, "bin", "runtime.txt"))); Assert.False(File.Exists(Path.Combine(host.Pgsql, "lib", "half-extracted.dll"))); @@ -3201,7 +3214,7 @@ public async Task RestoreRescuedRuntime_NoStore_MovesNothing() ReadRuntimeVersionLine = VersionsByBin((host.PreviousBin, "pg_ctl (PostgreSQL) 17.6")), }; - Assert.False(await upgrade.TryRestoreRescuedRuntimeAsync(host.RuntimeRoot, host.DataDirectory, TestContext.Current.CancellationToken)); + Assert.False(await upgrade.TryRestoreRescuedRuntimeAsync(host.RuntimeRoot, host.Zip, host.DataDirectory, TestContext.Current.CancellationToken)); Assert.False(Directory.Exists(host.Pgsql)); Assert.True(File.Exists(Path.Combine(host.PreviousBin, "pg_ctl.exe"))); @@ -3224,7 +3237,7 @@ public async Task RestoreRescuedRuntime_AStoreWithNoRescuedRuntime_MovesNothing( ReadRuntimeVersionLine = VersionsByBin((host.PreviousBin, "pg_ctl (PostgreSQL) 17.6")), }; - Assert.False(await upgrade.TryRestoreRescuedRuntimeAsync(host.RuntimeRoot, host.DataDirectory, TestContext.Current.CancellationToken)); + Assert.False(await upgrade.TryRestoreRescuedRuntimeAsync(host.RuntimeRoot, host.Zip, host.DataDirectory, TestContext.Current.CancellationToken)); Assert.False(Directory.Exists(host.Pgsql)); Assert.False(Directory.Exists(host.PreviousPgsql)); @@ -3248,7 +3261,7 @@ public async Task RestoreRescuedRuntime_ARescuedRuntimeOfAnotherMajor_MovesNothi ReadRuntimeVersionLine = VersionsByBin((host.PreviousBin, "pg_ctl (PostgreSQL) 18.4")), }; - Assert.False(await upgrade.TryRestoreRescuedRuntimeAsync(host.RuntimeRoot, host.DataDirectory, TestContext.Current.CancellationToken)); + Assert.False(await upgrade.TryRestoreRescuedRuntimeAsync(host.RuntimeRoot, host.Zip, host.DataDirectory, TestContext.Current.CancellationToken)); Assert.False(Directory.Exists(host.Pgsql)); Assert.True(File.Exists(Path.Combine(host.PreviousBin, "pg_ctl.exe"))); @@ -3273,7 +3286,7 @@ public async Task RestoreRescuedRuntime_ALiveRuntimeInPlace_MovesNothing() ReadRuntimeVersionLine = VersionsByBin((host.PreviousBin, "pg_ctl (PostgreSQL) 17.6")), }; - Assert.False(await upgrade.TryRestoreRescuedRuntimeAsync(host.RuntimeRoot, host.DataDirectory, TestContext.Current.CancellationToken)); + Assert.False(await upgrade.TryRestoreRescuedRuntimeAsync(host.RuntimeRoot, host.Zip, host.DataDirectory, TestContext.Current.CancellationToken)); Assert.Equal("live", File.ReadAllText(Path.Combine(host.Pgsql, "bin", "runtime.txt"))); Assert.Equal("rescued", File.ReadAllText(Path.Combine(host.PreviousBin, "runtime.txt"))); @@ -3284,6 +3297,182 @@ public async Task RestoreRescuedRuntime_ALiveRuntimeInPlace_MovesNothing() } } + /// Plants the shape of a restore that WOULD fire (no pg_ctl, a same-major rescued copy), for a gate to refuse. + private static DarlingStoreUpgrade PlantRestorableHost(RestoreHost host, CapturingLogger log) + { + Directory.CreateDirectory(host.Pgsql); + PlantRuntime(host.PreviousPgsql, "rescued"); + return new DarlingStoreUpgrade(log) + { + ReadRuntimeVersionLine = VersionsByBin((host.PreviousBin, "pg_ctl (PostgreSQL) 17.6")), + }; + } + + private static void AssertNothingMoved(RestoreHost host) + { + Assert.False(File.Exists(Path.Combine(host.Pgsql, "bin", "pg_ctl.exe"))); + Assert.False(Directory.Exists(host.Pgsql + ".failed")); + Assert.Equal("rescued", File.ReadAllText(Path.Combine(host.PreviousBin, "runtime.txt"))); + } + + [Fact] + public async Task RestoreRescuedRuntime_AStampThatMatchesThePackage_MovesNothing() + { + var root = Directory.CreateTempSubdirectory("darling-restore-finished-"); + try + { + var host = PlantRestoreHost(root.FullName, "17"); + var upgrade = PlantRestorableHost(host, new CapturingLogger()); + File.WriteAllText(host.StampPath, DarlingStoreUpgrade.ComputeFileHash(host.Zip).ToUpperInvariant()); + + Assert.False(await upgrade.TryRestoreRescuedRuntimeAsync(host.RuntimeRoot, host.Zip, host.DataDirectory, TestContext.Current.CancellationToken)); + + AssertNothingMoved(host); + } + finally + { + TryDeleteTree(root.FullName); + } + } + + [Fact] + public async Task RestoreRescuedRuntime_NoStampAtAll_MovesNothing() + { + var root = Directory.CreateTempSubdirectory("darling-restore-nostamp-"); + try + { + var host = PlantRestoreHost(root.FullName, "17"); + var upgrade = PlantRestorableHost(host, new CapturingLogger()); + File.Delete(host.StampPath); + + Assert.False(await upgrade.TryRestoreRescuedRuntimeAsync(host.RuntimeRoot, host.Zip, host.DataDirectory, TestContext.Current.CancellationToken)); + + AssertNothingMoved(host); + } + finally + { + TryDeleteTree(root.FullName); + } + } + + [Fact] + public async Task RestoreRescuedRuntime_AMissingPackage_MovesNothing() + { + var root = Directory.CreateTempSubdirectory("darling-restore-nozip-"); + try + { + var host = PlantRestoreHost(root.FullName, "17"); + var upgrade = PlantRestorableHost(host, new CapturingLogger()); + File.Delete(host.Zip); + + Assert.False(await upgrade.TryRestoreRescuedRuntimeAsync(host.RuntimeRoot, host.Zip, host.DataDirectory, TestContext.Current.CancellationToken)); + + AssertNothingMoved(host); + } + finally + { + TryDeleteTree(root.FullName); + } + } + + [Fact] + public async Task RestoreRescuedRuntime_AServerRunningOnTheStore_MovesNothing_AndNamesThePid() + { + var root = Directory.CreateTempSubdirectory("darling-restore-live-pm-"); + try + { + var host = PlantRestoreHost(root.FullName, "17"); + var log = new CapturingLogger(); + var upgrade = PlantRestorableHost(host, log); + using var postmaster = StartProcessNamedPostgres(Path.Combine(root.FullName, "fake")); + File.WriteAllText( + Path.Combine(host.DataDirectory, "postmaster.pid"), + postmaster.Id.ToString(System.Globalization.CultureInfo.InvariantCulture) + "\n" + host.DataDirectory + "\n"); + + Assert.False(await upgrade.TryRestoreRescuedRuntimeAsync(host.RuntimeRoot, host.Zip, host.DataDirectory, TestContext.Current.CancellationToken)); + + AssertNothingMoved(host); + Assert.Contains("PID " + postmaster.Id.ToString(System.Globalization.CultureInfo.InvariantCulture), log.ToString(), StringComparison.Ordinal); + Assert.Equal(1, CountWarnings(log)); + } + finally + { + TryDeleteTree(root.FullName); + } + } + + /// + /// A live process whose image is named postgres (a copy of a harmless system tool that idles for a + /// minute), which is all the liveness check can see of a real postmaster. Killed on dispose. + /// + private static ProcessHandle StartProcessNamedPostgres(string directory) + { + Directory.CreateDirectory(directory); + var windows = OperatingSystem.IsWindows(); + var source = windows ? Path.Combine(Environment.SystemDirectory, "PING.EXE") : "/bin/sleep"; + var image = Path.Combine(directory, windows ? "postgres.exe" : "postgres"); + File.Copy(source, image, overwrite: true); + if (OperatingSystem.IsMacOS()) + { + using var sign = System.Diagnostics.Process.Start("codesign", ["-f", "-s", "-", image]); + sign!.WaitForExit(); + } + + var process = System.Diagnostics.Process.Start(new System.Diagnostics.ProcessStartInfo(image, windows ? "-n 60 127.0.0.1" : "60") + { + UseShellExecute = false, + CreateNoWindow = true, + RedirectStandardOutput = true, + RedirectStandardError = true, + }) ?? throw new InvalidOperationException("Could not start " + image); + return new ProcessHandle(process); + } + + private sealed class ProcessHandle(System.Diagnostics.Process process) : IDisposable + { + public int Id => process.Id; + + public void Dispose() + { + try + { + if (!process.HasExited) + { + process.Kill(); + process.WaitForExit(10_000); + } + } + catch (InvalidOperationException) + { + } + + process.Dispose(); + } + } + + [Fact] + public async Task RestoreRescuedRuntime_AMoveThatKeepsFailing_LogsAndReturnsFalse() + { + var root = Directory.CreateTempSubdirectory("darling-restore-stuck-"); + try + { + var host = PlantRestoreHost(root.FullName, "17"); + var log = new CapturingLogger(); + var upgrade = PlantRestorableHost(host, log); + upgrade.RetryDelay = (_, _) => Task.CompletedTask; + upgrade.MoveRuntimeDirectory = (_, _) => throw new IOException("The process cannot access the file because it is being used by another process."); + + Assert.False(await upgrade.TryRestoreRescuedRuntimeAsync(host.RuntimeRoot, host.Zip, host.DataDirectory, TestContext.Current.CancellationToken)); + + Assert.True(File.Exists(Path.Combine(host.PreviousBin, "pg_ctl.exe"))); + Assert.Contains("could not be put back", log.ToString(), StringComparison.Ordinal); + } + finally + { + TryDeleteTree(root.FullName); + } + } + [Fact] public async Task RestoreRescuedRuntime_AMoveLockedTwice_RetriesAndRestores() { @@ -3309,7 +3498,7 @@ public async Task RestoreRescuedRuntime_AMoveLockedTwice_RetriesAndRestores() }, }; - Assert.True(await upgrade.TryRestoreRescuedRuntimeAsync(host.RuntimeRoot, host.DataDirectory, TestContext.Current.CancellationToken)); + Assert.True(await upgrade.TryRestoreRescuedRuntimeAsync(host.RuntimeRoot, host.Zip, host.DataDirectory, TestContext.Current.CancellationToken)); Assert.Equal(3, attempts); Assert.Equal(2, CountRetryLines(log)); @@ -3328,7 +3517,7 @@ public void EnsureRuntime_RestoresTheRescuedRuntime_BeforeItLooksForPgCtl() var start = source.IndexOf("private async Task EnsureRuntimeAsync(", StringComparison.Ordinal); Assert.True(start >= 0, "EnsureRuntimeAsync must exist"); var body = source[start..]; - var restore = body.IndexOf("_storeUpgrade.TryRestoreRescuedRuntimeAsync(_runtimeRoot, _dataDirectory, cancellationToken)", StringComparison.Ordinal); + var restore = body.IndexOf("_storeUpgrade.TryRestoreRescuedRuntimeAsync(_runtimeRoot, _runtimeZipPath, _dataDirectory, cancellationToken)", StringComparison.Ordinal); var existsCheck = body.IndexOf("if (File.Exists(pgCtl))", StringComparison.Ordinal); Assert.True(restore >= 0, "EnsureRuntimeAsync must try to restore the rescued runtime"); Assert.True(existsCheck >= 0, "EnsureRuntimeAsync must test for pg_ctl.exe"); diff --git a/Darling/PerformanceMonitor.Darling.Service/DarlingManagedPostgres.cs b/Darling/PerformanceMonitor.Darling.Service/DarlingManagedPostgres.cs index b3249d7d7..72ee9e2a1 100644 --- a/Darling/PerformanceMonitor.Darling.Service/DarlingManagedPostgres.cs +++ b/Darling/PerformanceMonitor.Darling.Service/DarlingManagedPostgres.cs @@ -3339,7 +3339,7 @@ private async Task EnsureRuntimeAsync(CancellationToken cancellationToke /* #4934: a runtime update that died between the rescue and a good extract leaves no pg_ctl.exe here and the store's own runtime in pg-runtime-prev. Put it back first, so the branch below takes the normal path (and retries the update) instead of the first-run extract. */ - await _storeUpgrade.TryRestoreRescuedRuntimeAsync(_runtimeRoot, _dataDirectory, cancellationToken); + await _storeUpgrade.TryRestoreRescuedRuntimeAsync(_runtimeRoot, _runtimeZipPath, _dataDirectory, cancellationToken); if (File.Exists(pgCtl)) { diff --git a/Darling/PerformanceMonitor.Darling.Service/DarlingStoreUpgrade.cs b/Darling/PerformanceMonitor.Darling.Service/DarlingStoreUpgrade.cs index c92067372..1f949428f 100644 --- a/Darling/PerformanceMonitor.Darling.Service/DarlingStoreUpgrade.cs +++ b/Darling/PerformanceMonitor.Darling.Service/DarlingStoreUpgrade.cs @@ -1069,21 +1069,34 @@ internal sealed record RuntimeAdvance(bool Swapped, string? PreviousBinDirectory } /// - /// Puts the store's own rescued runtime back at pgsql when the live runtime folder holds no - /// bin\pg_ctl.exe and the rescued copy under is the one that - /// last opened the store. Returns true when it moved the runtime back; false, having changed nothing, - /// in every other case (a live runtime is there, there is no store, no rescued copy, or the rescued copy - /// is another major than the store's, which already refuses). + /// Puts the store's own rescued runtime back at pgsql when an INTERRUPTED runtime update left no + /// bin\pg_ctl.exe there and the rescued copy under is the one + /// that last opened the store. Returns true when it moved the runtime back; false, having changed + /// nothing, in every other case: a live runtime is there, there is no store or no rescued copy of the + /// store's major (which already refuses), no stamp exists, a + /// server is running on the data directory, the shipped package is missing, the stamp already names the + /// shipped package, or a restore move still fails after its retries (logged; the first-run branch then + /// runs as it did before this restore existed). /// /// The shape: a runtime update moved the live runtime aside and its extract never finished (the /// process died, or an antivirus scan held the folder), leaving an empty or partial pgsql with the /// good runtime in pg-runtime-prev. Without this, the next start saw no pg_ctl.exe, took the /// first-run branch and extracted the package as if there were no store. /// - /// A move that still fails after its retries throws, as the first-run branch does: there is no - /// runtime to start on, and no fallback state is invented for it. + /// The stamp is what separates an interrupted update from a finished one. The advance writes the + /// stamp only after a good extract, so an interrupted update leaves the OLD stamp, which differs from the + /// shipped package. pg-runtime-prev is never emptied after a successful update, so a finished host + /// that later loses pgsql still has an older same-major runtime there; putting that back would + /// leave it in front of the store with a stamp that never triggers a retry. That host re-extracts the + /// shipped package instead. A host with no stamp at all cannot be proven interrupted, so it takes the + /// same path. + /// + /// The checks run cheapest first. The same-major test runs the rescued binary's version probe, + /// which can take up to the tool timeout (about five minutes) on a hung binary. The package is hashed + /// last, only when everything else says the restore is due. /// - internal async Task TryRestoreRescuedRuntimeAsync(string runtimeRoot, string dataDirectory, CancellationToken cancellationToken) + internal async Task TryRestoreRescuedRuntimeAsync( + string runtimeRoot, string runtimeZipPath, string dataDirectory, CancellationToken cancellationToken) { var pgsqlDirectory = Path.Combine(runtimeRoot, "pgsql"); if (File.Exists(Path.Combine(pgsqlDirectory, "bin", "pg_ctl.exe"))) @@ -1096,24 +1109,64 @@ internal async Task TryRestoreRescuedRuntimeAsync(string runtimeRoot, stri return false; } + var stamp = ReadTrimmedOrNull(Path.Combine(runtimeRoot, RuntimeStampFileName)) + ?? ReadTrimmedOrNull(Path.Combine(runtimeRoot, LegacyRuntimeStampFileName)); + if (stamp is null) + { + return false; + } + + /* Windows lets a folder be moved while an exe inside it runs, so a server started by hand from the + rescued runtime would lose its binaries. The same guard RevertRuntime uses. */ + var livePostmaster = FindLivePostmaster(dataDirectory); + if (livePostmaster is not null) + { + _logger.LogWarning( + "The Postgres runtime at {Runtime} had no pg_ctl.exe, but a PostgreSQL server (PID {Pid}) is running on {DataDirectory}, so the rescued runtime was not moved.", + pgsqlDirectory, livePostmaster, dataDirectory); + return false; + } + + if (!File.Exists(runtimeZipPath)) + { + return false; + } + + var zipHash = await Task.Run(() => ComputeFileHash(runtimeZipPath), cancellationToken); + if (string.Equals(stamp, zipHash, StringComparison.OrdinalIgnoreCase)) + { + return false; + } + var previousPgsql = Path.Combine(PreviousRuntimeRootFor(runtimeRoot), "pgsql"); /* A partial pgsql goes aside before the restore, exactly as the extract-failure revert does it: a move is one operation, a recursive delete is not, and a half-deleted folder is no runtime. */ var failedExtract = pgsqlDirectory + ".failed"; - TryDeleteDirectory(failedExtract); - if (Directory.Exists(pgsqlDirectory)) + try { + TryDeleteDirectory(failedExtract); + if (Directory.Exists(pgsqlDirectory)) + { + await RetryTransientIoAsync( + () => MoveRuntimeDirectory(pgsqlDirectory, failedExtract), + $"the move aside of the incomplete runtime at {pgsqlDirectory}", + cancellationToken); + } + await RetryTransientIoAsync( - () => MoveRuntimeDirectory(pgsqlDirectory, failedExtract), - $"the move aside of the incomplete runtime at {pgsqlDirectory}", + () => MoveRuntimeDirectory(previousPgsql, pgsqlDirectory), + $"the restore of the rescued runtime to {pgsqlDirectory}", cancellationToken); } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) + { + _logger.LogWarning( + "The rescued Postgres runtime at {Previous} could not be put back at {Runtime}: {Reason}. The runtime is extracted from the package instead.", + previousPgsql, pgsqlDirectory, ex.Message); + return false; + } - await RetryTransientIoAsync( - () => MoveRuntimeDirectory(previousPgsql, pgsqlDirectory), - $"the restore of the rescued runtime to {pgsqlDirectory}", - cancellationToken); TryDeleteDirectory(failedExtract); /* The stamp still names the runtime that was live before the interrupted update: the advance writes it @@ -1121,7 +1174,7 @@ only after a good extract (File.WriteAllText(stampPath, zipHash) in TryAdvanceRu normal path that follows compares the package against that stamp, sees the difference, and retries the update. */ _logger.LogWarning( - "The Postgres runtime at {Runtime} had no pg_ctl.exe, and the runtime that last opened the store at {DataDirectory} was found at {Previous}. It was put back, and the runtime update is retried on this start.", + "The Postgres runtime at {Runtime} had no pg_ctl.exe, and the runtime that last opened the store at {DataDirectory} was found at {Previous}. The incomplete runtime was moved aside and deleted, the rescued runtime was put back, and the runtime update is retried on this start.", pgsqlDirectory, dataDirectory, previousPgsql); return true; } From ef41185c02cdcda5c6b7e2a144f16616b0cf3a8d Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Fri, 2 Oct 2026 00:19:15 -0400 Subject: [PATCH 10/11] Darling: the runtime restore's Warning says the incomplete runtime was moved aside only when it was --- .../Darling.Tests/DarlingStoreUpgradeTests.cs | 31 ++++++++++++++++++- .../DarlingStoreUpgrade.cs | 18 +++++++++-- 2 files changed, 45 insertions(+), 4 deletions(-) diff --git a/Darling/Darling.Tests/DarlingStoreUpgradeTests.cs b/Darling/Darling.Tests/DarlingStoreUpgradeTests.cs index 4342f77ef..e3d687e6b 100644 --- a/Darling/Darling.Tests/DarlingStoreUpgradeTests.cs +++ b/Darling/Darling.Tests/DarlingStoreUpgradeTests.cs @@ -3167,6 +3167,7 @@ public async Task RestoreRescuedRuntime_AnEmptyPgsqlAndTheStoresRescuedRuntime_P Assert.True(File.Exists(Path.Combine(host.Pgsql, "bin", "pg_ctl.exe"))); Assert.False(Directory.Exists(host.PreviousPgsql)); Assert.Equal(1, CountWarnings(log)); + Assert.Contains("The incomplete runtime was moved aside and deleted", log.ToString()); } finally { @@ -3184,13 +3185,15 @@ public async Task RestoreRescuedRuntime_APartialPgsql_IsMovedAsideAndDeleted_And Directory.CreateDirectory(Path.Combine(host.Pgsql, "lib")); File.WriteAllText(Path.Combine(host.Pgsql, "lib", "half-extracted.dll"), "partial"); PlantRuntime(host.PreviousPgsql, "the-stores-own"); - var upgrade = new DarlingStoreUpgrade(new CapturingLogger()) + var log = new CapturingLogger(); + var upgrade = new DarlingStoreUpgrade(log) { ReadRuntimeVersionLine = VersionsByBin((host.PreviousBin, "pg_ctl (PostgreSQL) 17.6")), }; Assert.True(await upgrade.TryRestoreRescuedRuntimeAsync(host.RuntimeRoot, host.Zip, host.DataDirectory, TestContext.Current.CancellationToken)); + Assert.Contains("The incomplete runtime was moved aside and deleted", log.ToString()); Assert.Equal("the-stores-own", File.ReadAllText(Path.Combine(host.Pgsql, "bin", "runtime.txt"))); Assert.False(File.Exists(Path.Combine(host.Pgsql, "lib", "half-extracted.dll"))); Assert.False(Directory.Exists(host.Pgsql + ".failed")); @@ -3201,6 +3204,32 @@ public async Task RestoreRescuedRuntime_APartialPgsql_IsMovedAsideAndDeleted_And } } + [Fact] + public async Task RestoreRescuedRuntime_NoRuntimeFolderAtAll_PutsTheRescuedOneBack_AndSaysOnlyThat() + { + var root = Directory.CreateTempSubdirectory("darling-restore-absent-"); + try + { + var host = PlantRestoreHost(root.FullName, "17"); + PlantRuntime(host.PreviousPgsql, "the-stores-own"); + var log = new CapturingLogger(); + var upgrade = new DarlingStoreUpgrade(log) + { + ReadRuntimeVersionLine = VersionsByBin((host.PreviousBin, "pg_ctl (PostgreSQL) 17.6")), + }; + + Assert.True(await upgrade.TryRestoreRescuedRuntimeAsync(host.RuntimeRoot, host.Zip, host.DataDirectory, TestContext.Current.CancellationToken)); + + Assert.Equal("the-stores-own", File.ReadAllText(Path.Combine(host.Pgsql, "bin", "runtime.txt"))); + Assert.Contains("The rescued runtime was put back", log.ToString()); + Assert.DoesNotContain("moved aside", log.ToString()); + } + finally + { + TryDeleteTree(root.FullName); + } + } + [Fact] public async Task RestoreRescuedRuntime_NoStore_MovesNothing() { diff --git a/Darling/PerformanceMonitor.Darling.Service/DarlingStoreUpgrade.cs b/Darling/PerformanceMonitor.Darling.Service/DarlingStoreUpgrade.cs index 1f949428f..f9b126a7d 100644 --- a/Darling/PerformanceMonitor.Darling.Service/DarlingStoreUpgrade.cs +++ b/Darling/PerformanceMonitor.Darling.Service/DarlingStoreUpgrade.cs @@ -1143,11 +1143,13 @@ rescued runtime would lose its binaries. The same guard RevertRuntime uses. */ /* A partial pgsql goes aside before the restore, exactly as the extract-failure revert does it: a move is one operation, a recursive delete is not, and a half-deleted folder is no runtime. */ var failedExtract = pgsqlDirectory + ".failed"; + var movedAside = false; try { TryDeleteDirectory(failedExtract); if (Directory.Exists(pgsqlDirectory)) { + movedAside = true; await RetryTransientIoAsync( () => MoveRuntimeDirectory(pgsqlDirectory, failedExtract), $"the move aside of the incomplete runtime at {pgsqlDirectory}", @@ -1173,9 +1175,19 @@ await RetryTransientIoAsync( only after a good extract (File.WriteAllText(stampPath, zipHash) in TryAdvanceRuntimeAsync). So the normal path that follows compares the package against that stamp, sees the difference, and retries the update. */ - _logger.LogWarning( - "The Postgres runtime at {Runtime} had no pg_ctl.exe, and the runtime that last opened the store at {DataDirectory} was found at {Previous}. The incomplete runtime was moved aside and deleted, the rescued runtime was put back, and the runtime update is retried on this start.", - pgsqlDirectory, dataDirectory, previousPgsql); + if (movedAside) + { + _logger.LogWarning( + "The Postgres runtime at {Runtime} had no pg_ctl.exe, and the runtime that last opened the store at {DataDirectory} was found at {Previous}. The incomplete runtime was moved aside and deleted, the rescued runtime was put back, and the runtime update is retried on this start.", + pgsqlDirectory, dataDirectory, previousPgsql); + } + else + { + _logger.LogWarning( + "The Postgres runtime at {Runtime} had no pg_ctl.exe, and the runtime that last opened the store at {DataDirectory} was found at {Previous}. The rescued runtime was put back, and the runtime update is retried on this start.", + pgsqlDirectory, dataDirectory, previousPgsql); + } + return true; } From 2461a09ea96340d0687dc29b1b1dd26506dfbb5f Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Fri, 2 Oct 2026 00:32:49 -0400 Subject: [PATCH 11/11] Darling: the start-time runtime restore also refuses a rescued runtime that cannot load the store's TimescaleDB --- .../Darling.Tests/DarlingStoreUpgradeTests.cs | 155 ++++++++++++++++++ .../DarlingStoreUpgrade.cs | 42 ++++- 2 files changed, 190 insertions(+), 7 deletions(-) diff --git a/Darling/Darling.Tests/DarlingStoreUpgradeTests.cs b/Darling/Darling.Tests/DarlingStoreUpgradeTests.cs index e3d687e6b..2c3c376c0 100644 --- a/Darling/Darling.Tests/DarlingStoreUpgradeTests.cs +++ b/Darling/Darling.Tests/DarlingStoreUpgradeTests.cs @@ -3384,6 +3384,161 @@ public async Task RestoreRescuedRuntime_NoStampAtAll_MovesNothing() } } + [Fact] + public async Task RestoreRescuedRuntime_ARescuedRuntimeThatCannotLoadTheStoresTimescale_MovesNothing() + { + var root = Directory.CreateTempSubdirectory("darling-restore-timescale-"); + try + { + var host = PlantRestoreHost(root.FullName, "17"); + var upgrade = PlantRestorableHost(host, new CapturingLogger()); + File.WriteAllText(Path.Combine(host.DataDirectory, DarlingStoreUpgrade.TimescaleRecordFileName), "2.28.1"); + var lib = Path.Combine(host.PreviousPgsql, "lib"); + Directory.CreateDirectory(lib); + File.WriteAllText(Path.Combine(lib, "timescaledb-2.24.0.dll"), "x"); + File.WriteAllText(Path.Combine(lib, "timescaledb-tsl-2.24.0.dll"), "x"); + + Assert.False(await upgrade.TryRestoreRescuedRuntimeAsync(host.RuntimeRoot, host.Zip, host.DataDirectory, TestContext.Current.CancellationToken)); + + AssertNothingMoved(host); + } + finally + { + TryDeleteTree(root.FullName); + } + } + + [Fact] + public async Task RestoreRescuedRuntime_ARescuedRuntimeThatCarriesTheStoresTimescale_PutsTheRuntimeBack() + { + var root = Directory.CreateTempSubdirectory("darling-restore-timescale-ok-"); + try + { + var host = PlantRestoreHost(root.FullName, "17"); + var upgrade = PlantRestorableHost(host, new CapturingLogger()); + File.WriteAllText(Path.Combine(host.DataDirectory, DarlingStoreUpgrade.TimescaleRecordFileName), "2.24.0"); + var lib = Path.Combine(host.PreviousPgsql, "lib"); + Directory.CreateDirectory(lib); + File.WriteAllText(Path.Combine(lib, "timescaledb-2.24.0.dll"), "x"); + File.WriteAllText(Path.Combine(lib, "timescaledb-tsl-2.24.0.dll"), "x"); + + Assert.True(await upgrade.TryRestoreRescuedRuntimeAsync(host.RuntimeRoot, host.Zip, host.DataDirectory, TestContext.Current.CancellationToken)); + + Assert.True(File.Exists(Path.Combine(host.Pgsql, "bin", "pg_ctl.exe"))); + Assert.False(Directory.Exists(host.PreviousPgsql)); + } + finally + { + TryDeleteTree(root.FullName); + } + } + + [Fact] + public async Task RestoreRescuedRuntime_BothStampsPresent_TheMainStampDecides_MovesNothing() + { + var root = Directory.CreateTempSubdirectory("darling-restore-bothstamps-"); + try + { + var host = PlantRestoreHost(root.FullName, "17"); + var upgrade = PlantRestorableHost(host, new CapturingLogger()); + File.WriteAllText(host.StampPath, DarlingStoreUpgrade.ComputeFileHash(host.Zip)); + File.WriteAllText( + Path.Combine(host.RuntimeRoot, DarlingStoreUpgrade.LegacyRuntimeStampFileName), + DarlingStoreUpgrade.LegacyRuntimePackageHash); + + Assert.False(await upgrade.TryRestoreRescuedRuntimeAsync(host.RuntimeRoot, host.Zip, host.DataDirectory, TestContext.Current.CancellationToken)); + + AssertNothingMoved(host); + } + finally + { + TryDeleteTree(root.FullName); + } + } + + [Fact] + public async Task RestoreRescuedRuntime_OnlyTheLegacyStamp_AnInterruptedFirstUpdate_PutsTheRuntimeBack() + { + var root = Directory.CreateTempSubdirectory("darling-restore-legacystamp-"); + try + { + var host = PlantRestoreHost(root.FullName, "17"); + var upgrade = PlantRestorableHost(host, new CapturingLogger()); + File.Delete(host.StampPath); + File.WriteAllText( + Path.Combine(host.RuntimeRoot, DarlingStoreUpgrade.LegacyRuntimeStampFileName), + DarlingStoreUpgrade.LegacyRuntimePackageHash); + + Assert.True(await upgrade.TryRestoreRescuedRuntimeAsync(host.RuntimeRoot, host.Zip, host.DataDirectory, TestContext.Current.CancellationToken)); + + Assert.Equal("rescued", File.ReadAllText(Path.Combine(host.Pgsql, "bin", "runtime.txt"))); + Assert.False(Directory.Exists(host.PreviousPgsql)); + } + finally + { + TryDeleteTree(root.FullName); + } + } + + [Fact] + public async Task RestoreRescuedRuntime_AMainStampThatExistsButIsEmpty_MovesNothing() + { + var root = Directory.CreateTempSubdirectory("darling-restore-emptystamp-"); + try + { + var host = PlantRestoreHost(root.FullName, "17"); + var upgrade = PlantRestorableHost(host, new CapturingLogger()); + File.WriteAllText(host.StampPath, string.Empty); + File.WriteAllText( + Path.Combine(host.RuntimeRoot, DarlingStoreUpgrade.LegacyRuntimeStampFileName), + DarlingStoreUpgrade.LegacyRuntimePackageHash); + + Assert.False(await upgrade.TryRestoreRescuedRuntimeAsync(host.RuntimeRoot, host.Zip, host.DataDirectory, TestContext.Current.CancellationToken)); + + AssertNothingMoved(host); + } + finally + { + TryDeleteTree(root.FullName); + } + } + + [Fact] + public async Task RestoreRescuedRuntime_APackageThatCannotBeRead_MovesNothing_AndDoesNotThrow() + { + var root = Directory.CreateTempSubdirectory("darling-restore-lockedzip-"); + FileStream? hold = null; + UnixFileMode? original = null; + var host = PlantRestoreHost(root.FullName, "17"); + try + { + var upgrade = PlantRestorableHost(host, new CapturingLogger()); + if (OperatingSystem.IsWindows()) + { + hold = new FileStream(host.Zip, FileMode.Open, FileAccess.Read, FileShare.None); + } + else + { + original = File.GetUnixFileMode(host.Zip); + File.SetUnixFileMode(host.Zip, UnixFileMode.None); + } + + Assert.False(await upgrade.TryRestoreRescuedRuntimeAsync(host.RuntimeRoot, host.Zip, host.DataDirectory, TestContext.Current.CancellationToken)); + + AssertNothingMoved(host); + } + finally + { + hold?.Dispose(); + if (!OperatingSystem.IsWindows() && original is { } mode) + { + File.SetUnixFileMode(host.Zip, mode); + } + + TryDeleteTree(root.FullName); + } + } + [Fact] public async Task RestoreRescuedRuntime_AMissingPackage_MovesNothing() { diff --git a/Darling/PerformanceMonitor.Darling.Service/DarlingStoreUpgrade.cs b/Darling/PerformanceMonitor.Darling.Service/DarlingStoreUpgrade.cs index f9b126a7d..47cec4e65 100644 --- a/Darling/PerformanceMonitor.Darling.Service/DarlingStoreUpgrade.cs +++ b/Darling/PerformanceMonitor.Darling.Service/DarlingStoreUpgrade.cs @@ -1089,7 +1089,9 @@ internal sealed record RuntimeAdvance(bool Swapped, string? PreviousBinDirectory /// that later loses pgsql still has an older same-major runtime there; putting that back would /// leave it in front of the store with a stamp that never triggers a retry. That host re-extracts the /// shipped package instead. A host with no stamp at all cannot be proven interrupted, so it takes the - /// same path. + /// same path. A finished host that later receives a newer package can also pass the stamp test, so the + /// store's recorded TimescaleDB versions must all be carried by the rescued runtime: a rescued runtime + /// that could not open the store stays where it is. /// /// The checks run cheapest first. The same-major test runs the rescued binary's version probe, /// which can take up to the tool timeout (about five minutes) on a hung binary. The package is hashed @@ -1109,9 +1111,16 @@ internal async Task TryRestoreRescuedRuntimeAsync( return false; } - var stamp = ReadTrimmedOrNull(Path.Combine(runtimeRoot, RuntimeStampFileName)) - ?? ReadTrimmedOrNull(Path.Combine(runtimeRoot, LegacyRuntimeStampFileName)); - if (stamp is null) + /* Only a MISSING main stamp falls back to the legacy one. A main stamp file that exists but cannot be + read is no proof of an interrupted update, so nothing is restored. */ + var mainStampPath = Path.Combine(runtimeRoot, RuntimeStampFileName); + var stamp = ReadTrimmedOrNull(mainStampPath); + if (stamp is null && !File.Exists(mainStampPath)) + { + stamp = ReadTrimmedOrNull(Path.Combine(runtimeRoot, LegacyRuntimeStampFileName)); + } + + if (string.IsNullOrEmpty(stamp)) { return false; } @@ -1127,18 +1136,37 @@ rescued runtime would lose its binaries. The same guard RevertRuntime uses. */ return false; } + /* In an interrupted update the store is still on the rescued runtime's extension, because the extension + moves only after a good swap, so the rescued runtime carries every version the store records. A + finished host that later receives a newer package also passes the stamp test below; its rescued + runtime predates the store's extension, and putting it back would leave a runtime that cannot load + TimescaleDB in front of the store. With no record this abstains. */ + var previousPgsql = Path.Combine(PreviousRuntimeRootFor(runtimeRoot), "pgsql"); + if (ReadTimescaleRecord(dataDirectory) is { StoreVersions.Count: > 0 } record + && record.StoreVersions.Except(TryReadTimescaleLibraryVersions(previousPgsql), StringComparer.Ordinal).Any()) + { + return false; + } + if (!File.Exists(runtimeZipPath)) { return false; } - var zipHash = await Task.Run(() => ComputeFileHash(runtimeZipPath), cancellationToken); - if (string.Equals(stamp, zipHash, StringComparison.OrdinalIgnoreCase)) + string zipHash; + try + { + zipHash = await Task.Run(() => ComputeFileHash(runtimeZipPath), cancellationToken); + } + catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) { return false; } - var previousPgsql = Path.Combine(PreviousRuntimeRootFor(runtimeRoot), "pgsql"); + if (string.Equals(stamp, zipHash, StringComparison.OrdinalIgnoreCase)) + { + return false; + } /* A partial pgsql goes aside before the restore, exactly as the extract-failure revert does it: a move is one operation, a recursive delete is not, and a half-deleted folder is no runtime. */