Skip to content

Commit ad1e055

Browse files
brdeyobrdeyo
committed
Fix bug in server-side network workloads related to timeout race condition preventing metric capture.
1 parent 8fa26d5 commit ad1e055

23 files changed

+286
-357
lines changed

‎.pipelines/azure-pipelines-linux.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ resources:
1616
options: --entrypoint=""
1717

1818
variables:
19-
VcVersion : 1.14.15
19+
VcVersion : 1.14.16
2020
ROOT: $(Build.SourcesDirectory)
2121
CDP_DEFINITION_BUILD_COUNT: $[counter('', 0)] # needed for onebranch.pipeline.version task https://aka.ms/obpipelines/versioning
2222
ENABLE_PRS_DELAYSIGN: 1

‎.pipelines/azure-pipelines.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ pool:
1818
vmImage: windows-latest
1919

2020
variables:
21-
VcVersion : 1.14.15
21+
VcVersion : 1.14.16
2222
ROOT: $(Build.SourcesDirectory)
2323
CDP_DEFINITION_BUILD_COUNT: $[counter('', 0)] # needed for onebranch.pipeline.version task https://aka.ms/obpipelines/versioning
2424
ENABLE_PRS_DELAYSIGN: 1

‎src/VirtualClient/VirtualClient.Actions.UnitTests/Network/NTttcp/NTttcpExecutorTests.cs

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,57 @@ public async Task NTttcpExecutorClientExecutesAsExpected()
124124
Assert.AreEqual(2, processExecuted);
125125
}
126126

