-
Notifications
You must be signed in to change notification settings - Fork 4.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[HTTP/3] Stress hack for msquic dropping connections #84793
Changes from 4 commits
df45d59
df7b541
bd6c04d
fa5820e
b649716
ed73b02
7e4d512
1f8f56e
d2c113c
7613f06
0a9731f
e316a13
949d82b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,6 +13,8 @@ | |
using System.Threading.Tasks; | ||
using System.Net; | ||
using HttpStress; | ||
using System.Net.Quic; | ||
using Microsoft.Quic; | ||
|
||
[assembly:SupportedOSPlatform("windows")] | ||
[assembly:SupportedOSPlatform("linux")] | ||
|
@@ -26,6 +28,8 @@ public static class Program | |
{ | ||
public enum ExitCode { Success = 0, StressError = 1, CliError = 2 }; | ||
|
||
public static readonly bool IsQuicSupported = QuicListener.IsSupported && QuicConnection.IsSupported; | ||
|
||
public static async Task<int> Main(string[] args) | ||
{ | ||
if (!TryParseCli(args, out Configuration? config)) | ||
|
@@ -158,6 +162,9 @@ private static async Task<ExitCode> Run(Configuration config) | |
|
||
string GetAssemblyInfo(Assembly assembly) => $"{assembly.Location}, modified {new FileInfo(assembly.Location).LastWriteTime}"; | ||
|
||
Type msQuicApiType = typeof(QuicConnection).Assembly.GetType("System.Net.Quic.MsQuicApi"); | ||
string msQuicLibraryVersion = (string)msQuicApiType.GetProperty("MsQuicLibraryVersion", BindingFlags.NonPublic | BindingFlags.Static).GetGetMethod(true).Invoke(null, Array.Empty<object?>()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @ManickaP unfortunately, this line fails with We could workaround this by moving the version extraction to a shared test-utility (together with the MaxWorkerQueue hack helper), but IMHO it's not worth it unless we think that it's super-important to know the msquic version for stress test troubleshooting. For now I'm just removing it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is it possible that there is image without MsQuic? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What do you mean? If you are suspecting lack of msquic as the root cause of this error - it's not the case. Removing code messing with MsQuicLibraryVersion makes things work (= HTTP/3 stress runs). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
In that case you'd get "unknown" as the MsQuic version, the code expects that. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Another option would be to tell the trimmer to keep it there via There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should try to clarify whether test-only usage of |
||
|
||
Console.WriteLine(" .NET Core: " + GetAssemblyInfo(typeof(object).Assembly)); | ||
Console.WriteLine(" ASP.NET Core: " + GetAssemblyInfo(typeof(WebHost).Assembly)); | ||
Console.WriteLine(" System.Net.Http: " + GetAssemblyInfo(typeof(System.Net.Http.HttpClient).Assembly)); | ||
|
@@ -169,6 +176,8 @@ private static async Task<ExitCode> Run(Configuration config) | |
Console.WriteLine(" Concurrency: " + config.ConcurrentRequests); | ||
Console.WriteLine(" Content Length: " + config.MaxContentLength); | ||
Console.WriteLine(" HTTP Version: " + config.HttpVersion); | ||
Console.WriteLine(" QUIC supported: " + (IsQuicSupported ? "yes" : "no")); | ||
Console.WriteLine(" MsQuic Version: " + msQuicLibraryVersion); | ||
Console.WriteLine(" Lifetime: " + (config.ConnectionLifetime.HasValue ? $"{config.ConnectionLifetime.Value.TotalMilliseconds}ms" : "(infinite)")); | ||
Console.WriteLine(" Operations: " + string.Join(", ", usedClientOperations.Select(o => o.name))); | ||
Console.WriteLine(" Random Seed: " + config.RandomSeed); | ||
|
@@ -177,6 +186,21 @@ private static async Task<ExitCode> Run(Configuration config) | |
Console.WriteLine("Query Parameters: " + config.MaxParameters); | ||
Console.WriteLine(); | ||
|
||
if (config.HttpVersion == HttpVersion.Version30 && IsQuicSupported) | ||
{ | ||
unsafe | ||
{ | ||
object msQuicApiInstance = msQuicApiType.GetProperty("Api", BindingFlags.NonPublic | BindingFlags.Static).GetGetMethod(true).Invoke(null, Array.Empty<object?>()); | ||
QUIC_API_TABLE* apiTable = (QUIC_API_TABLE*)(Pointer.Unbox(msQuicApiType.GetProperty("ApiTable").GetGetMethod().Invoke(msQuicApiInstance, Array.Empty<object?>()))); | ||
QUIC_SETTINGS settings = default(QUIC_SETTINGS); | ||
settings.IsSet.MaxWorkerQueueDelayUs = 1; | ||
settings.MaxWorkerQueueDelayUs = 2_500_000u; // 2.5s, 10x the default | ||
if (MsQuic.StatusFailed(apiTable->SetParam(null, MsQuic.QUIC_PARAM_GLOBAL_SETTINGS, (uint)sizeof(QUIC_SETTINGS), (byte*)&settings))) | ||
{ | ||
Console.WriteLine($"Unable to set MsQuic MaxWorkerQueueDelayUs."); | ||
} | ||
} | ||
} | ||
|
||
StressServer? server = null; | ||
if (config.RunMode.HasFlag(RunMode.server)) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,8 +13,8 @@ | |
using Xunit.Abstractions; | ||
using System.Diagnostics.Tracing; | ||
using System.Net.Sockets; | ||
using System.Reflection; | ||
using Microsoft.Quic; | ||
using static Microsoft.Quic.MsQuic; | ||
|
||
namespace System.Net.Quic.Tests | ||
{ | ||
|
@@ -44,14 +44,20 @@ public abstract class QuicTestBase : IDisposable | |
|
||
static unsafe QuicTestBase() | ||
{ | ||
Console.WriteLine($"MsQuic {(IsSupported ? "supported" : "not supported")} and using '{MsQuicApi.MsQuicLibraryVersion}'."); | ||
// If any of the reflection bellow breaks due to changes in "System.Net.Quic.MsQuicApi", also check and fix HttpStress project as it uses the same hack. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm wondering if this should go somewhere to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds good, I'll look into it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should keep in mind that whatever utility helper we add, its' file has to be also copied to the container, so avoiding duplication has a higher cost in complexity here than normally. At the moment I'm undecided if it's worth it, would prefer to do the docker copying work mentioned in #84793 (comment) first, planning to jump to it tomorrow. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, I'm holding back. Let me know when/if I should look into it. |
||
Type msQuicApiType = typeof(QuicConnection).Assembly.GetType("System.Net.Quic.MsQuicApi"); | ||
|
||
string msQuicLibraryVersion = (string)msQuicApiType.GetProperty("MsQuicLibraryVersion", BindingFlags.NonPublic | BindingFlags.Static).GetGetMethod(true).Invoke(null, Array.Empty<object?>()); | ||
Console.WriteLine($"MsQuic {(IsSupported ? "supported" : "not supported")} and using '{msQuicLibraryVersion}'."); | ||
|
||
if (IsSupported) | ||
{ | ||
object msQuicApiInstance = msQuicApiType.GetProperty("Api", BindingFlags.NonPublic | BindingFlags.Static).GetGetMethod(true).Invoke(null, Array.Empty<object?>()); | ||
QUIC_API_TABLE* apiTable = (QUIC_API_TABLE*)(Pointer.Unbox(msQuicApiType.GetProperty("ApiTable").GetGetMethod().Invoke(msQuicApiInstance, Array.Empty<object?>()))); | ||
QUIC_SETTINGS settings = default(QUIC_SETTINGS); | ||
settings.IsSet.MaxWorkerQueueDelayUs = 1; | ||
settings.MaxWorkerQueueDelayUs = 2_500_000u; // 2.5s, 10x the default | ||
if (StatusFailed(MsQuicApi.Api.ApiTable->SetParam(null, QUIC_PARAM_GLOBAL_SETTINGS, (uint)sizeof(QUIC_SETTINGS), (byte*)&settings))) | ||
if (MsQuic.StatusFailed(apiTable->SetParam(null, MsQuic.QUIC_PARAM_GLOBAL_SETTINGS, (uint)sizeof(QUIC_SETTINGS), (byte*)&settings))) | ||
{ | ||
Console.WriteLine($"Unable to set MsQuic MaxWorkerQueueDelayUs."); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
8.0 will not support Centos 7 AFAIK (hurray!) we should update this at some point
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stream9??? Or should we swap for different distro?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason we are running stress tests on Centos and not ubuntu, unlike enterprise tests?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the
centos-7
images have build tools and we use them for product. I don't know if that is needed.I think we should pick whatever is easiest for us to maintain.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm switching to
centos-stream8
in #85342, which seems to work.