From af0f05d8bff10819bfbe5b08d1148fc16a460d5e Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Sat, 25 Nov 2023 15:05:03 -0500 Subject: [PATCH] Nullify `SwarmService` Had to use many forgiveness operators here, but I've validated them in their current state --- .../Swarm/ISwarmService.cs | 2 +- .../Swarm/SwarmService.cs | 119 ++++++++++-------- .../Swarm/SwarmUpdateOperation.cs | 2 +- 3 files changed, 70 insertions(+), 53 deletions(-) diff --git a/src/Tgstation.Server.Host/Swarm/ISwarmService.cs b/src/Tgstation.Server.Host/Swarm/ISwarmService.cs index 9b86389540..8e827eb131 100644 --- a/src/Tgstation.Server.Host/Swarm/ISwarmService.cs +++ b/src/Tgstation.Server.Host/Swarm/ISwarmService.cs @@ -38,6 +38,6 @@ namespace Tgstation.Server.Host.Swarm /// Gets the list of s in the swarm, including the current one. /// /// A of s in the swarm. If the server is not part of a swarm, will be returned. - ICollection GetSwarmServers(); + ICollection? GetSwarmServers(); } } diff --git a/src/Tgstation.Server.Host/Swarm/SwarmService.cs b/src/Tgstation.Server.Host/Swarm/SwarmService.cs index d081050754..7850f9e5e5 100644 --- a/src/Tgstation.Server.Host/Swarm/SwarmService.cs +++ b/src/Tgstation.Server.Host/Swarm/SwarmService.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; using System.Linq; using System.Net; using System.Net.Http; @@ -26,8 +27,6 @@ using Tgstation.Server.Host.System; using Tgstation.Server.Host.Transfer; using Tgstation.Server.Host.Utils; -#nullable disable - namespace Tgstation.Server.Host.Swarm { /// @@ -51,6 +50,7 @@ namespace Tgstation.Server.Host.Swarm /// /// If the swarm system is enabled. /// + [MemberNotNullWhen(true, nameof(serverHealthCheckTask), nameof(forceHealthCheckTcs), nameof(serverHealthCheckCancellationTokenSource), nameof(swarmServers))] bool SwarmMode => swarmConfiguration.PrivateKey != null; /// @@ -101,17 +101,17 @@ namespace Tgstation.Server.Host.Swarm /// /// The for . /// - readonly CancellationTokenSource serverHealthCheckCancellationTokenSource; + readonly CancellationTokenSource? serverHealthCheckCancellationTokenSource; /// /// of connected s. /// - readonly List swarmServers; + readonly List? swarmServers; /// /// of s to registration s and when they were created. /// - readonly Dictionary registrationIdsAndTimes; + readonly Dictionary? registrationIdsAndTimes; /// /// If the current server is the swarm controller. @@ -121,17 +121,17 @@ namespace Tgstation.Server.Host.Swarm /// /// A that is currently in progress. /// - volatile SwarmUpdateOperation updateOperation; + volatile SwarmUpdateOperation? updateOperation; /// /// A that is used to force a health check. /// - volatile TaskCompletionSource forceHealthCheckTcs; + volatile TaskCompletionSource? forceHealthCheckTcs; /// /// The for the . /// - Task serverHealthCheckTask; + Task? serverHealthCheckTask; /// /// The registration provided by the swarm controller. @@ -185,18 +185,17 @@ namespace Tgstation.Server.Host.Swarm { if (swarmConfiguration.Address == null) throw new InvalidOperationException("Swarm configuration missing Address!"); + if (String.IsNullOrWhiteSpace(swarmConfiguration.Identifier)) throw new InvalidOperationException("Swarm configuration missing Identifier!"); - } - swarmController = !SwarmMode || swarmConfiguration.ControllerAddress == null; - if (SwarmMode) - { - serverHealthCheckCancellationTokenSource = new CancellationTokenSource(); - forceHealthCheckTcs = new TaskCompletionSource(); + swarmController = swarmConfiguration.ControllerAddress == null; if (swarmController) registrationIdsAndTimes = new(); + serverHealthCheckCancellationTokenSource = new CancellationTokenSource(); + forceHealthCheckTcs = new TaskCompletionSource(); + swarmServers = new List { new SwarmServerResponse @@ -208,6 +207,8 @@ namespace Tgstation.Server.Host.Swarm }, }; } + else + swarmController = true; } /// @@ -348,7 +349,7 @@ namespace Tgstation.Server.Host.Swarm } /// - public ICollection GetSwarmServers() + public ICollection? GetSwarmServers() { if (!SwarmMode) return null; @@ -426,7 +427,7 @@ namespace Tgstation.Server.Host.Swarm { logger.LogTrace("Begin Shutdown"); - async ValueTask SendUnregistrationRequest(SwarmServerResponse swarmServer) + async ValueTask SendUnregistrationRequest(SwarmServerResponse? swarmServer) { using var httpClient = httpClientFactory.CreateClient(); using var request = PrepareSwarmRequest( @@ -445,13 +446,13 @@ namespace Tgstation.Server.Host.Swarm logger.LogWarning( ex, "Error unregistering {nodeType}!", - swarmController + swarmServer != null ? $"node {swarmServer.Identifier}" : "from controller"); } } - if (serverHealthCheckTask != null) + if (SwarmMode && serverHealthCheckTask != null) { serverHealthCheckCancellationTokenSource.Cancel(); await serverHealthCheckTask; @@ -491,7 +492,7 @@ namespace Tgstation.Server.Host.Swarm .Select(SendUnregistrationRequest) .ToList()); swarmServers.RemoveRange(1, swarmServers.Count - 1); - registrationIdsAndTimes.Clear(); + registrationIdsAndTimes!.Clear(); } await task; @@ -506,6 +507,9 @@ namespace Tgstation.Server.Host.Swarm { ArgumentNullException.ThrowIfNull(swarmServers); + if (!SwarmMode) + throw new InvalidOperationException("Swarm mode not enabled!"); + if (swarmController) throw new InvalidOperationException("Cannot UpdateSwarmServersList on swarm controller!"); @@ -520,9 +524,12 @@ namespace Tgstation.Server.Host.Swarm /// public bool ValidateRegistration(Guid registrationId) { + if (!SwarmMode) + throw new InvalidOperationException("Swarm mode not enabled!"); + if (swarmController) lock (swarmServers) - return registrationIdsAndTimes.Values.Any(x => x.RegistrationId == registrationId); + return registrationIdsAndTimes!.Values.Any(x => x.RegistrationId == registrationId); if (registrationId != controllerRegistration) return false; @@ -542,6 +549,9 @@ namespace Tgstation.Server.Host.Swarm if (node.Address == null) throw new ArgumentException("Node missing Address!", nameof(node)); + if (!SwarmMode) + throw new InvalidOperationException("Swarm mode not enabled!"); + if (!swarmController) throw new InvalidOperationException("Cannot RegisterNode on swarm node!"); @@ -549,6 +559,7 @@ namespace Tgstation.Server.Host.Swarm await AbortUpdate(); + var registrationIdsAndTimes = this.registrationIdsAndTimes!; lock (swarmServers) { if (registrationIdsAndTimes.Any(x => x.Value.RegistrationId == registrationId)) @@ -642,6 +653,9 @@ namespace Tgstation.Server.Host.Swarm /// public async ValueTask UnregisterNode(Guid registrationId, CancellationToken cancellationToken) { + if (!SwarmMode) + throw new InvalidOperationException("Swarm mode not enabled!"); + logger.LogTrace("UnregisterNode {registrationId}", registrationId); await AbortUpdate(); @@ -663,7 +677,7 @@ namespace Tgstation.Server.Host.Swarm lock (swarmServers) { swarmServers.RemoveAll(x => x.Identifier == nodeIdentifier); - registrationIdsAndTimes.Remove(nodeIdentifier); + registrationIdsAndTimes!.Remove(nodeIdentifier); } MarkServersDirty(); @@ -710,7 +724,7 @@ namespace Tgstation.Server.Host.Swarm Address = swarmConfiguration.ControllerAddress, }); - lock (swarmServers) + lock (swarmServers!) return ValueTaskExtensions.WhenAll( swarmServers .Where(x => !x.Controller) @@ -760,7 +774,7 @@ namespace Tgstation.Server.Host.Swarm /// The . Must always have populated. If is , it must be fully populated. /// The for the operation. /// A resulting in the . - async ValueTask PrepareUpdateImpl(ISeekableFileStreamProvider initiatorProvider, SwarmUpdateRequest updateRequest, CancellationToken cancellationToken) + async ValueTask PrepareUpdateImpl(ISeekableFileStreamProvider? initiatorProvider, SwarmUpdateRequest updateRequest, CancellationToken cancellationToken) { if (!SwarmMode) { @@ -777,7 +791,7 @@ namespace Tgstation.Server.Host.Swarm SwarmUpdateOperation localUpdateOperation; try { - SwarmServerResponse sourceNode = null; + SwarmServerResponse? sourceNode = null; List currentNodes; lock (swarmServers) { @@ -814,7 +828,7 @@ namespace Tgstation.Server.Host.Swarm if (!swarmController && initiator) { - var downloadTickets = await CreateDownloadTickets(initiatorProvider, currentNodes, cancellationToken); + var downloadTickets = await CreateDownloadTickets(initiatorProvider!, currentNodes, cancellationToken); // condition of initiator logger.LogInformation("Forwarding update request to swarm controller..."); using var httpClient = httpClientFactory.CreateClient(); @@ -854,7 +868,7 @@ namespace Tgstation.Server.Host.Swarm return SwarmPrepareResult.Failure; } - if (!updateRequest.DownloadTickets.TryGetValue(swarmConfiguration.Identifier, out var ticket)) + if (!updateRequest.DownloadTickets.TryGetValue(swarmConfiguration.Identifier!, out var ticket)) { logger.Log( swarmController @@ -924,7 +938,7 @@ namespace Tgstation.Server.Host.Swarm /// The for the operation. /// A resulting in the . async ValueTask ControllerDistributedPrepareUpdate( - ISeekableFileStreamProvider initiatorProvider, + ISeekableFileStreamProvider? initiatorProvider, SwarmUpdateRequest updateRequest, SwarmUpdateOperation currentUpdateOperation, CancellationToken cancellationToken) @@ -967,7 +981,7 @@ namespace Tgstation.Server.Host.Swarm } var downloadTicketDictionary = weAreInitiator - ? await CreateDownloadTickets(initiatorProvider, currentUpdateOperation.InvolvedServers, cancellationToken) + ? await CreateDownloadTickets(initiatorProvider!, currentUpdateOperation.InvolvedServers, cancellationToken) : updateRequest.DownloadTickets; var sourceNode = weAreInitiator @@ -984,18 +998,19 @@ namespace Tgstation.Server.Host.Swarm .Select(node => { // only send the necessary ticket to each node from the controller - Dictionary localTicketDictionary; - if (!downloadTicketDictionary.TryGetValue(node.Identifier, out var ticket) - && node.Identifier != sourceNode) + Dictionary localTicketDictionary; + var nodeId = node.Identifier!; + if (!downloadTicketDictionary.TryGetValue(nodeId, out var ticket) + && nodeId != sourceNode) { - logger.LogError("Missing download ticket for node {missingNodeId}!", node.Identifier); + logger.LogError("Missing download ticket for node {missingNodeId}!", nodeId); anyFailed = true; return null; } else - localTicketDictionary = new Dictionary + localTicketDictionary = new Dictionary { - { node.Identifier, ticket }, + { nodeId, ticket }, }; var request = new SwarmUpdateRequest @@ -1015,7 +1030,7 @@ namespace Tgstation.Server.Host.Swarm var tasks = updateRequests .Select(async tuple => { - var node = tuple.Item1; + var node = tuple!.Item1; var body = tuple.Item2; using var request = PrepareSwarmRequest( @@ -1088,7 +1103,7 @@ namespace Tgstation.Server.Host.Swarm var downloadTickets = new Dictionary(serversRequiringTickets.Count); foreach (var node in serversRequiringTickets) downloadTickets.Add( - node.Identifier, + node.Identifier!, transferService.CreateDownload(downloadProvider)); await streamRetrievalTask; @@ -1105,9 +1120,10 @@ namespace Tgstation.Server.Host.Swarm using var httpClient = httpClientFactory.CreateClient(); List currentSwarmServers; - lock (swarmServers) + lock (swarmServers!) currentSwarmServers = swarmServers.ToList(); + var registrationIdsAndTimes = this.registrationIdsAndTimes!; async ValueTask HealthRequestForServer(SwarmServerResponse swarmServer) { using var request = PrepareSwarmRequest( @@ -1133,14 +1149,14 @@ namespace Tgstation.Server.Host.Swarm lock (swarmServers) { swarmServers.Remove(swarmServer); - registrationIdsAndTimes.Remove(swarmServer.Identifier); + registrationIdsAndTimes.Remove(swarmServer.Identifier!); } } await ValueTaskExtensions.WhenAll( currentSwarmServers .Where(node => !node.Controller - && registrationIdsAndTimes.TryGetValue(node.Identifier, out var registrationAndTime) + && registrationIdsAndTimes.TryGetValue(node.Identifier!, out var registrationAndTime) && registrationAndTime.RegisteredAt.AddMinutes(SwarmConstants.ControllerHealthCheckIntervalMinutes) < DateTimeOffset.UtcNow) .Select(HealthRequestForServer)); @@ -1169,7 +1185,7 @@ namespace Tgstation.Server.Host.Swarm bool TriggerHealthCheck() { var currentTcs = Interlocked.Exchange(ref forceHealthCheckTcs, new TaskCompletionSource()); - return currentTcs.TrySetResult(); + return currentTcs!.TrySetResult(); } /// @@ -1299,7 +1315,7 @@ namespace Tgstation.Server.Host.Swarm async ValueTask SendUpdatedServerListToNodes(CancellationToken cancellationToken) { List currentSwarmServers; - lock (swarmServers) + lock (swarmServers!) { serversDirty = false; currentSwarmServers = swarmServers.ToList(); @@ -1337,7 +1353,7 @@ namespace Tgstation.Server.Host.Swarm lock (swarmServers) { swarmServers.Remove(swarmServer); - registrationIdsAndTimes.Remove(swarmServer.Identifier); + registrationIdsAndTimes!.Remove(swarmServer.Identifier!); } } } @@ -1352,17 +1368,17 @@ namespace Tgstation.Server.Host.Swarm /// /// Prepares a for swarm communication. /// - /// The the message is for, if null will be sent to swarm controller. + /// The the message is for. Must have and set. If , will be sent to swarm controller. /// The . /// The route on to use. /// The body if any. /// An optional override to the . /// A new . HttpRequestMessage PrepareSwarmRequest( - SwarmServerResponse swarmServer, + SwarmServerResponse? swarmServer, HttpMethod httpMethod, string route, - object body, + object? body, Guid? registrationIdOverride = null) { swarmServer ??= new SwarmServerResponse @@ -1375,7 +1391,7 @@ namespace Tgstation.Server.Host.Swarm "{method} {route} to swarm server {nodeIdOrAddress}", httpMethod, fullRoute, - swarmServer.Identifier ?? swarmServer.Address.ToString()); + swarmServer.Identifier ?? swarmServer.Address!.ToString()); var request = new HttpRequestMessage( httpMethod, @@ -1390,8 +1406,8 @@ namespace Tgstation.Server.Host.Swarm request.Headers.Add(SwarmConstants.RegistrationIdHeader, registrationIdOverride.Value.ToString()); else if (swarmController) { - lock (swarmServers) - if (registrationIdsAndTimes.TryGetValue(swarmServer.Identifier, out var registrationIdAndTime)) + lock (swarmServers!) + if (registrationIdsAndTimes!.TryGetValue(swarmServer.Identifier!, out var registrationIdAndTime)) request.Headers.Add(SwarmConstants.RegistrationIdHeader, registrationIdAndTime.RegistrationId.ToString()); } else if (controllerRegistration.HasValue) @@ -1422,7 +1438,7 @@ namespace Tgstation.Server.Host.Swarm logger.LogTrace("Starting HealthCheckLoop..."); try { - var nextForceHealthCheckTask = forceHealthCheckTcs.Task; + var nextForceHealthCheckTask = forceHealthCheckTcs!.Task; while (!cancellationToken.IsCancellationRequested) { TimeSpan delay; @@ -1500,13 +1516,14 @@ namespace Tgstation.Server.Host.Swarm /// /// The registration . /// The registered or if it does not exist. - string NodeIdentifierFromRegistration(Guid registrationId) + string? NodeIdentifierFromRegistration(Guid registrationId) { if (!swarmController) throw new InvalidOperationException("NodeIdentifierFromRegistration on node!"); - lock (swarmServers) + lock (swarmServers!) { + var registrationIdsAndTimes = this.registrationIdsAndTimes!; var exists = registrationIdsAndTimes.Any(x => x.Value.RegistrationId == registrationId); if (!exists) { diff --git a/src/Tgstation.Server.Host/Swarm/SwarmUpdateOperation.cs b/src/Tgstation.Server.Host/Swarm/SwarmUpdateOperation.cs index 3ebe8576e2..56d7496a26 100644 --- a/src/Tgstation.Server.Host/Swarm/SwarmUpdateOperation.cs +++ b/src/Tgstation.Server.Host/Swarm/SwarmUpdateOperation.cs @@ -60,7 +60,7 @@ namespace Tgstation.Server.Host.Swarm /// Initializes a new instance of the class. /// /// The value of . - /// An of the controller's current nodes as s. + /// An of the controller's current nodes as s. Must have and set. /// This is the variant for use by the controller. public SwarmUpdateOperation(Version targetVersion, IEnumerable currentNodes) : this(targetVersion)