Skip to content

Commit c61a186

Browse files
RakeshwarKRakeshwar Reddy Kambaiahgari
RakeshwarK
and
Rakeshwar Reddy Kambaiahgari
authored
Fix Wrk CPU affinity process invocation and ParametersOn condition evaluation (#690)
* wrk fix for cpu affinity and condition evaluation for ParametersOn * up version * wrk2 fix --------- Co-authored-by: Rakeshwar Reddy Kambaiahgari <rkambaiahgar@microsoft.com>
1 parent 3ccab7a commit c61a186

9 files changed

Lines changed: 152 additions & 30 deletions

File tree

‎VERSION‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
3.0.16
1+
3.0.17

‎src/VirtualClient/VirtualClient.Actions.UnitTests/Wrk/Wrk2ExecutorTest.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -469,7 +469,7 @@ public async Task WrkClientExecutorReturnsCorrectArguments()
469469
}
470470
else
471471
{
472-
Assert.AreEqual(arguments, $"bash {executor.Combine(directory, "runwrk.sh")} \"{results}\"");
472+
Assert.AreEqual(arguments, $"bash {executor.Combine(directory, "runwrk.sh")} {results}");
473473
string examplesDirectory = Path.Combine(Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location), "Examples", "Wrk");
474474
string outputPath = Path.Combine(examplesDirectory, @"wrkStandardExample1.txt");
475475
this.memoryProcess.StandardOutput = new ConcurrentBuffer(new StringBuilder(File.ReadAllText(outputPath)));

‎src/VirtualClient/VirtualClient.Actions.UnitTests/Wrk/WrkExecutorTest.cs‎

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ public async Task WrkClientExecutorRunsWorkloadWithCorrectArguments()
164164
}
165165
else
166166
{
167-
Assert.AreEqual(arguments, $"bash {executor.Combine(directory, "runwrk.sh")} \"{results}\"");
167+
Assert.AreEqual(arguments, $"bash {executor.Combine(directory, "runwrk.sh")} {results}");
168168
string examplesDirectory = Path.Combine(Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location), "Examples", "Wrk");
169169
string outputPath = Path.Combine(examplesDirectory, @"wrkStandardExample1.txt");
170170
this.memoryProcess.StandardOutput = new ConcurrentBuffer(new StringBuilder(File.ReadAllText(outputPath)));
@@ -612,6 +612,52 @@ public async Task GetWrkVersion_ReturnsNull_WhenVersionCannotBeParsed()
612612
this.mockFixture.Tracking.AssertCommandsExecuted(true, "sudo bash .* --version");
613613
}
614614

615+
[Test]
616+
public async Task WrkClientExecutorRunsWorkloadWithAffinityUsingCorrectQuoting()
617+
{
618+
string commandArgumentInput = @"--latency --threads 8 --connections 256 --duration 30s --timeout 10s http://1.2.3.4:9876/json --header ""Accept: application/json""";
619+
ClientInstance serverInstance = new ClientInstance(name: nameof(ClientRole.Server), ipAddress: "1.2.3.4", role: ClientRole.Server);
620+
ClientInstance clientInstance = new ClientInstance(name: nameof(ClientRole.Client), ipAddress: "5.6.7.8", role: ClientRole.Client);
621+
622+
string directory = @"/some/random/dir/name/";
623+
this.mockFixture.Setup(PlatformID.Unix, Architecture.X64, nameof(State));
624+
this.mockFixture.Layout = new EnvironmentLayout(new List<ClientInstance>() { serverInstance, clientInstance });
625+
this.mockFixture.Parameters = new Dictionary<string, IConvertible>()
626+
{
627+
{ "CommandArguments", commandArgumentInput },
628+
{ "Scenario", "affinity_test" },
629+
{ "ToolName", "wrk" },
630+
{ "PackageName", "wrk" },
631+
{ "BindToCores", true },
632+
{ "CoreAffinity", "8-15" },
633+
{ "TargetService", "server" }
634+
};
635+
636+
TestWrkExecutor executor = new TestWrkExecutor(this.mockFixture);
637+
executor.PackageDirectory = directory;
638+
639+
this.mockFixture.FileSystem
640+
.Setup(x => x.File.Exists(It.IsAny<string>()))
641+
.Returns(true);
642+
643+
string examplesDirectory = Path.Combine(Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location), "Examples", "Wrk");
644+
string wrkOutput = File.ReadAllText(Path.Combine(examplesDirectory, @"wrkStandardExample1.txt"));
645+
646+
this.mockFixture
647+
.TrackProcesses()
648+
.SetupProcessOutput("--version", "wrk 4.2.0 [epoll] Copyright (C) 2012 Will Glozer")
649+
.SetupProcessOutput("numactl", wrkOutput);
650+
651+
string result = executor.GetCommandLineArguments();
652+
await executor.ExecuteWorkloadAsync(result, workingDir: directory).ConfigureAwait(false);
653+
654+
// The affinity path uses GetAffinityProcessInfo which sets numactl as the
655+
// executable, then CreateElevatedProcess wraps it with sudo to ensure
656+
// ulimit and process elevation work correctly (matching non-affinity path).
657+
string scriptPath = Regex.Escape(executor.Combine(directory, WrkExecutor.WrkRunShell));
658+
this.mockFixture.Tracking.AssertCommandsExecuted(true,
659+
$@"sudo numactl -C 8-15 bash {scriptPath} {Regex.Escape(commandArgumentInput)}");
660+
}
615661

