Skip to content

Commit 0bfff90

Browse files
committed
fix(client): O(1) in-flight request tracking to fix H2 high-concurrency CPU collapse
The lock-free intrusive pending-request list added in 50918ff removes a node in SendAsync's finally with an O(N) walk; at high concurrency (512 H2 streams) this degrades to an O(N^2) walk plus a single-point CompareExchange storm on the list head, pinning CPU while throughput collapses (H2 @512 ~60K -> <1K rps). Restore the O(1) ConcurrentDictionary tracker and drop the now-dead Next field. Regression guards: - unit (SendAsyncPendingRequestContentionSpec): drives SendAsync against an in-memory fake pipeline (no sockets, no Akka graph) so only the in-flight bookkeeping is under load. 200k requests @512 concurrency: ~0.5s with the fix, ~1.2K rps / times out with the lock-free list. - integration (SendAsyncConcurrentThroughputRegressionSpec): sustains 512 concurrent H2 SendAsync against Kestrel and requires the batch to finish well within a deadline.
1 parent 9792830 commit 0bfff90

4 files changed

Lines changed: 304 additions & 82 deletions

File tree

Lines changed: 175 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,175 @@
1+
using System.Diagnostics;
2+
using System.Net;
3+
using Akka.Actor;
4+
using Akka.DependencyInjection;
5+
using Microsoft.AspNetCore.Builder;
6+
using Microsoft.AspNetCore.Hosting.Server;
7+
using Microsoft.AspNetCore.Hosting.Server.Features;
8+
using Microsoft.AspNetCore.Http;
9+
using Microsoft.AspNetCore.Server.Kestrel.Core;
10+
using Microsoft.Extensions.DependencyInjection;
11+
using Microsoft.Extensions.DependencyInjection.Extensions;
12+
using Microsoft.Extensions.Logging;
13+
using Microsoft.Extensions.Options;
14+
using GaudiHTTP.Client;
15+
16+
namespace GaudiHTTP.IntegrationTests.End2End.H2;
17+
18+
/// <summary>
19+
/// Regression guard for the client-side throughput collapse introduced by commit 50918ff2
20+
/// ("perf: round 4 — lock-free pending list"). That change replaced the O(1) ConcurrentDictionary
21+
/// tracking in-flight <c>SendAsync</c> calls with a hand-rolled lock-free linked list whose
22+
/// per-request removal is O(N); under high concurrency it becomes an O(N^2) walk plus a single-point
23+
/// CompareExchange storm on the list head, burning CPU while throughput collapses (~60K -> ~0.3K rps
24+
/// at 512 concurrent H2 streams).
25+
///
26+
/// The guard drives sustained 512-concurrent SendAsync against a localhost Kestrel h2c server and
27+
/// requires the batch to finish well within a deadline. With the bug the batch needs tens of seconds
28+
/// (or stalls); with O(1) tracking it finishes in ~1s of request time. Server is Kestrel (not
29+
/// GaudiServer) so the measurement isolates the client.
30+
/// </summary>
31+
public sealed class SendAsyncConcurrentThroughputRegressionSpec : IAsyncLifetime
32+
{
33+
private const int Concurrency = 512;
34+
private const int TotalRequests = 20_000;
35+
private static readonly TimeSpan Deadline = TimeSpan.FromSeconds(15);
36+
37+
private WebApplication? _kestrelApp;
38+
private IGaudiHttpClient? _client;
39+
private Microsoft.Extensions.DependencyInjection.ServiceProvider? _clientProvider;
40+
private string _baseUri = string.Empty;
41+
42+
private CancellationToken CT => TestContext.Current.CancellationToken;
43+
44+
public async ValueTask InitializeAsync()
45+
{
46+
var builder = WebApplication.CreateBuilder();
47+
builder.Logging.ClearProviders();
48+
builder.Services.Configure<KestrelServerOptions>(kestrel =>
49+
{
50+
kestrel.Listen(IPAddress.Loopback, 0, lo => lo.Protocols = HttpProtocols.Http2);
51+
kestrel.Limits.Http2.MaxStreamsPerConnection = 512;
52+
kestrel.Limits.MaxConcurrentConnections = null;
53+
});
54+
55+
_kestrelApp = builder.Build();
56+
_kestrelApp.MapGet("/simple", () => "OK\n");
57+
await _kestrelApp.StartAsync();
58+
59+
var port = new Uri(_kestrelApp.Services.GetRequiredService<IServer>()
60+
.Features.Get<IServerAddressesFeature>()!.Addresses.First()).Port;
61+
_baseUri = $"http://127.0.0.1:{port}";
62+
63+
var services = new ServiceCollection();
64+
var diSetup = DependencyResolverSetup.Create(services.BuildServiceProvider());
65+
var system = ActorSystem.Create($"regress-{Guid.NewGuid():N}", BootstrapSetup.Create().And(diSetup));
66+
services.AddSingleton(system);
67+
68+
var clientOptions = new GaudiClientOptions
69+
{
70+
BaseAddress = new Uri(_baseUri),
71+
Http2 = new Http2ClientOptions
72+
{
73+
MaxConnectionsPerServer = 16,
74+
MaxConcurrentStreams = 512,
75+
},
76+
};
77+
78+
services.AddGaudiHttpClient();
79+
services.Replace(ServiceDescriptor.Singleton<IOptionsFactory<GaudiClientOptions>>(
80+
new FixedOptionsFactory(clientOptions)));
81+
_clientProvider = services.BuildServiceProvider();
82+
83+
_client = _clientProvider.GetRequiredService<IGaudiHttpClientFactory>().CreateClient(string.Empty);
84+
_client.BaseAddress = new Uri(_baseUri);
85+
_client.DefaultRequestVersion = HttpVersion.Version20;
86+
_client.DefaultVersionPolicy = HttpVersionPolicy.RequestVersionExact;
87+
_client.Timeout = TimeSpan.FromSeconds(30);
88+
89+
// Warm the first connection so the measured batch excludes connection establishment.
90+
using var warmup = new HttpRequestMessage(HttpMethod.Get, $"{_baseUri}/simple");
91+
using var warmupResp = await _client.SendAsync(warmup, CT);
92+
warmupResp.EnsureSuccessStatusCode();
93+
}
94+
95+
public async ValueTask DisposeAsync()
96+
{
97+
_client?.Dispose();
98+
99+
if (_kestrelApp is not null)
100+
{
101+
await _kestrelApp.StopAsync();
102+
await _kestrelApp.DisposeAsync();
103+
}
104+
105+
if (_clientProvider is not null)
106+
{
107+
var system = _clientProvider.GetService<ActorSystem>();
108+
if (system is not null)
109+
{
110+
try { await system.Terminate().WaitAsync(TimeSpan.FromSeconds(10)); } catch { }
111+
try { await system.WhenTerminated.WaitAsync(TimeSpan.FromSeconds(5)); } catch { }
112+
}
113+
114+
await _clientProvider.DisposeAsync();
115+
}
116+
}
117+
118+
[Fact(Timeout = 60_000)]
119+
public async Task SendAsync_under_sustained_512_concurrency_should_not_collapse()
120+
{
121+
var dispatched = 0;
122+
var completed = 0;
123+
var failed = 0;
124+
var sw = Stopwatch.StartNew();
125+
126+
async Task Worker()
127+
{
128+
while (Interlocked.Increment(ref dispatched) <= TotalRequests)
129+
{
130+
try
131+
{
132+
using var request = new HttpRequestMessage(HttpMethod.Get, $"{_baseUri}/simple");
133+
using var response = await _client!.SendAsync(request, CT);
134+
response.EnsureSuccessStatusCode();
135+
Interlocked.Increment(ref completed);
136+
}
137+
catch
138+
{
139+
Interlocked.Increment(ref failed);
140+
}
141+
}
142+
}
143+
144+
var workers = new Task[Concurrency];
145+
for (var i = 0; i < Concurrency; i++)
146+
{
147+
workers[i] = Worker();
148+
}
149+
150+
try
151+
{
152+
await Task.WhenAll(workers).WaitAsync(Deadline, CT);
153+
}
154+
catch (TimeoutException)
155+
{
156+
Assert.Fail(
157+
$"Sustained {Concurrency}-concurrent SendAsync completed only {completed}/{TotalRequests} " +
158+
$"within {Deadline.TotalSeconds:F0}s ({completed / sw.Elapsed.TotalSeconds:F0} rps) — " +
159+
$"in-flight request-tracking contention regression (lock-free pending list, commit 50918ff2).");
160+
}
161+
162+
sw.Stop();
163+
TestContext.Current.TestOutputHelper?.WriteLine(
164+
$"{TotalRequests} requests @ {Concurrency} concurrent in {sw.Elapsed.TotalMilliseconds:F0}ms " +
165+
$"({TotalRequests / sw.Elapsed.TotalSeconds:F0} rps)");
166+
167+
Assert.Equal(0, failed);
168+
Assert.Equal(TotalRequests, completed);
169+
}
170+
171+
private sealed class FixedOptionsFactory(GaudiClientOptions options) : IOptionsFactory<GaudiClientOptions>
172+
{
173+
public GaudiClientOptions Create(string name) => options;
174+
}
175+
}
Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
using System.Diagnostics;
2+
using System.Net;
3+
using System.Threading.Channels;
4+
using Akka.Actor;
5+
using GaudiHTTP.Client;
6+
using GaudiHTTP.Internal;
7+
using GaudiHTTP.Streams.Lifecycle;
8+
9+
namespace GaudiHTTP.Tests.Client;
10+
11+
/// <summary>
12+
/// Unit regression guard for the lock-free pending-request list (commit 50918ff2). That change
13+
/// tracked in-flight <see cref="GaudiHttpClient.SendAsync"/> calls in a hand-rolled lock-free linked
14+
/// list whose per-request removal is O(N); under high concurrency it degrades to an O(N^2) walk plus
15+
/// a single-point CompareExchange storm on the list head, collapsing throughput.
16+
///
17+
/// This drives <c>SendAsync</c> directly against an in-memory fake pipeline — no sockets, no Akka
18+
/// stream graph, no transport — so the only thing exercised under concurrency is the in-flight
19+
/// request bookkeeping. With O(1) tracking the batch finishes in well under a second; with the
20+
/// regressed list it cannot finish within the deadline.
21+
/// </summary>
22+
public sealed class SendAsyncPendingRequestContentionSpec
23+
{
24+
private const int Concurrency = 512;
25+
private const int TotalRequests = 200_000;
26+
private static readonly TimeSpan Deadline = TimeSpan.FromSeconds(10);
27+
28+
[Fact(Timeout = 30_000)]
29+
public async Task SendAsync_tracks_in_flight_requests_without_quadratic_contention()
30+
{
31+
var requests = Channel.CreateUnbounded<HttpRequestMessage>(
32+
new UnboundedChannelOptions { SingleReader = true });
33+
var responses = Channel.CreateUnbounded<HttpResponseMessage>(
34+
new UnboundedChannelOptions { SingleWriter = true });
35+
36+
// Infinite timeout + CancellationToken.None below selects SendAsync's no-CTS fast path,
37+
// so per-request work is purely the pending-request add/await/remove bookkeeping.
38+
var options = new GaudiRequestOptions(
39+
BaseAddress: new Uri("http://localhost"),
40+
DefaultRequestHeaders: new HttpRequestMessage().Headers,
41+
DefaultRequestVersion: HttpVersion.Version11,
42+
DefaultVersionPolicy: HttpVersionPolicy.RequestVersionOrLower,
43+
Timeout: System.Threading.Timeout.InfiniteTimeSpan,
44+
Credentials: null,
45+
PreAuthenticate: false,
46+
UseProxy: false,
47+
Proxy: null);
48+
49+
using var client = new GaudiHttpClient(
50+
requests.Writer,
51+
responses.Reader,
52+
options,
53+
new NamedClientConsumerRegistration(ActorRefs.Nobody, "test", Guid.NewGuid()));
54+
55+
// Fake pipeline: drain the request channel and complete each PendingRequest immediately.
56+
using var pipelineCts = new CancellationTokenSource();
57+
var pipeline = Task.Run(async () =>
58+
{
59+
try
60+
{
61+
await foreach (var req in requests.Reader.ReadAllAsync(pipelineCts.Token))
62+
{
63+
if (req.Options.TryGetValue(OptionsKey.Key, out var pending) &&
64+
req.Options.TryGetValue(OptionsKey.VersionKey, out var version))
65+
{
66+
pending.TrySetResult(new HttpResponseMessage(HttpStatusCode.OK), version);
67+
}
68+
}
69+
}
70+
catch (OperationCanceledException)
71+
{
72+
}
73+
});
74+
75+
var dispatched = 0;
76+
var completed = 0;
77+
var failed = 0;
78+
79+
async Task Worker()
80+
{
81+
while (Interlocked.Increment(ref dispatched) <= TotalRequests)
82+
{
83+
try
84+
{
85+
using var request = new HttpRequestMessage(HttpMethod.Get, "http://localhost/");
86+
using var response = await client.SendAsync(request, CancellationToken.None);
87+
Interlocked.Increment(ref completed);
88+
}
89+
catch
90+
{
91+
Interlocked.Increment(ref failed);
92+
}
93+
}
94+
}
95+
96+
var workers = new Task[Concurrency];
97+
var sw = Stopwatch.StartNew();
98+
for (var i = 0; i < Concurrency; i++)
99+
{
100+
workers[i] = Worker();
101+
}
102+
103+
try
104+
{
105+
await Task.WhenAll(workers).WaitAsync(Deadline);
106+
}
107+
catch (TimeoutException)
108+
{
109+
pipelineCts.Cancel();
110+
Assert.Fail(
111+
$"SendAsync completed only {Volatile.Read(ref completed):N0}/{TotalRequests:N0} requests within " +
112+
$"{Deadline.TotalSeconds:F0}s at concurrency {Concurrency} — quadratic in-flight request-tracking " +
113+
$"regression (lock-free pending list, commit 50918ff2).");
114+
}
115+
116+
sw.Stop();
117+
pipelineCts.Cancel();
118+
119+
Assert.Equal(0, failed);
120+
Assert.Equal(TotalRequests, completed);
121+
}
122+
}

0 commit comments

Comments
 (0)