Skip to content

Commit 34f9f2b

Browse files
brdeyobrdeyo
committed
Fix bug in SockPerf that allows the executable to remain running after the VC scenario completes. Include client request ID in networking workloads for ease-of-correlation between client operations and like server operations.
1 parent 75be809 commit 34f9f2b

File tree

23 files changed

+1338
-68
lines changed

23 files changed

+1338
-68
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.11.3
19+
VcVersion : 1.11.4
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.11.3
21+
VcVersion : 1.11.4
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/Network/NetworkingWorkload/Latte/LatteServerExecutor.cs

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -56,19 +56,26 @@ await this.ProcessStartRetryPolicy.ExecuteAsync(async () =>
5656
await this.LogProcessDetailsAsync(process, relatedContext, "Latte");
5757
process.ThrowIfErrored<WorkloadException>(errorReason: ErrorReason.WorkloadFailed);
5858
}
59+
else
60+
{
61+
try
62+
{
63+
await this.WaitAsync(cancellationToken);
64+
process.Close();
5965

60-
this.CleanupTasks.Add(() => process.SafeKill());
61-
62-
// Run the server slightly longer than the test duration.
63-
TimeSpan serverWaitTime = TimeSpan.FromMilliseconds(this.Iterations * .5);
64-
await this.WaitAsync(serverWaitTime, cancellationToken);
65-
await this.LogProcessDetailsAsync(process, relatedContext, "Latte", logToFile: true);
66+
await process.WaitForExitAsync(cancellationToken);
67+
await this.LogProcessDetailsAsync(process, relatedContext, "Latte");
68+
}
69+
finally
70+
{
71+
process.SafeKill();
72+
}
73+
}
6674
}
6775
}
6876
catch (Exception exc)
6977
{
7078
this.Logger.LogMessage($"{this.TypeName}.WorkloadStartupError", LogLevel.Warning, relatedContext.AddError(exc));
71-
process.SafeKill();
7279
throw;
7380
}
7481
});

‎src/VirtualClient/VirtualClient.Actions/Network/NetworkingWorkload/NetworkingWorkloadExecutor.cs

Lines changed: 24 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -728,23 +728,23 @@ protected void OnInstructionsReceived(object sender, JObject instructions)
728728
this.Logger.LogTraceMessage($"{nameof(NetworkingWorkloadExecutor)}.Notification = {instructions}");
729729

730730
Item<State> notification = instructions.ToObject<Item<State>>();
731-
EventContext relatedContext = EventContext.Persisted();
732731