616662
public void SetUpWorkloadOutput()
617663
{

‎src/VirtualClient/VirtualClient.Actions/Examples/ExampleWorkloadWithAffinityExecutor.cs‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -240,15 +240,17 @@ private Task<string> ExecuteWorkloadAsync(string commandArguments, EventContext
240240
}
241241
else
242242
{
243-
// Linux: Wrap command with numactl for CPU binding
243+
// Linux: Wrap command with numactl for CPU binding.
244+
// Pass null for command and include the executable path in the arguments
245+
// so that GetCommandWithAffinity returns "numactl -C {cores} {exe} {args}"
246+
// wrapped in double quotes for bash -c.
247+
string fullCommandLine = $"{this.WorkloadExecutablePath} {commandArguments}";
244248
LinuxProcessAffinityConfiguration linuxConfig = (LinuxProcessAffinityConfiguration)affinityConfig;
245-
string fullCommand = linuxConfig.GetCommandWithAffinity(
246-
this.WorkloadExecutablePath,
247-
commandArguments);
249+
string wrappedCommand = linuxConfig.GetCommandWithAffinity(null, fullCommandLine);
248250

249251
workloadProcess = this.processManager.CreateProcess(
250252
"/bin/bash",
251-
$"-c \"{fullCommand}\"",
253+
$"-c {wrappedCommand}",
252254
this.WorkloadPackage.Path);
253255
}
254256
}

‎src/VirtualClient/VirtualClient.Actions/Wrk/WrkExecutor.cs‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright (c) Microsoft Corporation.
1+
// Copyright (c) Microsoft Corporation.
22
// Licensed under the MIT License.
33

44
namespace VirtualClient.Actions
@@ -427,18 +427,17 @@ protected async Task ExecuteWorkloadAsync(string commandArguments, string workin
427427

428428
relatedContext.AddContext("affinityMask", affinityConfig.ToString());
429429

430-
string fullCommandLine = $"{command} \"{commandArguments}\"";
431430
LinuxProcessAffinityConfiguration linuxConfig = (LinuxProcessAffinityConfiguration)affinityConfig;
432-
string wrappedCommand = linuxConfig.GetCommandWithAffinity(null, fullCommandLine);
433431

434-
process = this.SystemManagement.ProcessManager.CreateProcess(
435-
"/bin/bash",
436-
$"-c {wrappedCommand}",
437-
workingDir);
432+
// Use direct numactl invocation to avoid bash -c shell wrapping.
433+
// The wrk arguments contain embedded double quotes (e.g., --header "Accept: ..."),
434+
// which break the nested quoting in bash -c "numactl ... \"args\"" patterns.
435+
var (executable, numaArguments) = linuxConfig.GetAffinityProcessInfo(command, commandArguments);
436+
process = this.SystemManagement.ProcessManager.CreateElevatedProcess(this.Platform, executable, numaArguments, workingDir);
438437
}
439438
else
440439
{
441-
process = await this.ExecuteCommandAsync(command, commandArguments: $"\"{commandArguments}\"", workingDir, telemetryContext, cancellationToken, runElevated: true);
440+
process = await this.ExecuteCommandAsync(command, commandArguments: $"{commandArguments}", workingDir, telemetryContext, cancellationToken, runElevated: true);
442441
}
443442

444443
using (process)

‎src/VirtualClient/VirtualClient.Actions/Wrk/runwrk.sh‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,2 @@
11
ulimit -n 65535
2-
./wrk $1
2+
./wrk "$@"

‎src/VirtualClient/VirtualClient.Common.UnitTests/ProcessAffinity/LinuxProcessAffinityConfigurationTests.cs‎

