Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 3 additions & 3 deletions src/ReverseProxy/Service/Proxy/HttpProxy.cs
Original file line number Diff line number Diff line change
Expand Up @@ -122,8 +122,8 @@ public async Task ProxyAsync(

// :: Step 4: Send the outgoing request using HttpClient
HttpResponseMessage destinationResponse;
var requestTimeoutSource = CancellationTokenSource.CreateLinkedTokenSource(requestAborted);
requestTimeoutSource.CancelAfter(requestOptions?.Timeout ?? DefaultTimeout);

var requestTimeoutSource = PooledCTS.Rent(requestOptions?.Timeout ?? DefaultTimeout, requestAborted);
var requestTimeoutToken = requestTimeoutSource.Token;
try
{
Expand Down Expand Up @@ -151,7 +151,7 @@ public async Task ProxyAsync(
}
finally
{
requestTimeoutSource.Dispose();
requestTimeoutSource.Return();
}

// Detect connection downgrade, which may be problematic for e.g. gRPC.
Expand Down
62 changes: 62 additions & 0 deletions src/ReverseProxy/Utilities/PooledCTS.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT License.

#nullable enable

using System;
using System.Collections.Concurrent;
using System.Diagnostics;
using System.Threading;

namespace Yarp.ReverseProxy.Utilities
{
internal sealed class PooledCTS : CancellationTokenSource
{
private static readonly ConcurrentQueue<PooledCTS> _sharedSources = new();

private static readonly Action<object?> _linkedTokenCancelDelegate = static s =>
{
((CancellationTokenSource)s!).Cancel(throwOnFirstException: false);
};

private const long SafeToReuseTicks = TimeSpan.TicksPerSecond * 10;
private static readonly double _stopwatchTicksPerTimeSpanTick = Stopwatch.Frequency / (double)TimeSpan.TicksPerSecond;

private long _safeToReuseBeforeTimestamp;
private CancellationTokenRegistration _registration;

public static PooledCTS Rent(TimeSpan timeout, CancellationToken linkedToken)
{
if (!_sharedSources.TryDequeue(out var cts))
{
cts = new PooledCTS();
}

cts._registration = linkedToken.UnsafeRegister(_linkedTokenCancelDelegate, cts);

cts._safeToReuseBeforeTimestamp = Stopwatch.GetTimestamp() + (long)((timeout.Ticks - SafeToReuseTicks) * _stopwatchTicksPerTimeSpanTick);
Copy link
Member

Choose a reason for hiding this comment

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

Do you get weird results if the timeout is less than SafeToReuseTicks (10s)?

10s is a arbitrary margin of safety. We should wait for the deterministic 6.0 API.

Copy link
Member Author

Choose a reason for hiding this comment

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

If the timeout is shorter than 10s, we would never reuse the token.

10s is arbitrary, but still huge - it means your app is already on fire.
As an extreme, would you be okay with a 90s safety on a 100s timeout - meaning we would only reuse the CTS if the request finished within 10 seconds?

Copy link
Member

Choose a reason for hiding this comment

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

It's hard to agree to a non-deterministic solution when we know a deterministic one is possible in 6.0. For a 1% improvement I'm content to wait for 6.0.


cts.CancelAfter(timeout);

return cts;
}

public void Return()
{
_registration.Dispose();
_registration = default;

// TODO: Use TryReset in 6.0+
CancelAfter(Timeout.Infinite);

if (IsCancellationRequested || Stopwatch.GetTimestamp() > _safeToReuseBeforeTimestamp)
{
Dispose();
}
else
{
_sharedSources.Enqueue(this);
}
}
}
}