Merge pull request #1794 from tgstation/1792-PlsStopKillingOD

`oom_score_adj` support. Test merge URL deduction
This commit is contained in:
Jordan Dominion
2024-03-03 17:55:13 -05:00
committed by GitHub
23 changed files with 185 additions and 34 deletions
+1
View File
@@ -157,6 +157,7 @@ docker run \
--network="host" \ # Not recommended, eases networking setup if your sql server is on the same machine
--name="tgs" \ # Name for the container
--cap-add=sys_nice \ # Recommended, allows TGS to lower the niceness of child processes if it sees fit
--cap-add=sys_resource \ # Recommended, allows TGS to not be killed by the OOM killer before its child processes
--init \ #Highly recommended, reaps potential zombie processes
-p 5000:5000 \ # Port bridge for accessing TGS, you can change this if you need
-p 0.0.0.0:<public game port>:<public game port> \ # Port bridge for accessing DreamDaemon
@@ -27,7 +27,7 @@
<!-- Usage: HTTP constants reference -->
<PackageReference Include="Microsoft.AspNetCore.Http.Extensions" Version="2.2.0" />
<!-- Usage: Decoding the 'nbf' property of JWTs -->
<PackageReference Include="Microsoft.IdentityModel.JsonWebTokens" Version="7.3.1" />
<PackageReference Include="Microsoft.IdentityModel.JsonWebTokens" Version="7.4.0" />
<!-- Usage: Primary JSON library -->
<PackageReference Include="Newtonsoft.Json" Version="13.0.3" />
<!-- Usage: Data model annotating -->
@@ -855,11 +855,12 @@ namespace Tgstation.Server.Host.Components.Deployment
var environment = await engineLock.LoadEnv(logger, true, cancellationToken);
var arguments = engineLock.FormatCompilerArguments($"{job.DmeName}.{DmeExtension}");
await using var dm = processExecutor.LaunchProcess(
await using var dm = await processExecutor.LaunchProcess(
engineLock.CompilerExePath,
ioManager.ResolvePath(
job.DirectoryName!.Value.ToString()),
arguments,
cancellationToken,
environment,
readStandardHandles: true,
noShellExecute: true);
@@ -241,10 +241,11 @@ namespace Tgstation.Server.Host.Components.Engine
async shortenedPath =>
{
var shortenedDeployPath = IOManager.ConcatPath(shortenedPath, DeployDir);
await using var buildProcess = ProcessExecutor.LaunchProcess(
await using var buildProcess = await ProcessExecutor.LaunchProcess(
dotnetPath,
shortenedPath,
$"run -c Release --project OpenDreamPackageTool -- --tgs -o {shortenedDeployPath}",
cancellationToken,
null,
null,
!GeneralConfiguration.OpenDreamSuppressInstallOutput,
@@ -283,10 +283,11 @@ namespace Tgstation.Server.Host.Components.Engine
try
{
// noShellExecute because we aren't doing runas shennanigans
await using var directXInstaller = processExecutor.LaunchProcess(
await using var directXInstaller = await processExecutor.LaunchProcess(
IOManager.ConcatPath(rbdx, "DXSETUP.exe"),
rbdx,
"/silent",
cancellationToken,
noShellExecute: true);
int exitCode;
@@ -93,7 +93,7 @@ namespace Tgstation.Server.Host.Components.Repository
Comment = parameters.Comment,
Number = parameters.Number,
TargetCommitSha = revisionToUse,
Url = pr?.HtmlUrl ?? errorMessage,
Url = pr?.HtmlUrl ?? $"https://github.com/{RemoteRepositoryOwner}/{RemoteRepositoryName}/pull/{parameters.Number}",
};
return testMerge;
@@ -85,7 +85,7 @@ namespace Tgstation.Server.Host.Components.Repository
Comment = parameters.Comment,
Number = parameters.Number,
TargetCommitSha = parameters.TargetCommitSha,
Url = ex.Message,
Url = $"https://gitlab.com/{RemoteRepositoryOwner}/{RemoteRepositoryName}/-/merge_requests/{parameters.Number}",
};
}
}
@@ -516,10 +516,11 @@ namespace Tgstation.Server.Host.Components.Session
? logFilePath
: null);
var process = processExecutor.LaunchProcess(
var process = await processExecutor.LaunchProcess(
engineLock.ServerExePath,
dmbProvider.Directory,
arguments,
cancellationToken,
environment,
logFilePath,
engineLock.HasStandardOutput,
@@ -761,7 +761,7 @@ namespace Tgstation.Server.Host.Components.StaticFiles
foreach (var scriptFile in scriptFiles)
{
logger.LogTrace("Running event script {scriptFile}...", scriptFile);
await using (var script = processExecutor.LaunchProcess(
await using (var script = await processExecutor.LaunchProcess(
ioManager.ConcatPath(resolvedScriptsDir, scriptFile),
resolvedScriptsDir,
String.Join(
@@ -778,6 +778,7 @@ namespace Tgstation.Server.Host.Components.StaticFiles
return $"\"{arg}\"";
})),
cancellationToken,
readStandardHandles: true,
noShellExecute: true))
using (cancellationToken.Register(() => script.Terminate()))
@@ -356,6 +356,7 @@ namespace Tgstation.Server.Host.Core
services.AddSingleton<IPostWriteHandler, PosixPostWriteHandler>();
services.AddSingleton<IProcessFeatures, PosixProcessFeatures>();
services.AddHostedService<PosixProcessFeatures>();
// PosixProcessFeatures also needs a IProcessExecutor for gcore
services.AddSingleton(x => new Lazy<IProcessExecutor>(() => x.GetRequiredService<IProcessExecutor>(), true));
@@ -221,14 +221,7 @@ namespace Tgstation.Server.Host.IO
/// <inheritdoc />
public async ValueTask<byte[]> ReadAllBytes(string path, CancellationToken cancellationToken)
{
path = ResolvePath(path);
await using var file = new FileStream(
path,
FileMode.Open,
FileAccess.Read,
FileShare.ReadWrite | FileShare.Delete,
DefaultBufferSize,
FileOptions.Asynchronous | FileOptions.SequentialScan);
await using var file = CreateAsyncSequentialReadStream(path);
byte[] buf;
buf = new byte[file.Length];
await file.ReadAsync(buf, cancellationToken);
@@ -261,6 +254,19 @@ namespace Tgstation.Server.Host.IO
FileOptions.Asynchronous | FileOptions.SequentialScan);
}
/// <inheritdoc />
public FileStream CreateAsyncSequentialReadStream(string path)
{
path = ResolvePath(path);
return new FileStream(
path,
FileMode.Open,
FileAccess.Read,
FileShare.ReadWrite | FileShare.Delete,
DefaultBufferSize,
FileOptions.Asynchronous | FileOptions.SequentialScan);
}
/// <inheritdoc />
public Task<IReadOnlyList<string>> GetDirectories(string path, CancellationToken cancellationToken) => Task.Factory.StartNew(
() =>
@@ -85,6 +85,7 @@ namespace Tgstation.Server.Host.IO
/// <param name="path">The path of the file to read.</param>
/// <param name="cancellationToken">A <see cref="CancellationToken"/> for the operation.</param>
/// <returns>A <see cref="ValueTask"/> that results in the contents of a file at <paramref name="path"/>.</returns>
/// <remarks>This function will fail to read files from the /proc filesystem on Linux.</remarks>
ValueTask<byte[]> ReadAllBytes(string path, CancellationToken cancellationToken);
/// <summary>
@@ -110,6 +111,13 @@ namespace Tgstation.Server.Host.IO
/// <returns>The open <see cref="FileStream"/>.</returns>
FileStream CreateAsyncSequentialWriteStream(string path);
/// <summary>
/// Creates an asynchronous <see cref="FileStream"/> for sequential reading.
/// </summary>
/// <param name="path">The path of the file to write, will be truncated.</param>
/// <returns>The open <see cref="FileStream"/>.</returns>
FileStream CreateAsyncSequentialReadStream(string path);
/// <summary>
/// Writes some <paramref name="contents"/> to a file at <paramref name="path"/> overwriting previous content.
/// </summary>
@@ -1,4 +1,6 @@
using System.Collections.Generic;
using System.Threading;
using System.Threading.Tasks;
namespace Tgstation.Server.Host.System
{
@@ -13,15 +15,17 @@ namespace Tgstation.Server.Host.System
/// <param name="fileName">The full path to the executable file.</param>
/// <param name="workingDirectory">The working directory for the <see cref="IProcess"/>.</param>
/// <param name="arguments">The arguments for the <see cref="IProcess"/>.</param>
/// <param name="cancellationToken">The <see cref="CancellationToken"/> for the operation.</param>
/// <param name="environment">A <see cref="IReadOnlyDictionary{TKey, TValue}"/> of environment variables to set.</param>
/// <param name="fileRedirect">File to write process output and error streams to. Requires <paramref name="readStandardHandles"/> to be <see langword="true"/>.</param>
/// <param name="readStandardHandles">If the process output and error streams should be read.</param>
/// <param name="noShellExecute">If shell execute should not be used. Must be set if <paramref name="readStandardHandles"/> is set.</param>
/// <returns>The new <see cref="IProcess"/>.</returns>
IProcess LaunchProcess(
/// <returns>A <see cref="ValueTask{TResult}"/> resulting in the new <see cref="IProcess"/>.</returns>
ValueTask<IProcess> LaunchProcess(
string fileName,
string workingDirectory,
string arguments,
CancellationToken cancellationToken,
IReadOnlyDictionary<string, string>? environment = null,
string? fileRedirect = null,
bool readStandardHandles = false,
@@ -18,11 +18,11 @@ namespace Tgstation.Server.Host.System
/// <summary>
/// Suspend a given <paramref name="process"/>.
/// </summary>
/// <param name="process">The <see cref="Process"/> to suspend.</param>
/// <param name="process">The <see cref="global::System.Diagnostics.Process"/> to suspend.</param>
void SuspendProcess(global::System.Diagnostics.Process process);
/// <summary>
/// Resume a given suspended <see cref="Process"/>.
/// Resume a given suspended <see cref="global::System.Diagnostics.Process"/>.
/// </summary>
/// <param name="process">The <see cref="Process"/> to suspended.</param>
void ResumeProcess(global::System.Diagnostics.Process process);
@@ -30,11 +30,19 @@ namespace Tgstation.Server.Host.System
/// <summary>
/// Create a dump file for a given <paramref name="process"/>.
/// </summary>
/// <param name="process">The <see cref="Process"/> to dump.</param>
/// <param name="process">The <see cref="global::System.Diagnostics.Process"/> to dump.</param>
/// <param name="outputFile">The full path to the output file.</param>
/// <param name="minidump">If a minidump should be taken as opposed to a full dump.</param>
/// <param name="cancellationToken">The <see cref="CancellationToken"/> for the operation.</param>
/// <returns>A <see cref="ValueTask"/> representing the running operation.</returns>
ValueTask CreateDump(global::System.Diagnostics.Process process, string outputFile, bool minidump, CancellationToken cancellationToken);
/// <summary>
/// Run events on starting a process.
/// </summary>
/// <param name="process">The <see cref="global::System.Diagnostics.Process"/> that was started.</param>
/// <param name="cancellationToken">The <see cref="CancellationToken"/> for the operation.</param>
/// <returns>A <see cref="ValueTask{TResult}"/> resulting in the <paramref name="process"/> ID.</returns>
ValueTask<int> HandleProcessStart(global::System.Diagnostics.Process process, CancellationToken cancellationToken);
}
}
@@ -1,7 +1,11 @@
using System;
using System.Globalization;
using System.IO;
using System.Text;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.Extensions.Hosting;
using Microsoft.Extensions.Logging;
using Mono.Unix;
using Mono.Unix.Native;
@@ -13,8 +17,18 @@ using Tgstation.Server.Host.Jobs;
namespace Tgstation.Server.Host.System
{
/// <inheritdoc />
sealed class PosixProcessFeatures : IProcessFeatures
sealed class PosixProcessFeatures : IProcessFeatures, IHostedService
{
/// <summary>
/// Difference from <see cref="baselineOomAdjust"/> to set our own oom_score_adj to. 1 higher host watchdog.
/// </summary>
const short SelfOomAdjust = 1;
/// <summary>
/// Difference from <see cref="baselineOomAdjust"/> to set the oom_score_adj of child processes to. 1 higher than ourselves.
/// </summary>
const short ChildProcessOomAdjust = SelfOomAdjust + 1;
/// <summary>
/// <see cref="Lazy{T}"/> loaded <see cref="IProcessExecutor"/>.
/// </summary>
@@ -30,6 +44,11 @@ namespace Tgstation.Server.Host.System
/// </summary>
readonly ILogger<PosixProcessFeatures> logger;
/// <summary>
/// The original value of oom_score_adj as read from the /proc/ filesystem. Inherited from parent process.
/// </summary>
short baselineOomAdjust;
/// <summary>
/// Initializes a new instance of the <see cref="PosixProcessFeatures"/> class.
/// </summary>
@@ -88,10 +107,11 @@ namespace Tgstation.Server.Host.System
string? output;
int exitCode;
await using (var gcoreProc = lazyLoadedProcessExecutor.Value.LaunchProcess(
await using (var gcoreProc = await lazyLoadedProcessExecutor.Value.LaunchProcess(
GCorePath,
Environment.CurrentDirectory,
$"{(!minidump ? "-a " : String.Empty)}-o {outputFile} {process.Id}",
cancellationToken,
readStandardHandles: true,
noShellExecute: true))
{
@@ -112,5 +132,85 @@ namespace Tgstation.Server.Host.System
var generatedGCoreFile = $"{outputFile}.{pid}";
await ioManager.MoveFile(generatedGCoreFile, outputFile, cancellationToken);
}
/// <inheritdoc />
public async ValueTask<int> HandleProcessStart(global::System.Diagnostics.Process process, CancellationToken cancellationToken)
{
ArgumentNullException.ThrowIfNull(process);
var pid = process.Id;
try
{
// make sure all processes we spawn are killed _before_ us
await AdjustOutOfMemoryScore(pid, ChildProcessOomAdjust, cancellationToken);
}
catch (Exception ex) when (ex is not OperationCanceledException)
{
logger.LogWarning(ex, "Failed to adjust OOM killer score for pid {pid}!", pid);
}
return pid;
}
/// <inheritdoc />
public async Task StartAsync(CancellationToken cancellationToken)
{
// let this all throw
string originalString;
{
// can't use ReadAllBytes here, /proc files have 0 length so the buffer is initialized to empty
// https://stackoverflow.com/questions/12237712/how-can-i-show-the-size-of-files-in-proc-it-should-not-be-size-zero
await using var fileStream = ioManager.CreateAsyncSequentialReadStream(
"/proc/self/oom_score_adj");
using var reader = new StreamReader(fileStream, Encoding.UTF8, leaveOpen: true);
originalString = await reader.ReadToEndAsync(cancellationToken);
}
var trimmedString = originalString.Trim();
logger.LogTrace("Original oom_score_adj is \"{original}\"", trimmedString);
var originalOomAdjust = Int16.Parse(trimmedString, CultureInfo.InvariantCulture);
baselineOomAdjust = Math.Clamp(originalOomAdjust, (short)-1000, (short)1000);
if (originalOomAdjust != baselineOomAdjust)
logger.LogWarning("oom_score_adj is at it's limit of 1000 (Clamped from {original}). TGS cannot guarantee the kill order of its parent/child processes!", originalOomAdjust);
else
logger.LogWarning("oom_score_adj is at it's limit of 1000. TGS cannot guarantee the kill order of its parent/child processes!");
try
{
// we do not want to be killed before the host watchdog
await AdjustOutOfMemoryScore(null, SelfOomAdjust, cancellationToken);
}
catch (Exception ex) when (ex is not OperationCanceledException)
{
logger.LogWarning(ex, "Could not increase oom_score_adj!");
}
}
/// <inheritdoc />
public Task StopAsync(CancellationToken cancellationToken) => Task.CompletedTask;
/// <summary>
/// Set oom_score_adj for a given <paramref name="pid"/>.
/// </summary>
/// <param name="pid">The <see cref="global::System.Diagnostics.Process.Id"/> or <see langword="null"/> to self adjust.</param>
/// <param name="adjustment">The value being written to the adjustment file.</param>
/// <param name="cancellationToken">The <see cref="CancellationToken"/> for the operation.</param>
/// <returns>A <see cref="ValueTask"/> representing the running operation.</returns>
ValueTask AdjustOutOfMemoryScore(int? pid, short adjustment, CancellationToken cancellationToken)
{
var adjustedValue = Math.Clamp(baselineOomAdjust + adjustment, -1000, 1000);
var pidStr = pid.HasValue
? pid.Value.ToString(CultureInfo.InvariantCulture)
: "self";
logger.LogTrace(
"Setting oom_score_adj of {pid} to {adjustment}...", pidStr, adjustedValue);
return ioManager.WriteAllBytes(
$"/proc/{pidStr}/oom_score_adj",
Encoding.UTF8.GetBytes(adjustedValue.ToString(CultureInfo.InvariantCulture)),
cancellationToken);
}
}
}
@@ -105,10 +105,11 @@ namespace Tgstation.Server.Host.System
}
/// <inheritdoc />
public IProcess LaunchProcess(
public async ValueTask<IProcess> LaunchProcess(
string fileName,
string workingDirectory,
string arguments,
CancellationToken cancellationToken,
IReadOnlyDictionary<string, string>? environment,
string? fileRedirect,
bool readStandardHandles,
@@ -174,7 +175,16 @@ namespace Tgstation.Server.Host.System
ExclusiveProcessLaunchLock.ExitReadLock();
}
pid = handle.Id;
try
{
pid = await processFeatures.HandleProcessStart(handle, cancellationToken);
}
catch
{
handle.Kill();
throw;
}
processStartTcs?.SetResult(pid);
}
catch (Exception ex)
@@ -31,10 +31,11 @@ namespace Tgstation.Server.Host.System
{
logger.LogInformation("Adding Windows Firewall exception for {path}...", exePath);
var arguments = $"advfirewall firewall add rule name=\"{exceptionName}\" program=\"{exePath}\" protocol=tcp dir=in enable=yes action=allow";
await using var netshProcess = processExecutor.LaunchProcess(
await using var netshProcess = await processExecutor.LaunchProcess(
"netsh.exe",
Environment.CurrentDirectory,
arguments,
cancellationToken,
readStandardHandles: true,
noShellExecute: true);
@@ -172,5 +172,9 @@ namespace Tgstation.Server.Host.System
DefaultIOManager.BlockingTaskCreationOptions,
TaskScheduler.Current);
}
/// <inheritdoc />
public ValueTask<int> HandleProcessStart(global::System.Diagnostics.Process process, CancellationToken cancellationToken)
=> ValueTask.FromResult((process ?? throw new ArgumentNullException(nameof(process))).Id);
}
}
@@ -63,11 +63,12 @@ namespace Tgstation.Server.Host.System.Tests
Mock.Of<IIOManager>(),
loggerFactory.CreateLogger<ProcessExecutor>(),
loggerFactory);
await using var subProc = processExecutor
await using var subProc = await processExecutor
.LaunchProcess(
"dotnet",
pathToSignalTestApp,
$"run -c {CurrentConfig} --no-build",
CancellationToken.None,
null,
null,
true,
@@ -731,7 +731,7 @@ namespace Tgstation.Server.Tests.Live.Instance
var features = new PosixProcessFeatures(
new Lazy<IProcessExecutor>(Mock.Of<IProcessExecutor>()),
Mock.Of<IIOManager>(),
new DefaultIOManager(),
Mock.Of<ILogger<PosixProcessFeatures>>());
features.SuspendProcess(proc);
@@ -798,7 +798,7 @@ namespace Tgstation.Server.Tests.Live.Instance
executor = new ProcessExecutor(
RuntimeInformation.IsOSPlatform(OSPlatform.Windows)
? new WindowsProcessFeatures(Mock.Of<ILogger<WindowsProcessFeatures>>())
: new PosixProcessFeatures(new Lazy<IProcessExecutor>(() => executor), Mock.Of<IIOManager>(), Mock.Of<ILogger<PosixProcessFeatures>>()),
: new PosixProcessFeatures(new Lazy<IProcessExecutor>(() => executor), new DefaultIOManager(), Mock.Of<ILogger<PosixProcessFeatures>>()),
Mock.Of<IIOManager>(),
Mock.Of<ILogger<ProcessExecutor>>(),
LoggerFactory.Create(x => { }));
@@ -1100,10 +1100,11 @@ namespace Tgstation.Server.Tests.Live
async ValueTask RunGitCommand(string args)
{
await using var gitRemoteOriginFixProc = processExecutor.LaunchProcess(
await using var gitRemoteOriginFixProc = await processExecutor.LaunchProcess(
"git",
repoPath,
args,
cancellationToken,
null,
null,
true,
@@ -28,7 +28,7 @@ namespace Tgstation.Server.Tests
Mock.Of<ILogger<ProcessExecutor>>(),
loggerFactory);
await using var process = processExecutor.LaunchProcess("test." + platformIdentifier.ScriptFileExtension, ".", string.Empty, null, null, true, true);
await using var process = await processExecutor.LaunchProcess("test." + platformIdentifier.ScriptFileExtension, ".", string.Empty, CancellationToken.None, null, null, true, true);
using var cts = new CancellationTokenSource();
cts.CancelAfter(3000);
var exitCode = await process.Lifetime.WaitAsync(cts.Token);
@@ -63,7 +63,7 @@ namespace Tgstation.Server.Tests
File.Delete(tempFile);
try
{
await using (var process = processExecutor.LaunchProcess("test." + platformIdentifier.ScriptFileExtension, ".", string.Empty, null, tempFile, true, true))
await using (var process = await processExecutor.LaunchProcess("test." + platformIdentifier.ScriptFileExtension, ".", string.Empty, CancellationToken.None, null, tempFile, true, true))
{
using var cts = new CancellationTokenSource();
cts.CancelAfter(3000);
+3 -2
View File
@@ -208,7 +208,7 @@ namespace Tgstation.Server.Tests
? new WindowsProcessFeatures(Mock.Of<ILogger<WindowsProcessFeatures>>())
: new PosixProcessFeatures(
new Lazy<IProcessExecutor>(() => null),
Mock.Of<IIOManager>(),
new DefaultIOManager(),
loggerFactory.CreateLogger<PosixProcessFeatures>()),
Mock.Of<IIOManager>(),
loggerFactory.CreateLogger<ProcessExecutor>(),
@@ -498,10 +498,11 @@ namespace Tgstation.Server.Tests
try
{
await using var process = processExecutor.LaunchProcess(
await using var process = await processExecutor.LaunchProcess(
ddPath,
Environment.CurrentDirectory,
"fake.dmb -map-threads 3 -close",
CancellationToken.None,
null,
null,
true,