Lines changed: 52 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ public void LinuxProcessAffinityConfigurationGeneratesCorrectNumactlCommandForSi
8383
LinuxProcessAffinityConfiguration config = new LinuxProcessAffinityConfiguration(new[] { 0 });
8484

8585
string command = config.GetCommandWithAffinity(null, "myworkload --arg1 --arg2");
86-
86+
8787
Assert.AreEqual("\"numactl -C 0 myworkload --arg1 --arg2\"", command);
8888
}
8989

@@ -93,7 +93,7 @@ public void LinuxProcessAffinityConfigurationGeneratesCorrectNumactlCommandForMu
9393
LinuxProcessAffinityConfiguration config = new LinuxProcessAffinityConfiguration(new[] { 0, 1, 2 });
9494

9595
string command = config.GetCommandWithAffinity(null, "myworkload --arg1 --arg2");
96-
96+
9797
Assert.AreEqual("\"numactl -C 0-2 myworkload --arg1 --arg2\"", command);
9898
}
9999

@@ -103,7 +103,7 @@ public void LinuxProcessAffinityConfigurationGeneratesCorrectNumactlCommandWithE
103103
LinuxProcessAffinityConfiguration config = new LinuxProcessAffinityConfiguration(new[] { 1, 3, 5 });
104104

105105
string command = config.GetCommandWithAffinity(null, "myworkload");
106-
106+
107107
Assert.AreEqual("\"numactl -C 1,3,5 myworkload\"", command);
108108
}
109109

@@ -163,11 +163,58 @@ public void LinuxProcessAffinityConfigurationHandlesUnsortedCores()
163163
{
164164
// Cores should be sorted before optimization
165165
LinuxProcessAffinityConfiguration config = new LinuxProcessAffinityConfiguration(new[] { 5, 0, 2, 1, 3 });
166-
166+
167167
string command = config.GetCommandWithAffinity("test", null);
168-
168+
169169
// Should sort and optimize: 0-3,5
170170
Assert.IsTrue(command.Contains("-C 0-3,5"));
171171
}
172+
173+
[Test]
174+
public void GetAffinityProcessInfoReturnsNumactlAsExecutable()
175+
{
176+
LinuxProcessAffinityConfiguration config = new LinuxProcessAffinityConfiguration(new[] { 0 });
177+
178+
var (executable, arguments) = config.GetAffinityProcessInfo("mycommand");
179+
180+
Assert.AreEqual("numactl", executable);
181+
}
182+
183+
[Test]
184+
public void GetAffinityProcessInfoReturnsCorrectArgumentsWithCommand()
185+
{
186+
LinuxProcessAffinityConfiguration config = new LinuxProcessAffinityConfiguration(new[] { 8, 9, 10, 11, 12, 13, 14, 15 });
187+
188+
var (executable, arguments) = config.GetAffinityProcessInfo("bash /path/to/script.sh", "--latency --threads 64");
189+
190+
Assert.AreEqual("numactl", executable);
191+
Assert.AreEqual("-C 8-15 bash /path/to/script.sh --latency --threads 64", arguments);
192+
}
193+
194+
[Test]
195+
public void GetAffinityProcessInfoReturnsCorrectArgumentsWithoutArguments()
196+
{
197+
LinuxProcessAffinityConfiguration config = new LinuxProcessAffinityConfiguration(new[] { 0, 1, 2 });
198+
199+
var (executable, arguments) = config.GetAffinityProcessInfo("bash /path/to/script.sh");
200+
201+
Assert.AreEqual("numactl", executable);
202+
Assert.AreEqual("-C 0-2 bash /path/to/script.sh", arguments);
203+
}
204+
205+
[Test]
206+
public void GetAffinityProcessInfoHandlesArgumentsWithEmbeddedDoubleQuotes()
207+
{
208+
LinuxProcessAffinityConfiguration config = new LinuxProcessAffinityConfiguration(new[] { 0, 1 });
209+
210+
var (executable, arguments) = config.GetAffinityProcessInfo(
211+
"bash /path/runwrk.sh",
212+
"--latency --header \"Accept: application/json\"");
213+
214+
Assert.AreEqual("numactl", executable);
215+
Assert.AreEqual(
216+
"-C 0,1 bash /path/runwrk.sh --latency --header \"Accept: application/json\"",
217+
arguments);
218+
}
172219
}
173220
}

