diff --git a/src/Tgstation.Server.Host/Components/InstanceFactory.cs b/src/Tgstation.Server.Host/Components/InstanceFactory.cs index c9b891f087..6f68acec65 100644 --- a/src/Tgstation.Server.Host/Components/InstanceFactory.cs +++ b/src/Tgstation.Server.Host/Components/InstanceFactory.cs @@ -139,6 +139,11 @@ namespace Tgstation.Server.Host.Components /// readonly IRemoteDeploymentManagerFactory remoteDeploymentManagerFactory; + /// + /// The for the . + /// + readonly GeneralConfiguration generalConfiguration; + /// /// The for the . /// @@ -177,6 +182,7 @@ namespace Tgstation.Server.Host.Components /// The value of . /// The value of . /// The value of . + /// The containing the value of . /// The containing the value of . public InstanceFactory( IIOManager ioManager, @@ -201,6 +207,7 @@ namespace Tgstation.Server.Host.Components IFileTransferTicketProvider fileTransferService, IGitRemoteFeaturesFactory gitRemoteFeaturesFactory, IRemoteDeploymentManagerFactory remoteDeploymentManagerFactory, + IOptions generalConfigurationOptions, IOptions sessionConfigurationOptions) { this.ioManager = ioManager ?? throw new ArgumentNullException(nameof(ioManager)); @@ -225,6 +232,7 @@ namespace Tgstation.Server.Host.Components this.fileTransferService = fileTransferService ?? throw new ArgumentNullException(nameof(fileTransferService)); this.gitRemoteFeaturesFactory = gitRemoteFeaturesFactory ?? throw new ArgumentNullException(nameof(gitRemoteFeaturesFactory)); this.remoteDeploymentManagerFactory = remoteDeploymentManagerFactory ?? throw new ArgumentNullException(nameof(remoteDeploymentManagerFactory)); + generalConfiguration = generalConfigurationOptions?.Value ?? throw new ArgumentNullException(nameof(generalConfigurationOptions)); sessionConfiguration = sessionConfigurationOptions?.Value ?? throw new ArgumentNullException(nameof(sessionConfigurationOptions)); } #pragma warning restore CA1502 @@ -266,7 +274,8 @@ namespace Tgstation.Server.Host.Components postWriteHandler, platformIdentifier, fileTransferService, - loggerFactory.CreateLogger()); + loggerFactory.CreateLogger(), + generalConfiguration); var eventConsumer = new EventConsumer(configuration); var repoManager = new RepositoryManager( repositoryFactory, @@ -276,7 +285,8 @@ namespace Tgstation.Server.Host.Components postWriteHandler, gitRemoteFeaturesFactory, loggerFactory.CreateLogger(), - loggerFactory.CreateLogger()); + loggerFactory.CreateLogger(), + generalConfiguration); try { var byond = new ByondManager(byondIOManager, byondInstaller, eventConsumer, loggerFactory.CreateLogger()); diff --git a/src/Tgstation.Server.Host/Components/Repository/Repository.cs b/src/Tgstation.Server.Host/Components/Repository/Repository.cs index 8bdbadc48e..62e41c67c2 100644 --- a/src/Tgstation.Server.Host/Components/Repository/Repository.cs +++ b/src/Tgstation.Server.Host/Components/Repository/Repository.cs @@ -12,6 +12,7 @@ using Microsoft.Extensions.Logging; using Tgstation.Server.Api.Models; using Tgstation.Server.Api.Models.Internal; using Tgstation.Server.Host.Components.Events; +using Tgstation.Server.Host.Configuration; using Tgstation.Server.Host.IO; using Tgstation.Server.Host.Jobs; @@ -106,6 +107,11 @@ namespace Tgstation.Server.Host.Components.Repository /// readonly ILogger logger; + /// + /// The for the . + /// + readonly GeneralConfiguration generalConfiguration; + /// /// to be taken when is called. /// @@ -127,6 +133,7 @@ namespace Tgstation.Server.Host.Components.Repository /// The value of . /// The to provide the value of . /// The value of . + /// The value of . /// The value if . public Repository( LibGit2Sharp.IRepository libGitRepo, @@ -137,6 +144,7 @@ namespace Tgstation.Server.Host.Components.Repository IPostWriteHandler postWriteHandler, IGitRemoteFeaturesFactory gitRemoteFeaturesFactory, ILogger logger, + GeneralConfiguration generalConfiguration, Action onDispose) { this.libGitRepo = libGitRepo ?? throw new ArgumentNullException(nameof(libGitRepo)); @@ -149,6 +157,7 @@ namespace Tgstation.Server.Host.Components.Repository throw new ArgumentNullException(nameof(gitRemoteFeaturesFactory)); this.logger = logger ?? throw new ArgumentNullException(nameof(logger)); + this.generalConfiguration = generalConfiguration ?? throw new ArgumentNullException(nameof(generalConfiguration)); this.onDispose = onDispose ?? throw new ArgumentNullException(nameof(onDispose)); gitRemoteFeatures = gitRemoteFeaturesFactory.CreateGitRemoteFeatures(this); @@ -524,8 +533,6 @@ namespace Tgstation.Server.Host.Components.Repository throw new ArgumentNullException(nameof(path)); logger.LogTrace("Copying to {0}...", path); await ioMananger.CopyDirectory( - ioMananger.ResolvePath(), - path, new List { ".git" }, (src, dest) => { @@ -534,6 +541,9 @@ namespace Tgstation.Server.Host.Components.Repository return Task.CompletedTask; }, + ioMananger.ResolvePath(), + path, + generalConfiguration.GetCopyDirectoryTaskThrottle(), cancellationToken); } diff --git a/src/Tgstation.Server.Host/Components/Repository/RepositoryManager.cs b/src/Tgstation.Server.Host/Components/Repository/RepositoryManager.cs index ba78fb23c5..237053a5d0 100644 --- a/src/Tgstation.Server.Host/Components/Repository/RepositoryManager.cs +++ b/src/Tgstation.Server.Host/Components/Repository/RepositoryManager.cs @@ -7,6 +7,7 @@ using Microsoft.Extensions.Logging; using Tgstation.Server.Api.Models; using Tgstation.Server.Host.Components.Events; +using Tgstation.Server.Host.Configuration; using Tgstation.Server.Host.Core; using Tgstation.Server.Host.IO; using Tgstation.Server.Host.Jobs; @@ -62,6 +63,11 @@ namespace Tgstation.Server.Host.Components.Repository /// readonly ILogger logger; + /// + /// The for the . + /// + readonly GeneralConfiguration generalConfiguration; + /// /// Used for controlling single access to the . /// @@ -78,6 +84,7 @@ namespace Tgstation.Server.Host.Components.Repository /// The value of . /// The value of . /// The value of . + /// The value of . public RepositoryManager( ILibGit2RepositoryFactory repositoryFactory, ILibGit2Commands commands, @@ -86,7 +93,8 @@ namespace Tgstation.Server.Host.Components.Repository IPostWriteHandler postWriteHandler, IGitRemoteFeaturesFactory gitRemoteFeaturesFactory, ILogger repositoryLogger, - ILogger logger) + ILogger logger, + GeneralConfiguration generalConfiguration) { this.repositoryFactory = repositoryFactory ?? throw new ArgumentNullException(nameof(repositoryFactory)); this.commands = commands ?? throw new ArgumentNullException(nameof(commands)); @@ -96,6 +104,7 @@ namespace Tgstation.Server.Host.Components.Repository this.gitRemoteFeaturesFactory = gitRemoteFeaturesFactory ?? throw new ArgumentNullException(nameof(gitRemoteFeaturesFactory)); this.repositoryLogger = repositoryLogger ?? throw new ArgumentNullException(nameof(repositoryLogger)); this.logger = logger ?? throw new ArgumentNullException(nameof(logger)); + this.generalConfiguration = generalConfiguration ?? throw new ArgumentNullException(nameof(generalConfiguration)); semaphore = new SemaphoreSlim(1); } @@ -217,6 +226,7 @@ namespace Tgstation.Server.Host.Components.Repository postWriteHandler, gitRemoteFeaturesFactory, repositoryLogger, + generalConfiguration, () => { logger.LogTrace("Releasing semaphore due to Repository disposal..."); diff --git a/src/Tgstation.Server.Host/Components/StaticFiles/Configuration.cs b/src/Tgstation.Server.Host/Components/StaticFiles/Configuration.cs index 9ed5959137..2f8002e354 100644 --- a/src/Tgstation.Server.Host/Components/StaticFiles/Configuration.cs +++ b/src/Tgstation.Server.Host/Components/StaticFiles/Configuration.cs @@ -13,6 +13,7 @@ using Microsoft.Extensions.Logging; using Tgstation.Server.Api.Models; using Tgstation.Server.Api.Models.Response; using Tgstation.Server.Host.Components.Events; +using Tgstation.Server.Host.Configuration; using Tgstation.Server.Host.Core; using Tgstation.Server.Host.IO; using Tgstation.Server.Host.Jobs; @@ -111,6 +112,11 @@ namespace Tgstation.Server.Host.Components.StaticFiles /// readonly ILogger logger; + /// + /// The for . + /// + readonly GeneralConfiguration generalConfiguration; + /// /// The for . Also used as a . /// @@ -137,6 +143,7 @@ namespace Tgstation.Server.Host.Components.StaticFiles /// The value of . /// The value of . /// The value of . + /// The value of . public Configuration( IIOManager ioManager, ISynchronousIOManager synchronousIOManager, @@ -145,7 +152,8 @@ namespace Tgstation.Server.Host.Components.StaticFiles IPostWriteHandler postWriteHandler, IPlatformIdentifier platformIdentifier, IFileTransferTicketProvider fileTransferService, - ILogger logger) + ILogger logger, + GeneralConfiguration generalConfiguration) { this.ioManager = ioManager ?? throw new ArgumentNullException(nameof(ioManager)); this.synchronousIOManager = synchronousIOManager ?? throw new ArgumentNullException(nameof(synchronousIOManager)); @@ -155,6 +163,7 @@ namespace Tgstation.Server.Host.Components.StaticFiles this.platformIdentifier = platformIdentifier ?? throw new ArgumentNullException(nameof(platformIdentifier)); this.fileTransferService = fileTransferService ?? throw new ArgumentNullException(nameof(fileTransferService)); this.logger = logger ?? throw new ArgumentNullException(nameof(logger)); + this.generalConfiguration = generalConfiguration ?? throw new ArgumentNullException(nameof(generalConfiguration)); semaphore = new SemaphoreSlim(1); disposeCts = new CancellationTokenSource(); @@ -180,7 +189,13 @@ namespace Tgstation.Server.Host.Components.StaticFiles var dmeExistsTask = ioManager.FileExists(ioManager.ConcatPath(CodeModificationsSubdirectory, dmeFile), cancellationToken); var headFileExistsTask = ioManager.FileExists(ioManager.ConcatPath(CodeModificationsSubdirectory, CodeModificationsHeadFile), cancellationToken); var tailFileExistsTask = ioManager.FileExists(ioManager.ConcatPath(CodeModificationsSubdirectory, CodeModificationsTailFile), cancellationToken); - var copyTask = ioManager.CopyDirectory(CodeModificationsSubdirectory, destination, null, null, cancellationToken); + var copyTask = ioManager.CopyDirectory( + null, + null, + CodeModificationsSubdirectory, + destination, + generalConfiguration.GetCopyDirectoryTaskThrottle(), + cancellationToken); await Task.WhenAll(dmeExistsTask, headFileExistsTask, tailFileExistsTask, copyTask); diff --git a/src/Tgstation.Server.Host/Configuration/GeneralConfiguration.cs b/src/Tgstation.Server.Host/Configuration/GeneralConfiguration.cs index 7be48930ed..0bfab0901b 100644 --- a/src/Tgstation.Server.Host/Configuration/GeneralConfiguration.cs +++ b/src/Tgstation.Server.Host/Configuration/GeneralConfiguration.cs @@ -116,6 +116,11 @@ namespace Tgstation.Server.Host.Configuration /// public bool SkipAddingByondFirewallException { get; set; } + /// + /// A limit on the amount of tasks used for asynchronous I/O when copying directories during the deployment process as a multiplier to the machine's . Too few can significantly increase deployment times, too many can make TGS unresponsive and slowdown other I/O operations on the machine. + /// + public uint? DeploymentDirectoryCopyTasksPerCore { get; set; } + /// /// Initializes a new instance of the class. /// @@ -147,6 +152,26 @@ namespace Tgstation.Server.Host.Configuration CurrentConfigVersion); else logger.LogWarning("Your `ConfigVersion` is out-of-date. Please follow migration instructions from the TGS release notes."); + + if (DeploymentDirectoryCopyTasksPerCore == 0) + throw new InvalidOperationException( + $"{nameof(DeploymentDirectoryCopyTasksPerCore)} must be at least 1!"); + else if (GetCopyDirectoryTaskThrottle() < 1) + throw new InvalidOperationException( + $"{nameof(DeploymentDirectoryCopyTasksPerCore)} is too large for the CPU core count of {Environment.ProcessorCount} and overflows a 32-bit signed integer. Please lower the value!"); + } + + /// + /// Gets the total number of tasks that may run simultaneously during an asynchronous directory copy operation. + /// + /// The total number of tasks that may run simultaneously during an asynchronous directory copy operation. + public int? GetCopyDirectoryTaskThrottle() + { + if (!DeploymentDirectoryCopyTasksPerCore.HasValue) + return null; + + var taskThrottle = (uint)Environment.ProcessorCount * DeploymentDirectoryCopyTasksPerCore.Value; + return (int)taskThrottle; } } } diff --git a/src/Tgstation.Server.Host/IO/DefaultIOManager.cs b/src/Tgstation.Server.Host/IO/DefaultIOManager.cs index 6edc397308..75f0c66f1d 100644 --- a/src/Tgstation.Server.Host/IO/DefaultIOManager.cs +++ b/src/Tgstation.Server.Host/IO/DefaultIOManager.cs @@ -62,10 +62,11 @@ namespace Tgstation.Server.Host.IO /// public async Task CopyDirectory( - string src, - string dest, IEnumerable ignore, Func postCopyCallback, + string src, + string dest, + int? taskThrottle, CancellationToken cancellationToken) { if (src == null) @@ -73,10 +74,13 @@ namespace Tgstation.Server.Host.IO if (dest == null) throw new ArgumentNullException(nameof(src)); + if (taskThrottle.HasValue && taskThrottle < 1) + throw new ArgumentOutOfRangeException(nameof(taskThrottle), taskThrottle, "taskThrottle must be at least 1!"); + src = ResolvePath(src); dest = ResolvePath(dest); - using var semaphore = new SemaphoreSlim(100 * Environment.ProcessorCount); + using var semaphore = taskThrottle.HasValue ? new SemaphoreSlim(taskThrottle.Value) : null; await Task.WhenAll(CopyDirectoryImpl(src, dest, ignore, postCopyCallback, semaphore, cancellationToken)); } @@ -327,7 +331,7 @@ namespace Tgstation.Server.Host.IO /// The destination directory path. /// Files and folders to ignore at the root level. /// The optional callback called for each source/dest file pair post copy. - /// used to limit degree of parallelism. + /// Optional used to limit degree of parallelism. /// The for the operation. /// A of s representing the running operations. The first returned is always the necessary call to . IEnumerable CopyDirectoryImpl( @@ -377,7 +381,9 @@ namespace Tgstation.Server.Host.IO async Task CopyThisFile() { await subdirCreationTask; - using var lockContext = await SemaphoreSlimContext.Lock(semaphore, cancellationToken); + using var lockContext = semaphore != null + ? await SemaphoreSlimContext.Lock(semaphore, cancellationToken) + : null; await CopyFile(sourceFile, destFile, cancellationToken); if (postCopyCallback != null) await postCopyCallback(sourceFile, destFile); diff --git a/src/Tgstation.Server.Host/IO/IIOManager.cs b/src/Tgstation.Server.Host/IO/IIOManager.cs index f9275e1216..56a5343573 100644 --- a/src/Tgstation.Server.Host/IO/IIOManager.cs +++ b/src/Tgstation.Server.Host/IO/IIOManager.cs @@ -48,17 +48,19 @@ namespace Tgstation.Server.Host.IO /// /// Copies a directory from to . /// - /// The source directory path. - /// The destination directory path. /// Files and folders to ignore at the root level. /// The optional callback called for each source/dest file pair post copy. + /// The source directory path. + /// The destination directory path. + /// The optional maximum number of simultaneous tasks allowed to execute. /// The for the operation. /// A representing the running operation. Task CopyDirectory( - string src, - string dest, IEnumerable ignore, Func postCopyCallback, + string src, + string dest, + int? taskThrottle, CancellationToken cancellationToken); /// diff --git a/src/Tgstation.Server.Host/appsettings.yml b/src/Tgstation.Server.Host/appsettings.yml index a94f7e0d47..38fe9f57bd 100644 --- a/src/Tgstation.Server.Host/appsettings.yml +++ b/src/Tgstation.Server.Host/appsettings.yml @@ -12,6 +12,7 @@ General: ValidInstancePaths: HostApiDocumentation: false SkipAddingByondFirewallException: false + DeploymentDirectoryCopyTasksPerCore: 100 Session: HighPriorityLiveDreamDaemon: false LowPriorityDeploymentProcesses: true diff --git a/tests/Tgstation.Server.Host.Tests/IO/TestIOManager.cs b/tests/Tgstation.Server.Host.Tests/IO/TestIOManager.cs index d4a486b584..f92ffaa04d 100644 --- a/tests/Tgstation.Server.Host.Tests/IO/TestIOManager.cs +++ b/tests/Tgstation.Server.Host.Tests/IO/TestIOManager.cs @@ -1,5 +1,8 @@ using Microsoft.VisualStudio.TestTools.UnitTesting; +using Remora.Discord.API.Objects; + +using System; using System.IO; using System.Threading.Tasks; @@ -71,5 +74,108 @@ namespace Tgstation.Server.Host.IO.Tests throw; } } + + [TestMethod] + public async Task TestCopyDirectoryThrows() + { + int? throttle = null; + var tempPath1 = Guid.NewGuid().ToString(); + var tempPath2 = Guid.NewGuid().ToString(); + + await Assert.ThrowsExceptionAsync(() => ioManager.CopyDirectory( + null, + null, + null, + tempPath2, + throttle, + default)); + await Assert.ThrowsExceptionAsync(() => ioManager.CopyDirectory( + null, + null, + tempPath1, + null, + throttle, + default)); + await Assert.ThrowsExceptionAsync(() => ioManager.CopyDirectory( + null, + null, + null, + null, + throttle, + default)); + await Assert.ThrowsExceptionAsync(() => ioManager.CopyDirectory( + null, + null, + tempPath1, + tempPath2, + -1, + default)); + } + + [TestMethod] + public async Task TestCopyDirectoryOneTask() + { + await TestCopyDirectory(1); + } + + [TestMethod] + public async Task TestCopyDirectoryMaxTasks() + { + await TestCopyDirectory(Int32.MaxValue); + } + + [TestMethod] + public async Task TestCopyDirectoryUnlimitedTasks() + { + await TestCopyDirectory(null); + } + + async Task TestCopyDirectory(int? throttle) + { + var tempPath = Path.GetTempFileName(); + File.Delete(tempPath); + Directory.CreateDirectory(tempPath); + try + { + var tempPath2 = Path.GetTempFileName(); + File.Delete(tempPath2); + + await File.WriteAllTextAsync(Path.Combine(tempPath, "file.txt"), "asdf"); + var subDir = Path.Combine(tempPath, "subdir"); + Directory.CreateDirectory(subDir); + await File.WriteAllTextAsync(Path.Combine(subDir, "file2.txt"), "fdsa"); + + try + { + await ioManager.CopyDirectory( + null, + null, + tempPath, + tempPath2, + throttle, + default); + + Assert.IsTrue(Directory.Exists(tempPath2)); + var newFilePath = Path.Combine(tempPath2, "file.txt"); + Assert.IsTrue(File.Exists(newFilePath)); + var newFileText = await File.ReadAllTextAsync(newFilePath); + Assert.AreEqual("asdf", newFileText); + var newDirPath = Path.Combine(tempPath2, "subdir"); + Assert.IsTrue(Directory.Exists(newDirPath)); + var newFile2Path = Path.Combine(newDirPath, "file2.txt"); + Assert.IsTrue(File.Exists(newFile2Path)); + var newFile2Text = await File.ReadAllTextAsync(newFile2Path); + Assert.AreEqual("fdsa", newFile2Text); + } + finally + { + Directory.Delete(tempPath2, true); + } + } + finally + { + Directory.Delete(tempPath, true); + } + } } } diff --git a/tests/Tgstation.Server.Tests/Live/Instance/ConfigurationTest.cs b/tests/Tgstation.Server.Tests/Live/Instance/ConfigurationTest.cs index cb98a62cba..46a0772c05 100644 --- a/tests/Tgstation.Server.Tests/Live/Instance/ConfigurationTest.cs +++ b/tests/Tgstation.Server.Tests/Live/Instance/ConfigurationTest.cs @@ -106,15 +106,17 @@ namespace Tgstation.Server.Tests.Live.Instance var ioManager = new DefaultIOManager(); return Task.WhenAll( ioManager.CopyDirectory( + Enumerable.Empty(), + null, "../../../../DMAPI", ioManager.ConcatPath(instance.Path, "Repository", "tests", "DMAPI"), - Enumerable.Empty(), null, cancellationToken), ioManager.CopyDirectory( + Enumerable.Empty(), + null, "../../../../../src/DMAPI", ioManager.ConcatPath(instance.Path, "Repository", "src", "DMAPI"), - Enumerable.Empty(), null, cancellationToken) ); diff --git a/tests/Tgstation.Server.Tests/TestRepository.cs b/tests/Tgstation.Server.Tests/TestRepository.cs index 7366c43e44..9a8a42c92a 100644 --- a/tests/Tgstation.Server.Tests/TestRepository.cs +++ b/tests/Tgstation.Server.Tests/TestRepository.cs @@ -7,6 +7,7 @@ using Moq; using Tgstation.Server.Host.Components.Events; using Tgstation.Server.Host.Components.Repository; +using Tgstation.Server.Host.Configuration; using Tgstation.Server.Host.IO; using Tgstation.Server.Host.Jobs; using Tgstation.Server.Tests.Live; @@ -31,6 +32,7 @@ namespace Tgstation.Server.Tests Mock.Of(), Mock.Of(), Mock.Of>(), + new GeneralConfiguration(), () => { }); const string StartSha = "af4da8beb9f9b374b04a3cc4d65acca662e8cc1a";