127+
[Test]
128+
public async Task NTttcpExecutorClientWillNotExitOnCancellationRequestUntilTheResultsAreCaptured()
129+
{
130+
NetworkingWorkloadExecutorTests.TestNetworkingWorkloadExecutor networkingWorkloadExecutor = new NetworkingWorkloadExecutorTests.TestNetworkingWorkloadExecutor(this.mockFixture.Dependencies, this.mockFixture.Parameters);
131+
await networkingWorkloadExecutor.OnInitialize.Invoke(EventContext.None, CancellationToken.None);
132+
133+
int processExecuted = 0;
134+
this.mockFixture.ProcessManager.OnCreateProcess = (file, arguments, workingDirectory) =>
135+
{
136+
InMemoryProcess process = new InMemoryProcess()
137+
{
138+
StartInfo = new System.Diagnostics.ProcessStartInfo()
139+
{
140+
FileName = file,
141+
Arguments = arguments,
142+
},
143+
OnHasExited = () => true,
144+
ExitCode = 0,
145+
OnStart = () => true,
146+
StandardOutput = new VirtualClient.Common.ConcurrentBuffer()
147+
};
148+
149+
if (!arguments.Contains("chmod"))
150+
{
151+
processExecuted++;
152+
this.networkingWorkloadState.ToolState = NetworkingWorkloadToolState.Stopped;
153+
var expectedStateItem = new Item<NetworkingWorkloadState>(nameof(NetworkingWorkloadState), this.networkingWorkloadState);
154+
155+
this.mockFixture.ApiClient.Setup(client => client.GetStateAsync(It.IsAny<string>(), It.IsAny<CancellationToken>(), It.IsAny<IAsyncPolicy<HttpResponseMessage>>()))
156+
.ReturnsAsync(this.mockFixture.CreateHttpResponse(HttpStatusCode.OK, expectedStateItem));
157+
}
158+
159+
string standardOutput = null;
160+
if (file.Contains("sysctl"))
161+
{
162+
string currentDirectory = Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location);
163+
standardOutput = File.ReadAllText(Path.Combine(currentDirectory, "Examples", "NTttcp", "sysctlExampleOutput.txt"));
164+
process.StandardOutput.Append(standardOutput);
165+
}
166+
167+
return process;
168+
};
169+
170+
TestNTttcpExecutor component = new TestNTttcpExecutor(this.mockFixture.Dependencies, this.mockFixture.Parameters);
171+
172+
await component.ExecuteAsync(CancellationToken.None).ConfigureAwait(false);
173+
174+
//Process 1: Ntttcp, Process 2: Sysctl
175+
Assert.AreEqual(2, processExecuted);
176+
}
177+
127178
[Test]
128179
public async Task NTttcpExecutorServerExecutesAsExpected()
129180
{

‎src/VirtualClient/VirtualClient.Actions/Network/NetworkingWorkload/CPS/CPSClientExecutor.cs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ public class CPSClientExecutor : CPSExecutor
2525
public CPSClientExecutor(IServiceCollection dependencies, IDictionary<string, IConvertible> parameters)
2626
: base(dependencies, parameters)
2727
{
28-
this.WorkloadEmitsResults = true;
2928
}
3029

3130
/// <summary>

‎src/VirtualClient/VirtualClient.Actions/Network/NetworkingWorkload/CPS/CPSExecutor.cs

Lines changed: 23 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ namespace VirtualClient.Actions.NetworkPerformance
55
{
66
using System;
77
using System.Collections.Generic;
8-
using System.Diagnostics;
98
using System.IO.Abstractions;
109
using System.Threading;
1110
using System.Threading.Tasks;
@@ -26,9 +25,7 @@ namespace VirtualClient.Actions.NetworkPerformance
2625
[UnixCompatible]
2726
public class CPSExecutor : NetworkingWorkloadToolExecutor
2827
{
29-
private const string OutputFileName = "cps-results.xml";
3028
private IFileSystem fileSystem;
31-
private string results;
3229

3330
/// <summary>
3431
/// Initializes a new instance of the <see cref="CPSClientExecutor"/> class.
@@ -38,7 +35,6 @@ public class CPSExecutor : NetworkingWorkloadToolExecutor
3835
public CPSExecutor(IServiceCollection dependencies, IDictionary<string, IConvertible> parameters)
3936
: base(dependencies, parameters)
4037
{
41-
this.WorkloadEmitsResults = true;
4238
this.fileSystem = dependencies.GetService<IFileSystem>();
4339
this.ProcessStartRetryPolicy = Policy.Handle<Exception>(exc => exc.Message.Contains("sockwiz")).Or<VirtualClientException>()
4440
.WaitAndRetryAsync(5, retries => TimeSpan.FromSeconds(retries * 3));
@@ -206,16 +202,14 @@ protected override Task InitializeAsync(EventContext telemetryContext, Cancellat
206202
this.Name = $"{this.Scenario} {this.Role}";
207203
this.ProcessName = "cps";
208204
this.Tool = NetworkingWorkloadTool.CPS;
209-
this.ResultsPath = this.PlatformSpecifics.Combine(workloadPackage.Path, CPSExecutor.OutputFileName);
210-
this.results = null;
211205

212206
if (this.Platform == PlatformID.Win32NT)
213207
{
214-
this.ExecutablePath = this.PlatformSpecifics.Combine(workloadPackage.Path, "cps.exe");
208+
this.ExecutablePath = this.Combine(workloadPackage.Path, "cps.exe");
215209
}
216210
else if (this.Platform == PlatformID.Unix)
217211
{
218-
this.ExecutablePath = this.PlatformSpecifics.Combine(workloadPackage.Path, "cps");
212+
this.ExecutablePath = this.Combine(workloadPackage.Path, "cps");
219213
}
220214
else
221215
{
@@ -252,7 +246,7 @@ protected override string GetCommandLineArguments()
252246
}
253247

254248
/// <inheritdoc/>
255-
protected override Task<IProcessProxy> ExecuteWorkloadAsync(string commandArguments, TimeSpan timeout, EventContext telemetryContext, CancellationToken cancellationToken)
249+
protected override Task<IProcessProxy> ExecuteWorkloadAsync(string commandArguments, EventContext telemetryContext, CancellationToken cancellationToken, TimeSpan? timeout = null)
256250
{
257251
IProcessProxy process = null;
258252

@@ -272,16 +266,19 @@ await this.ProcessStartRetryPolicy.ExecuteAsync(async () =>
272266
{
273267
this.CleanupTasks.Add(() => process.SafeKill());
274268
await process.StartAndWaitAsync(cancellationToken, timeout);
275-
276-
if (process.IsErrored())
277-
{
278-
await this.LogProcessDetailsAsync(process, telemetryContext, "CPS", logToFile: true);
279-
process.ThrowIfWorkloadFailed();
280-
}
281-
else
282-
{
283-
this.results = process.StandardOutput.ToString();
284-
}
269+
await this.LogProcessDetailsAsync(process, relatedContext, "CPS");
270+
process.ThrowIfWorkloadFailed();
271+
272+
this.CaptureMetrics(
273+
process.StandardOutput.ToString(),
274+
process.FullCommand(),
275+
process.StartTime,
276+
process.ExitTime,
277+
relatedContext);
278+
}
279+
catch (OperationCanceledException)
280+
{
281+
// Expected when the client signals a cancellation.
285282
}
286283
catch (TimeoutException exc)
287284
{
@@ -298,7 +295,7 @@ await this.ProcessStartRetryPolicy.ExecuteAsync(async () =>
298295
throw new WorkloadException($"CPS workload failed to start successfully", exc, ErrorReason.WorkloadFailed);
299296
}
300297
}
301-
}).ConfigureAwait(false);
298+
});
302299
}
303300

304301
return process;
@@ -308,9 +305,9 @@ await this.ProcessStartRetryPolicy.ExecuteAsync(async () =>
308305
/// <summary>
309306
/// Logs the workload metrics to the telemetry.
310307
/// </summary>
311-
protected override Task CaptureMetricsAsync(string commandArguments, DateTime startTime, DateTime endTime, EventContext telemetryContext)
308+
protected override void CaptureMetrics(string results, string commandArguments, DateTime startTime, DateTime endTime, EventContext telemetryContext)
312309
{
313-
if (!string.IsNullOrWhiteSpace(this.results))
310+
if (!string.IsNullOrWhiteSpace(results))
314311
{
315312
this.MetadataContract.AddForScenario(
316313
this.Tool.ToString(),
@@ -319,7 +316,7 @@ protected override Task CaptureMetricsAsync(string commandArguments, DateTime st
319316

320317
this.MetadataContract.Apply(telemetryContext);
321318

322-
MetricsParser parser = new CPSMetricsParser(this.results, this.ConfidenceLevel, this.WarmupTime);
319+
MetricsParser parser = new CPSMetricsParser(results, this.ConfidenceLevel, this.WarmupTime);
323320
IList<Metric> metrics = parser.Parse();
324321

325322
this.Logger.LogMetrics(
@@ -335,10 +332,10 @@ protected override Task CaptureMetricsAsync(string commandArguments, DateTime st
335332
}
336333
else
337334
{
338-
throw new WorkloadException($"Workload results missing. The CPS workload did not produce any valid results.");
335+
throw new WorkloadException(
336+
$"Workload results missing. The CPS workload did not produce valid results.",
337+
ErrorReason.WorkloadResultsNotFound);
339338
}
340-
341-
return Task.CompletedTask;
342339
}
343340
}
344341
}

‎src/VirtualClient/VirtualClient.Actions/Network/NetworkingWorkload/CPS/CPSServerExecutor.cs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ public class CPSServerExecutor : CPSExecutor
2525
public CPSServerExecutor(IServiceCollection dependencies, IDictionary<string, IConvertible> parameters)
2626
: base(dependencies, parameters)
2727
{
28-
this.WorkloadEmitsResults = true;
2928
}
3029

3130
/// <summary>

‎src/VirtualClient/VirtualClient.Actions/Network/NetworkingWorkload/Latte/LatteClientExecutor.cs

Lines changed: 33 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ namespace VirtualClient.Actions.NetworkPerformance
55
{
66
using System;
77
using System.Collections.Generic;
8-
using System.IO.Abstractions;
98
using System.Linq;
109
using System.Threading;
1110
using System.Threading.Tasks;
@@ -31,11 +30,10 @@ public class LatteClientExecutor : LatteExecutor
3130
public LatteClientExecutor(IServiceCollection dependencies, IDictionary<string, IConvertible> parameters)
3231
: base(dependencies, parameters)
3332
{
34-
this.WorkloadEmitsResults = true;
3533
}
3634

3735
/// <inheritdoc/>
38-
protected override Task<IProcessProxy> ExecuteWorkloadAsync(string commandArguments, TimeSpan timeout, EventContext telemetryContext, CancellationToken cancellationToken)
36+
protected override Task<IProcessProxy> ExecuteWorkloadAsync(string commandArguments, EventContext telemetryContext, CancellationToken cancellationToken, TimeSpan? timeout = null)
3937
{
4038
IProcessProxy process = null;
4139

@@ -58,10 +56,15 @@ await this.ProcessStartRetryPolicy.ExecuteAsync(async () =>
5856

5957
if (!cancellationToken.IsCancellationRequested)
6058
{
61-
await this.LogProcessDetailsAsync(process, telemetryContext, "Latte", logToFile: true);
62-
59+
await this.LogProcessDetailsAsync(process, relatedContext, "Latte");
6360
process.ThrowIfWorkloadFailed();
64-
await this.SystemManagement.FileSystem.File.WriteAllTextAsync(this.ResultsPath, process.StandardOutput.ToString());
61+
62+
this.CaptureMetrics(
63+
process.StandardOutput.ToString(),
64+
process.FullCommand(),
65+
process.StartTime,
66+
process.ExitTime,
67+
relatedContext);
6568
}
6669
}
6770
catch (TimeoutException exc)
@@ -78,7 +81,7 @@ await this.ProcessStartRetryPolicy.ExecuteAsync(async () =>
7881
throw;
7982
}
8083
}
81-
}).ConfigureAwait(false);
84+
});
8285
}
8386

8487
return process;
@@ -100,39 +103,31 @@ protected override string GetCommandLineArguments()
100103
/// <summary>
101104
/// Logs the workload metrics to the telemetry.
102105
/// </summary>
103-
protected override async Task CaptureMetricsAsync(string commandArguments, DateTime startTime, DateTime endTime, EventContext telemetryContext)
106+
protected override void CaptureMetrics(string results, string commandArguments, DateTime startTime, DateTime endTime, EventContext telemetryContext)
104107
{
105-
this.MetadataContract.AddForScenario(
106-
this.Tool.ToString(),
107-
commandArguments,
108-
toolVersion: null);
109-
110-
this.MetadataContract.Apply(telemetryContext);
111-
112-
IFile fileAccess = this.SystemManagement.FileSystem.File;
113-
114-
if (fileAccess.Exists(this.ResultsPath))
108+
if (!string.IsNullOrWhiteSpace(results))
115109
{
116-
string resultsContent = await this.SystemManagement.FileSystem.File.ReadAllTextAsync(this.ResultsPath)
117-
.ConfigureAwait(false);
118-
119-
if (!string.IsNullOrWhiteSpace(resultsContent))
120-
{
121-
MetricsParser parser = new LatteMetricsParser(resultsContent);
122-
IList<Metric> metrics = parser.Parse();
123-
124-
this.Logger.LogMetrics(
125-
this.Tool.ToString(),
126-
this.Name,
127-
startTime,
128-
endTime,
129-
metrics,
130-
string.Empty,
131-
commandArguments,
132-
this.Tags,
133-
telemetryContext,
134-
resultsContent);
135-
}
110+
this.MetadataContract.AddForScenario(
111+
this.Tool.ToString(),
112+
commandArguments,
113+
toolVersion: null);
114+
115+
this.MetadataContract.Apply(telemetryContext);
116+
117+
MetricsParser parser = new LatteMetricsParser(results);
118+
IList<Metric> metrics = parser.Parse();
119+
120+
this.Logger.LogMetrics(
121+
this.Tool.ToString(),
122+
this.Name,
123+
startTime,
124+
endTime,
125+
metrics,
126+
string.Empty,
127+
commandArguments,
128+
this.Tags,
129+
telemetryContext,
130+
results);
136131
}
137132
}
138133
}