‎src/VirtualClient/VirtualClient.Common/ProcessAffinity/LinuxProcessAffinityConfiguration.cs‎

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,13 +36,34 @@ public string NumactlCoreSpec
3636
/// <summary>
3737
/// Wraps a command with numactl to apply CPU affinity.
3838
/// Returns the full bash command string ready for execution.
39+
/// The returned string is wrapped in double quotes so that it can be passed
40+
/// as a single argument to "bash -c" (e.g., bash -c "numactl -C 0,1 command args").
3941
/// </summary>
4042
/// <param name="command">The command to wrap.</param>
4143
/// <param name="arguments">Optional arguments for the command.</param>
42-
/// <returns>The complete command string with numactl wrapper (e.g., "bash -c \"numactl -C 0,1 redis-server --port 6379\"").</returns>
44+
/// <returns>The complete command string with numactl wrapper (e.g., "\"numactl -C 0,1 redis-server --port 6379\"").</returns>
4345
public string GetCommandWithAffinity(string command, string arguments = null)
4446
{
45-
return string.IsNullOrEmpty(command) ? $"\"numactl -C {this.NumactlCoreSpec} {arguments}\"" : $"{command} \"numactl -C {this.NumactlCoreSpec} {arguments}\"";
47+
return string.IsNullOrEmpty(command)
48+
? $"\"numactl -C {this.NumactlCoreSpec} {arguments}\""
49+
: $"{command} \"numactl -C {this.NumactlCoreSpec} {arguments}\"";
50+
}
51+
52+
/// <summary>
53+
/// Gets the numactl executable name and arguments for direct process invocation
54+
/// without a bash -c shell wrapper. This approach avoids double-quote escaping
55+
/// issues when arguments contain embedded quotes (e.g., HTTP headers).
56+
/// </summary>
57+
/// <param name="command">The command to run under numactl (e.g., "bash /path/script.sh").</param>
58+
/// <param name="arguments">Optional arguments for the command.</param>
59+
/// <returns>A tuple of (Executable, Arguments) for use with ProcessManager.CreateProcess.</returns>
60+
public (string Executable, string Arguments) GetAffinityProcessInfo(string command, string arguments = null)
61+
{
62+
string numaArgs = string.IsNullOrWhiteSpace(arguments)
63+
? $"-C {this.NumactlCoreSpec} {command}"
64+
: $"-C {this.NumactlCoreSpec} {command} {arguments}";
65+
66+
return ("numactl", numaArgs);
4667
}
4768

4869
/// <summary>

‎src/VirtualClient/VirtualClient.Contracts/ExecutionProfileExtensions.cs‎

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -55,21 +55,28 @@ public static async Task EvaluateConditionalParametersAsync(this ExecutionProfil
5555
$"Invalid '{nameof(profile.ParametersOn)}' configuration. A '{conditionKey}' must be defined in each '{nameof(profile.ParametersOn)}' section.");
5656
}
5757

58-
// Parameters in ParametersOn sections take priority over the profile's default parameters.
59-
var conditionalParameters = new Dictionary<string, IConvertible>(profileParameters, StringComparer.OrdinalIgnoreCase);
60-
conditionalParameters.AddRange(profileConditionalParameters, true);
58+
// Evaluate the condition using only the original profile parameters so that
59+
// override values from the ParametersOn section do not affect the condition result.
60+
var conditionContext = new Dictionary<string, IConvertible>(profileParameters, StringComparer.OrdinalIgnoreCase)
61+
{
62+
[conditionKey] = condition
63+
};
6164

62-
await evaluator.EvaluateAsync(dependencies, conditionalParameters);
65+
await evaluator.EvaluateAsync(dependencies, conditionContext);
6366

64-
if (!bool.TryParse(conditionalParameters[conditionKey].ToString(), out bool conditionMatches))
67+
if (!bool.TryParse(conditionContext[conditionKey].ToString(), out bool conditionMatches))
6568
{
6669
throw new SchemaException(
6770
$"Invalid '{nameof(profile.ParametersOn)}' configuration. A '{conditionKey}' must always evaluate to true or false.");
6871
}
6972

7073
if (conditionMatches)
7174
{
72-
profile.Parameters.AddRange(conditionalParameters.Where(p => p.Key != conditionKey), true);
75+
// Merge override parameters and re-evaluate for expression resolution.
76+
var resolvedParameters = new Dictionary<string, IConvertible>(profileParameters, StringComparer.OrdinalIgnoreCase);
77+
resolvedParameters.AddRange(profileConditionalParameters.Where(p => p.Key != conditionKey), true);
78+
await evaluator.EvaluateAsync(dependencies, resolvedParameters);
79+
profile.Parameters.AddRange(resolvedParameters.Where(p => p.Key != conditionKey), true);
7380
break;
7481
}
7582
}

0 commit comments

Comments
 (0)