Skip to content

fix: Connection issues at high throughput #25

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.

Sign up for GitHub

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

Open
lwaddicor wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: Connection issues at high throughput #25

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 11 additions & 5 deletions modules/AgonesAllocator/Project/AgonesAllocator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,18 +26,24 @@ public class ModuleConfig : ICloudCodeSetup

public void Setup(ICloudCodeConfig config)
{
config.Dependencies.AddScoped<IRequestAdapter>(_ =>
config.Dependencies.AddSingleton<IRequestAdapter>(_ =>
{
// TODO: Replace with required auth of your service
var authProvider = new AnonymousAuthenticationProvider();

var handler = new HttpClientHandler
var handler = new SocketsHttpHandler

@lujain-aleryani lujain-aleryani Sep 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consistency: here the handler is a non-static local captured by the singleton adapter (effectively singleton-scoped, default disposeHandler: true), whereas Edgegap/Gameye/RocketScience use a static readonly SharedHandler with disposeHandler: false. Both work, but standardizing on one pattern makes it safer to copy into the remaining modules. Suggest the static readonly form everywhere.

Optional: SocketsHttpHandler defaults to HTTP/1.1. If providers support h2, EnableMultipleHttp2Connections = true would multiplex many requests over a few connections — a bigger win against connection exhaustion than raising MaxConnectionsPerServer.

{
PooledConnectionLifetime = TimeSpan.FromMinutes(5),
PooledConnectionIdleTimeout = TimeSpan.FromMinutes(2),
MaxConnectionsPerServer = 300,

@lujain-aleryani lujain-aleryani Sep 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MaxConnectionsPerServer = 300 is an unexplained magic number (repeated across all modules). Under a multi-thousand burst this becomes the per-pool concurrency ceiling; requests beyond it queue and can then trip the 10s timeout. Add a one-line comment on why 300, and sanity-check it against the expected concurrent-poll count.

// TODO: Implement MTLS or other cert validation here
// ServerCertificateCustomValidationCallback = (_, _, _, _) => throw new NotImplementedException()
// SslOptions = new SslClientAuthenticationOptions { RemoteCertificateValidationCallback = (_, _, _, _) => throw new NotImplementedException() },
};

return new HttpClientRequestAdapter(authProvider, httpClient: new HttpClient(handler))

// Cloud Code cancels an invocation at 15s; fail with budget left to return an error.
var httpClient = new HttpClient(handler) { Timeout = TimeSpan.FromSeconds(10) };

return new HttpClientRequestAdapter(authProvider, httpClient: httpClient)
{
BaseUrl = AllocatorServiceBaseUrl
};
Expand Down
15 changes: 14 additions & 1 deletion modules/EdgegapAllocator/Project/Client/EdgegapClient.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using System;
using System.Net.Http;

namespace EdgegapAllocatorModule.Client;
Expand All @@ -9,9 +10,21 @@ public interface IEdgegapHttpClientFactory

public class EdgegapHttpClientFactory : IEdgegapHttpClientFactory
{
// Static so the connection pool outlives a single invocation; a per-call handler re-handshakes every request.
private static readonly SocketsHttpHandler SharedHandler = new()
{
PooledConnectionLifetime = TimeSpan.FromMinutes(5),
PooledConnectionIdleTimeout = TimeSpan.FromMinutes(2),
MaxConnectionsPerServer = 300
};

public HttpClient Create(string apiToken)
{
var client = new HttpClient();
// Cloud Code cancels an invocation at 15s; fail with budget left to return an error.
var client = new HttpClient(SharedHandler, disposeHandler: false)
{
Timeout = TimeSpan.FromSeconds(10)
};
client.DefaultRequestHeaders.Add("Authorization", $"{apiToken}");
return client;
}
Expand Down
6 changes: 3 additions & 3 deletions modules/EdgegapAllocator/Project/EdgegapAllocator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@
{
public void Setup(ICloudCodeConfig config)
{
config.Dependencies.AddSingleton(GameApiClient.Create());

Check warning on line 30 in modules/EdgegapAllocator/Project/EdgegapAllocator.cs

View workflow job for this annotation

GitHub Actions / Test Modules 

'GameApiClient.Create()' is obsolete: 'Use extension methods in ICloudCodeConfig class instead. Register IGameApiClient in your Setup method using config.Dependencies.'
config.Dependencies.AddScoped<IEdgegapHttpClientFactory, EdgegapHttpClientFactory>();
config.Dependencies.AddSingleton<IEdgegapHttpClientFactory, EdgegapHttpClientFactory>();
}
}

Expand Down Expand Up @@ -143,8 +143,8 @@
try
{
Secret edgegapApiToken = await gameApiClient.SecretManager.GetSecret(context, EdgegapApiTokenSecretName);
HttpClient client = httpClientFactory.Create(edgegapApiToken.Value);
HttpResponseMessage response = await client.GetAsync($"{EdgegapApiUrl}/v1/status/{requestId}");
using HttpClient client = httpClientFactory.Create(edgegapApiToken.Value);
using HttpResponseMessage response = await client.GetAsync($"{EdgegapApiUrl}/v1/status/{requestId}");

@lujain-aleryani lujain-aleryani Sep 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good — using on both the client and the response. For consistency, apply the same disposal to the Gameye and RocketScience call sites (they weren't updated). Not a connection leak since the shared handler isn't disposed, but disposing the HttpResponseMessage everywhere is the right habit.

string responseContent = await response.Content.ReadAsStringAsync();

if (!response.IsSuccessStatusCode)
Expand Down
15 changes: 14 additions & 1 deletion modules/GameyeAllocator/Project/Client/GameyeClient.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using System;
using System.Net.Http;

namespace GameyeAllocatorModule.Client;
Expand All @@ -9,9 +10,21 @@ public interface IGameyeHttpClientFactory

public class GameyeHttpClientFactory : IGameyeHttpClientFactory
{
// Static so the connection pool outlives a single invocation; a per-call handler re-handshakes every request.
private static readonly SocketsHttpHandler SharedHandler = new()
{
PooledConnectionLifetime = TimeSpan.FromMinutes(5),
PooledConnectionIdleTimeout = TimeSpan.FromMinutes(2),
MaxConnectionsPerServer = 300
};

public HttpClient Create(string apiToken)
{
var client = new HttpClient();
// Cloud Code cancels an invocation at 15s; fail with budget left to return an error.
var client = new HttpClient(SharedHandler, disposeHandler: false)
{
Timeout = TimeSpan.FromSeconds(10)
};
client.DefaultRequestHeaders.Add("Authorization", $"Bearer {apiToken}");
return client;
}
Expand Down
2 changes: 1 addition & 1 deletion modules/GameyeAllocator/Project/GameyeAllocator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@
{
public void Setup(ICloudCodeConfig config)
{
config.Dependencies.AddSingleton(GameApiClient.Create());

Check warning on line 28 in modules/GameyeAllocator/Project/GameyeAllocator.cs

View workflow job for this annotation

GitHub Actions / Test Modules 

'GameApiClient.Create()' is obsolete: 'Use extension methods in ICloudCodeConfig class instead. Register IGameApiClient in your Setup method using config.Dependencies.'
config.Dependencies.AddScoped<IGameyeHttpClientFactory, GameyeHttpClientFactory>();
config.Dependencies.AddSingleton<IGameyeHttpClientFactory, GameyeHttpClientFactory>();

// ──────────────────────────────────────────────────────────────
// Gameye allocator configuration — edit the values below.
Expand Down
16 changes: 14 additions & 2 deletions modules/RocketScienceAllocator/Project/RocketScienceAllocator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@
{
public void Setup(ICloudCodeConfig config)
{
config.Dependencies.AddSingleton(GameApiClient.Create());

Check warning on line 20 in modules/RocketScienceAllocator/Project/RocketScienceAllocator.cs

View workflow job for this annotation

GitHub Actions / Test Modules 

'GameApiClient.Create()' is obsolete: 'Use extension methods in ICloudCodeConfig class instead. Register IGameApiClient in your Setup method using config.Dependencies.'
config.Dependencies.AddScoped<IRocketScienceHttpClientFactory, RocketScienceHttpClientFactory>();
config.Dependencies.AddSingleton<IRocketScienceHttpClientFactory, RocketScienceHttpClientFactory>();
}
}

Expand Down Expand Up @@ -173,9 +173,21 @@

public class RocketScienceHttpClientFactory : IRocketScienceHttpClientFactory
{
// Static so the connection pool outlives a single invocation; a per-call handler re-handshakes every request.
private static readonly SocketsHttpHandler SharedHandler = new()

@lujain-aleryani lujain-aleryani Sep 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Key assumption to validate: this only helps if the static handler (and the pool) actually persists across Cloud Code invocations. If CC runs each invocation in a fresh/gated worker, the pool is rebuilt every call and the reuse benefit disappears. Recommend confirming CC worker lifetime, and/or logging once on handler construction to measure reuse in practice.

{
PooledConnectionLifetime = TimeSpan.FromMinutes(5),
PooledConnectionIdleTimeout = TimeSpan.FromMinutes(2),
MaxConnectionsPerServer = 300
};

public HttpClient Create(string apiKey)
{
var client = new HttpClient();
// Cloud Code cancels an invocation at 15s; fail with budget left to return an error.
var client = new HttpClient(SharedHandler, disposeHandler: false)
{
Timeout = TimeSpan.FromSeconds(10)
};
client.DefaultRequestHeaders.Authorization = new AuthenticationHeaderValue("Bearer", apiKey);
return client;
}
Expand Down
Loading