‎src/VirtualClient/VirtualClient.Actions/Network/NetworkingWorkload/Latte/LatteExecutor.cs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ public class LatteExecutor : NetworkingWorkloadToolExecutor
2828
public LatteExecutor(IServiceCollection dependencies, IDictionary<string, IConvertible> parameters)
2929
: base(dependencies, parameters)
3030
{
31-
this.WorkloadEmitsResults = true;
3231
this.ProcessStartRetryPolicy = Policy.Handle<Exception>(exc => exc.Message.Contains("sockwiz_tcp_listener_open bind"))
3332
.WaitAndRetryAsync(5, retries => TimeSpan.FromSeconds(retries * 3));
3433
}
@@ -123,7 +122,6 @@ protected override Task InitializeAsync(EventContext telemetryContext, Cancellat
123122
this.Name = $"{this.Scenario} {this.Role}";
124123
this.ProcessName = "latte";
125124
this.Tool = NetworkingWorkloadTool.Latte;
126-
this.ResultsPath = this.PlatformSpecifics.Combine(workloadPackage.Path, LatteExecutor.OutputFileName);
127125
this.ExecutablePath = this.PlatformSpecifics.Combine(workloadPackage.Path, "latte.exe");
128126

129127
return Task.CompletedTask;

0 commit comments

Comments
 (0)