From 49f4c2e85ff6d47268197fbf1b17766ccc5e7992 Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Mon, 28 Oct 2024 18:05:11 -0400 Subject: [PATCH 1/2] Add a bunch of logging to config writing and semaphore usage --- build/Version.props | 2 +- .../Components/StaticFiles/Configuration.cs | 23 +++++++------- .../Utils/SemaphoreSlimContext.cs | 30 +++++++++++++++---- 3 files changed, 38 insertions(+), 17 deletions(-) diff --git a/build/Version.props b/build/Version.props index f6e5331f590..18219375ac8 100644 --- a/build/Version.props +++ b/build/Version.props @@ -3,7 +3,7 @@ - 6.11.2 + 6.11.3 5.3.0 10.10.0 0.2.0 diff --git a/src/Tgstation.Server.Host/Components/StaticFiles/Configuration.cs b/src/Tgstation.Server.Host/Components/StaticFiles/Configuration.cs index 93e1a0b9193..eff31ebb25c 100644 --- a/src/Tgstation.Server.Host/Components/StaticFiles/Configuration.cs +++ b/src/Tgstation.Server.Host/Components/StaticFiles/Configuration.cs @@ -185,7 +185,7 @@ public Configuration( this.generalConfiguration = generalConfiguration ?? throw new ArgumentNullException(nameof(generalConfiguration)); this.sessionConfiguration = sessionConfiguration ?? throw new ArgumentNullException(nameof(sessionConfiguration)); - semaphore = new SemaphoreSlim(1); + semaphore = new SemaphoreSlim(1, 1); disposeCts = new CancellationTokenSource(); uploadTasks = Task.CompletedTask; } @@ -201,7 +201,7 @@ public void Dispose() /// public async ValueTask CopyDMFilesTo(string dmeFile, string destination, CancellationToken cancellationToken) { - using (await SemaphoreSlimContext.Lock(semaphore, cancellationToken)) + using (await SemaphoreSlimContext.Lock(semaphore, cancellationToken, logger)) { var ensureDirectoriesTask = EnsureDirectories(cancellationToken); @@ -270,7 +270,7 @@ void ListImpl() })); } - using (SemaphoreSlimContext.TryLock(semaphore, out var locked)) + using (SemaphoreSlimContext.TryLock(semaphore, logger, out var locked)) { if (!locked) { @@ -376,7 +376,7 @@ void GetFileStream() } } - using (SemaphoreSlimContext.TryLock(semaphore, out var locked)) + using (SemaphoreSlimContext.TryLock(semaphore, logger, out var locked)) { if (!locked) { @@ -434,7 +434,7 @@ await ValueTaskExtensions.WhenAll(entries.Select(async file = })); } - using (await SemaphoreSlimContext.Lock(semaphore, cancellationToken)) + using (await SemaphoreSlimContext.Lock(semaphore, cancellationToken, logger)) { await EnsureDirectories(cancellationToken); var ignoreFileBytes = await ioManager.ReadAllBytes(StaticIgnorePath(), cancellationToken); @@ -501,7 +501,7 @@ void WriteCallback() return; } - using (SemaphoreSlimContext.TryLock(semaphore, out var locked)) + using (SemaphoreSlimContext.TryLock(semaphore, logger, out var locked)) { if (!locked) { @@ -509,6 +509,7 @@ void WriteCallback() return; } + logger.LogTrace("Kicking off write callback"); if (systemIdentity == null) await Task.Factory.StartNew(WriteCallback, cancellationToken, DefaultIOManager.BlockingTaskCreationOptions, TaskScheduler.Current); else @@ -519,6 +520,8 @@ void WriteCallback() fileTicket.SetError(ErrorCode.ConfigurationFileUpdated, fileHash); else if (uploadStream.Length > 0) postWriteHandler.HandleWrite(path); + else + logger.LogTrace("Write complete"); } } @@ -559,7 +562,7 @@ void WriteCallback() } } - using (SemaphoreSlimContext.TryLock(semaphore, out var locked)) + using (SemaphoreSlimContext.TryLock(semaphore, logger, out var locked)) { if (!locked) { @@ -585,7 +588,7 @@ void WriteCallback() bool? result = null; void DoCreate() => result = synchronousIOManager.CreateDirectory(path, cancellationToken); - using (SemaphoreSlimContext.TryLock(semaphore, out var locked)) + using (SemaphoreSlimContext.TryLock(semaphore, logger, out var locked)) { if (!locked) { @@ -651,7 +654,7 @@ public ValueTask HandleEvent(EventType eventType, IEnumerable parameter var path = ValidateConfigRelativePath(configurationRelativePath); var result = false; - using (SemaphoreSlimContext.TryLock(semaphore, out var locked)) + using (SemaphoreSlimContext.TryLock(semaphore, logger, out var locked)) { if (!locked) { @@ -750,7 +753,7 @@ async ValueTask ExecuteEventScripts(IEnumerable parameters, bool deploy await EnsureDirectories(cancellationToken); // always execute in serial - using (await SemaphoreSlimContext.Lock(semaphore, cancellationToken)) + using (await SemaphoreSlimContext.Lock(semaphore, cancellationToken, logger)) { var files = await ioManager.GetFilesWithExtension(EventScriptsSubdirectory, platformIdentifier.ScriptFileExtension, false, cancellationToken); var resolvedScriptsDir = ioManager.ResolvePath(EventScriptsSubdirectory); diff --git a/src/Tgstation.Server.Host/Utils/SemaphoreSlimContext.cs b/src/Tgstation.Server.Host/Utils/SemaphoreSlimContext.cs index e9a40d7c68d..5775947b458 100644 --- a/src/Tgstation.Server.Host/Utils/SemaphoreSlimContext.cs +++ b/src/Tgstation.Server.Host/Utils/SemaphoreSlimContext.cs @@ -2,6 +2,8 @@ using System.Threading; using System.Threading.Tasks; +using Microsoft.Extensions.Logging; + namespace Tgstation.Server.Host.Utils { /// @@ -14,29 +16,39 @@ sealed class SemaphoreSlimContext : IDisposable /// /// The to lock. /// The for the operation. + /// An optional to write to. /// A resulting in the for the lock. - public static async ValueTask Lock(SemaphoreSlim semaphore, CancellationToken cancellationToken) + public static async ValueTask Lock(SemaphoreSlim semaphore, CancellationToken cancellationToken, ILogger? logger = null) { ArgumentNullException.ThrowIfNull(semaphore); + logger?.LogTrace("Acquiring semaphore..."); await semaphore.WaitAsync(cancellationToken); - return new SemaphoreSlimContext(semaphore); + return new SemaphoreSlimContext(semaphore, logger); } /// /// Asyncronously attempts to lock a . /// /// The to lock. + /// An optional to write to. /// The result of the lock attempt. /// A for the lock on success, or if it was not acquired. - public static SemaphoreSlimContext? TryLock(SemaphoreSlim semaphore, out bool locked) + public static SemaphoreSlimContext? TryLock(SemaphoreSlim semaphore, ILogger? logger, out bool locked) { ArgumentNullException.ThrowIfNull(semaphore); + logger?.LogTrace("Trying to acquire semaphore..."); locked = semaphore.Wait(TimeSpan.Zero); + logger?.LogTrace("Acquired semaphore {un}successfully", locked ? String.Empty : "un"); return locked - ? new SemaphoreSlimContext(semaphore) + ? new SemaphoreSlimContext(semaphore, logger) : null; } + /// + /// An optional to write to. + /// + readonly ILogger? logger; + /// /// The locked . /// @@ -46,14 +58,20 @@ public static async ValueTask Lock(SemaphoreSlim semaphore /// Initializes a new instance of the class. /// /// The value of . - SemaphoreSlimContext(SemaphoreSlim lockedSemaphore) + /// The value of . + SemaphoreSlimContext(SemaphoreSlim lockedSemaphore, ILogger? logger) { this.lockedSemaphore = lockedSemaphore; + this.logger = logger; } /// /// Release the lock on . /// - public void Dispose() => lockedSemaphore.Release(); + public void Dispose() + { + logger?.LogTrace("Releasing semaphore..."); + lockedSemaphore.Release(); + } } } From 7521c78bf87a2a560dbaefe8a086a2cb76cde1b4 Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Tue, 29 Oct 2024 07:02:40 -0400 Subject: [PATCH 2/2] Add `Install winget` to list of flaky steps --- .github/workflows/scripts/rerunFlakyTests.js | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/scripts/rerunFlakyTests.js b/.github/workflows/scripts/rerunFlakyTests.js index b570c8c78b3..106bbd0f212 100644 --- a/.github/workflows/scripts/rerunFlakyTests.js +++ b/.github/workflows/scripts/rerunFlakyTests.js @@ -3,6 +3,7 @@ const CONSIDERED_STEP_PREFIXES = [ "Build", // Nuget.org sporadic issues "Install Native", // apt repository issues "Test Service", // systemd bollocks + "Install winget", // random-ass failures ]; // Otherwise only check jobs that start with these.