733732
if (notification.Definition.Properties.ContainsKey(nameof(NetworkingWorkloadState.Tool)))
734733
{
735734
NetworkingWorkloadState serverInstructions = new NetworkingWorkloadState(notification.Definition.Properties);
735+
telemetryContext.AddClientRequestId(serverInstructions.ClientRequestId);
736736

737737
if (serverInstructions.ToolState == NetworkingWorkloadToolState.Stop)
738738
{
739739
this.Logger.LogTraceMessage($"Synchronization: Stopping all workloads...");
740-
this.StopServerTool(relatedContext);
741-
this.DeleteWorkloadStateAsync(relatedContext, cancellationToken).GetAwaiter().GetResult();
740+
this.StopServerTool(telemetryContext);
741+
this.DeleteWorkloadStateAsync(telemetryContext, cancellationToken).GetAwaiter().GetResult();
742742
}
743743
else if (serverInstructions.ToolState == NetworkingWorkloadToolState.Start)
744744
{
745745
this.Logger.LogTraceMessage($"Synchronization: Starting {serverInstructions.Tool} workload...");
746-
this.StopServerTool(relatedContext);
747-
this.DeleteWorkloadStateAsync(relatedContext, cancellationToken).GetAwaiter().GetResult();
746+
this.StopServerTool(telemetryContext);
747+
this.DeleteWorkloadStateAsync(telemetryContext, cancellationToken).GetAwaiter().GetResult();
748748

749749
// The client will pass the settings to the server side. The server side will need to be updated
750750
// to use those settings specified below. (e.g. communications protocol, concurrent threads, network buffer size).
@@ -778,10 +778,10 @@ protected void OnInstructionsReceived(object sender, JObject instructions)
778778
NetworkingWorkloadExecutor.LocalApiClient,
779779
nameof(NetworkingWorkloadState),
780780
serverInstructions,
781-
relatedContext,
781+
telemetryContext,
782782
cancellationToken).GetAwaiter().GetResult();
783783

784-
this.StartServerTool(serverInstructions.Tool, relatedContext, cancellationToken);
784+
this.StartServerTool(serverInstructions.Tool, serverInstructions.ClientRequestId, telemetryContext, cancellationToken);
785785
}
786786
}
787787
});
@@ -877,24 +877,25 @@ await this.ClientExecutionRetryPolicy.ExecuteAsync(async () =>
877877
{
878878
if (!cancellationToken.IsCancellationRequested)
879879
{
880+
// The request ID enables correlation between client/server and server operations.
881+
Guid requestId = Guid.NewGuid();
882+
relatedContext.AddClientRequestId(requestId);
883+
880884
this.Logger.LogTraceMessage("Synchronization: Wait for server online...");
881885

882886
// 1) Confirm server is online.
883887
// ===========================================================================
884-
await NetworkingWorkloadExecutor.ServerApiClient.PollForHeartbeatAsync(this.ServerOnlinePollingTimeout, cancellationToken, logger: this.Logger)
885-
.ConfigureAwait(false);
888+
await NetworkingWorkloadExecutor.ServerApiClient.PollForHeartbeatAsync(this.ServerOnlinePollingTimeout, cancellationToken, logger: this.Logger);
886889

887890
// 2) Wait for the server to signal the eventing API is online.
888891
// ===========================================================================
889-
await NetworkingWorkloadExecutor.ServerApiClient.PollForServerOnlineAsync(this.ServerOnlinePollingTimeout, cancellationToken, logger: this.Logger)
890-
.ConfigureAwait(false);
892+
await NetworkingWorkloadExecutor.ServerApiClient.PollForServerOnlineAsync(this.ServerOnlinePollingTimeout, cancellationToken, logger: this.Logger);
891893

892894
// 3) Request the server to stop ALL workload processes
893895
// ===========================================================================
894896
this.Logger.LogTraceMessage("Synchronization: Request server to stop all workloads...");
895897

896-
await this.RequestStopAllWorkloadsAsync(telemetryContext, cancellationToken)
897-
.ConfigureAwait(false);
898+
await this.RequestStopAllWorkloadsAsync(relatedContext, cancellationToken);
898899

899900
// 4) Request the server start the next workload.
900901
// ===========================================================================
@@ -924,7 +925,8 @@ await this.RequestStopAllWorkloadsAsync(telemetryContext, cancellationToken)
924925
this.ProfilingEnabled,
925926
this.ProfilingScenario,
926927
this.ProfilingPeriod.ToString(),
927-
this.ProfilingWarmUpPeriod.ToString());
928+
this.ProfilingWarmUpPeriod.ToString(),
929+
requestId);
928930

929931
Item<State> instructions = new Item<State>(nameof(NetworkingWorkloadState), workloadInstructions);
930932
relatedContext.AddContext("instructions", instructions);
@@ -933,8 +935,8 @@ await this.RequestStopAllWorkloadsAsync(telemetryContext, cancellationToken)
933935
await NetworkingWorkloadExecutor.SendInstructionsAsync(
934936
NetworkingWorkloadExecutor.ServerApiClient,
935937
instructions,
936-
telemetryContext,
937-
cancellationToken).ConfigureAwait(false);
938+
relatedContext,
939+
cancellationToken);
938940

939941
// 5) Confirm the server has started the requested workload.
940942
// ===========================================================================
@@ -947,14 +949,15 @@ await NetworkingWorkloadExecutor.ServerApiClient.PollForExpectedStateAsync<Netwo
947949
&& serverState.ToolState == NetworkingWorkloadToolState.Running;
948950
},
949951
this.StateConfirmationPollingTimeout,
950-
cancellationToken).ConfigureAwait(false);
952+
cancellationToken);
951953

952954
this.Logger.LogTraceMessage("Synchronization: Server workload startup confirmed...");
953955
this.Logger.LogTraceMessage("Synchronization: Start client workload...");
954956

955957
// 6) Execute the client workload.
956958
// ===========================================================================
957959
NetworkingWorkloadToolExecutor executor = this.CreateWorkloadExecutor(toolName);
960+
executor.ClientRequestId = requestId;
958961

959962
try
960963
{
@@ -965,11 +968,8 @@ await executor.ExecuteAsync(cancellationToken)
965968
{
966969
this.Logger.LogTraceMessage("Synchronization: Wait for server to stop workload...");
967970

968-
await NetworkingWorkloadExecutor.PollUntilStateDeletedAsync(
969-
NetworkingWorkloadExecutor.ServerApiClient,
970-
nameof(NetworkingWorkloadState),
971-
this.StateConfirmationPollingTimeout,
972-
cancellationToken).ConfigureAwait(false);
971+
await this.RequestStopAllWorkloadsAsync(telemetryContext, cancellationToken)
972+
.ConfigureAwait(false);
973973
}
974974
}
975975

@@ -1026,7 +1026,7 @@ private Task DeleteWorkloadStateAsync(EventContext telemetryContext, Cancellatio
10261026
});
10271027
}
10281028

1029-
private void StartServerTool(NetworkingWorkloadTool toolName, EventContext telemetryContext, CancellationToken cancellationToken)
1029+
private void StartServerTool(NetworkingWorkloadTool toolName, Guid? clientRequestId, EventContext telemetryContext, CancellationToken cancellationToken)
10301030
{
10311031
try
10321032
{
@@ -1037,6 +1037,7 @@ private void StartServerTool(NetworkingWorkloadTool toolName, EventContext telem
10371037
{
10381038
CancellationTokenSource cancellationSource = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken);
10391039
NetworkingWorkloadToolExecutor action = this.CreateWorkloadExecutor(toolName);
1040+
action.ClientRequestId = clientRequestId;
10401041

10411042
this.backgroundWorkloadServer = new BackgroundWorkloadServer
10421043
{

‎src/VirtualClient/VirtualClient.Actions/Network/NetworkingWorkload/NetworkingWorkloadState.cs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ namespace VirtualClient.Actions.NetworkPerformance
66
using System;
77
using System.Collections.Generic;
88
using System.Globalization;
9+
using Microsoft.AspNetCore.Http.Features;
910
using Newtonsoft.Json;
1011
using VirtualClient.Common.Extensions;
1112
using VirtualClient.Contracts;
@@ -118,7 +119,8 @@ public NetworkingWorkloadState(
118119
bool profilingEnabled = false,
119120
string profilingScenario = null,
120121
string profilingPeriod = null,
121-
string profilingWarmUpPeriod = null)
122+
string profilingWarmUpPeriod = null,
123+
Guid? clientRequestId = null)
122124
{
123125
packageName.ThrowIfNull(nameof(packageName));
124126
scenario.ThrowIfNull(nameof(scenario));
@@ -151,6 +153,7 @@ public NetworkingWorkloadState(
151153
this.Properties[nameof(this.ProfilingScenario)] = profilingScenario;
152154
this.Properties[nameof(this.ProfilingPeriod)] = profilingPeriod;
153155
this.Properties[nameof(this.ProfilingWarmUpPeriod)] = profilingWarmUpPeriod;
156+
this.ClientRequestId = clientRequestId;
154157
}
155158

156159
/// <summary>

‎src/VirtualClient/VirtualClient.Actions/Network/NetworkingWorkload/SockPerf/SockPerfServerExecutor.cs

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ namespace VirtualClient.Actions.NetworkPerformance
55
{
66
using System;
77
using System.Collections.Generic;
8+
using System.Diagnostics;
89
using System.Linq;
910
using System.Threading;
1011
using System.Threading.Tasks;
@@ -61,19 +62,29 @@ await this.ProcessStartRetryPolicy.ExecuteAsync(async () =>
6162
List<int> successCodes = new List<int>() { 0, 137 };
6263
process.ThrowIfErrored<WorkloadException>(successCodes, errorReason: ErrorReason.WorkloadFailed);
6364
}
65+
else
66+
{
67+
try
68+
{
69+
// Wait until the cancellation token is signalled.
70+
await this.WaitAsync(cancellationToken);
71+
process.Close();
6472

65-
this.CleanupTasks.Add(() => process.SafeKill());
66-
67-
// Run the server slightly longer than the test duration.
68-
TimeSpan serverWaitTime = TimeSpan.FromSeconds(this.TestDuration + 10);
69-
await this.WaitAsync(serverWaitTime, cancellationToken);
70-
await this.LogProcessDetailsAsync(process, relatedContext, "SockPerf", logToFile: true);
73+
await process.WaitForExitAsync(cancellationToken);
74+
await this.LogProcessDetailsAsync(process, relatedContext, "SockPerf");
75+
}
76+
finally
77+
{
78+
// SockPerf must be explicitly terminated given the current implementation. If it is not,
79+
// the process will remain running in the background.
80+
process.SafeKill();
81+
}
82+
}
7183
}
7284
}
7385
catch (Exception exc)
7486
{
7587
this.Logger.LogMessage($"{this.GetType().Name}.WorkloadStartupError", LogLevel.Warning, relatedContext.AddError(exc));
76-
process.SafeKill();
7788
throw;
7889
}
7990
}).ConfigureAwait(false);

‎src/VirtualClient/VirtualClient.Actions/Network2/NetworkingWorkload2/CPS/CPSClientExecutor2.cs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -291,6 +291,9 @@ await this.ServerApiClient.SendInstructionsAsync(JObject.FromObject(instructions
291291

292292
private Task ExecuteClientWorkloadAsync(EventContext telemetryContext, CancellationToken cancellationToken)
293293
{
294+
Guid requestId = Guid.NewGuid();
295+
telemetryContext.AddClientRequestId(requestId);
296+
294297
return this.Logger.LogMessageAsync($"{this.TypeName}.ExecuteClientWorkload", telemetryContext, async () =>
295298
{
296299
this.Logger.LogTraceMessage("Synchronization: Wait for server online...");

‎src/VirtualClient/VirtualClient.Actions/Network2/NetworkingWorkload2/Latte/LatteClientExecutor2.cs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,9 @@ await this.ServerApiClient.SendInstructionsAsync(JObject.FromObject(instructions
175175

176176
private Task ExecuteClientWorkloadAsync(EventContext telemetryContext, CancellationToken cancellationToken)
177177
{
178+
Guid requestId = Guid.NewGuid();
179+
telemetryContext.AddClientRequestId(requestId);
180+
178181
return this.Logger.LogMessageAsync($"{this.TypeName}.ExecuteClientWorkload", telemetryContext, async () =>
179182
{
180183
this.Logger.LogTraceMessage("Synchronization: Wait for server online...");

‎src/VirtualClient/VirtualClient.Actions/Network2/NetworkingWorkload2/Latte/LatteServerExecutor2.cs

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -364,19 +364,26 @@ await this.ProcessStartRetryPolicy.ExecuteAsync(async () =>
364364
await this.LogProcessDetailsAsync(process, relatedContext, "Latte");
365365
process.ThrowIfErrored<WorkloadException>(errorReason: ErrorReason.WorkloadFailed);
366366
}
367-
368-
this.CleanupTasks.Add(() => process.SafeKill());
369-
370-
// Run the server slightly longer than the test duration.
371-
TimeSpan serverWaitTime = TimeSpan.FromMilliseconds(this.Iterations * .5);
372-
await this.WaitAsync(serverWaitTime, cancellationToken);
373-
await this.LogProcessDetailsAsync(process, relatedContext, "Latte", logToFile: true);
367+
else
368+
{
369+
try
370+
{
371+
await this.WaitAsync(cancellationToken);
372+
process.Close();
373+
374+
await process.WaitForExitAsync(cancellationToken);
375+
await this.LogProcessDetailsAsync(process, relatedContext, "Latte");
376+
}
377+
finally
378+
{
379+
process.SafeKill();
380+
}
381+
}
374382
}
375383
}
376384
catch (Exception exc)
377385
{
378386
this.Logger.LogMessage($"{this.GetType().Name}.WorkloadStartupError", LogLevel.Warning, relatedContext.AddError(exc));
379-
process.SafeKill();
380387
throw;
381388
}
382389
}

‎src/VirtualClient/VirtualClient.Actions/Network2/NetworkingWorkload2/NTttcp/NTttcpClientExecutor2.cs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -310,6 +310,9 @@ private string GetCommandLineArguments()
310310

311311
private Task ExecuteClientWorkloadAsync(EventContext telemetryContext, CancellationToken cancellationToken)
312312
{
313+
Guid requestId = Guid.NewGuid();
314+
telemetryContext.AddClientRequestId(requestId);
315+
313316
return this.Logger.LogMessageAsync($"{this.TypeName}.ExecuteClientWorkload", telemetryContext, async () =>
314317
{
315318
this.Logger.LogTraceMessage("Synchronization: Wait for server online...");

0 commit comments

Comments
 (0)