Skip to content
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

Jump to bottom

Add EnableMultipleHttp2Connections property to HttpHandlerOptions #2126

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open

Add EnableMultipleHttp2Connections property to HttpHandlerOptions #2126

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f760b11
feat: add EnableMultipleHttp2Connections
Jul 21, 2024
8b92cd0
feat: update doc
Jul 21, 2024
5921a9c
Sort properties
raman-m Jul 23, 2024
3efc97c
fix: apply comment
EngRajabi Aug 21, 2024
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: 13 additions & 3 deletions docs/features/configuration.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,8 @@ Use ``HttpHandlerOptions`` in a Route configuration to set up ``HttpHandler`` be
"AllowAutoRedirect": false,
"UseCookieContainer": false,
"UseTracing": true,
"MaxConnectionsPerServer": 100
"MaxConnectionsPerServer": 100,
"EnableMultipleHttp2Connections": false
EngRajabi marked this conversation as resolved.
Show resolved Hide resolved
},

* **AllowAutoRedirect** is a value that indicates whether the request should follow redirection responses.
Expand All @@ -260,6 +261,9 @@ Use ``HttpHandlerOptions`` in a Route configuration to set up ``HttpHandler`` be

* **MaxConnectionsPerServer** This controls how many connections the internal ``HttpClient`` will open. This can be set at Route or global level.

* **EnableMultipleHttp2Connections** Gets or sets a value that indicates whether additional HTTP/2 connections can be established to the same server.
true if additional HTTP/2 connections are allowed to be created; otherwise, false.

.. _ssl-errors:

SSL Errors
Expand Down Expand Up @@ -394,7 +398,10 @@ HTTP/2 version policy
"DownstreamScheme": "https",
"DownstreamHttpVersion": "2.0",
"DownstreamHttpVersionPolicy": "", // empty
"DangerousAcceptAnyServerCertificateValidator": true
"DangerousAcceptAnyServerCertificateValidator": true,
"HttpHandlerOptions":{
"EnableMultipleHttp2Connections": true
EngRajabi marked this conversation as resolved.
Show resolved Hide resolved
}
}

**And** you configure global settings to use Kestrel with this snippet:
Expand Down Expand Up @@ -425,7 +432,10 @@ Therefore, the ``DownstreamHttpVersionPolicy`` should be defined as follows:

{
"DownstreamHttpVersion": "2.0",
"DownstreamHttpVersionPolicy": "RequestVersionOrHigher" // !
"DownstreamHttpVersionPolicy": "RequestVersionOrHigher", // !
"HttpHandlerOptions":{
"EnableMultipleHttp2Connections": true
}
}

Dependency Injection
Expand Down
3 changes: 2 additions & 1 deletion src/Ocelot/Configuration/Creator/HttpHandlerOptionsCreator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ public HttpHandlerOptions Create(FileHttpHandlerOptions options)
var pooledConnectionLifetime = TimeSpan.FromSeconds(options.PooledConnectionLifetimeSeconds ?? DefaultPooledConnectionLifetimeSeconds);

return new HttpHandlerOptions(options.AllowAutoRedirect,
options.UseCookieContainer, useTracing, options.UseProxy, maxConnectionPerServer, pooledConnectionLifetime);
options.UseCookieContainer, useTracing, options.UseProxy, maxConnectionPerServer, pooledConnectionLifetime,
options.EnableMultipleHttp2Connections);
Comment on lines +28 to +29
Copy link
Member

Choose a reason for hiding this comment

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

Certainly, extending the default advanced initialization constructor with all initialization values is beneficial. However, it's also worth considering the introduction of a copy constructor.

Having the copy-constructor we can write:

            return new HttpHandlerOptions(options);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@raman-m
How to transfer the conditions of lines 21, 24 and 25? Line 21 uses ITracer

}
}
}
3 changes: 3 additions & 0 deletions src/Ocelot/Configuration/File/FileHttpHandlerOptions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ public FileHttpHandlerOptions()
UseCookieContainer = false;
UseProxy = true;
PooledConnectionLifetimeSeconds = null;
EnableMultipleHttp2Connections = false;
EngRajabi marked this conversation as resolved.
Show resolved Hide resolved
}

public FileHttpHandlerOptions(FileHttpHandlerOptions from)
Expand All @@ -18,6 +19,7 @@ public FileHttpHandlerOptions(FileHttpHandlerOptions from)
UseCookieContainer = from.UseCookieContainer;
UseProxy = from.UseProxy;
PooledConnectionLifetimeSeconds = from.PooledConnectionLifetimeSeconds;
EnableMultipleHttp2Connections = from.EnableMultipleHttp2Connections;
}

public bool AllowAutoRedirect { get; set; }
Expand All @@ -26,5 +28,6 @@ public FileHttpHandlerOptions(FileHttpHandlerOptions from)
public bool UseProxy { get; set; }
public bool UseTracing { get; set; }
public int? PooledConnectionLifetimeSeconds { get; set; }
public bool EnableMultipleHttp2Connections { get; set; }
}
}
9 changes: 8 additions & 1 deletion src/Ocelot/Configuration/HttpHandlerOptions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,15 @@
public class HttpHandlerOptions
{
public HttpHandlerOptions(bool allowAutoRedirect, bool useCookieContainer, bool useTracing, bool useProxy,
int maxConnectionsPerServer, TimeSpan pooledConnectionLifeTime)
int maxConnectionsPerServer, TimeSpan pooledConnectionLifeTime, bool enableMultipleHttp2Connections)
Copy link
Member

Choose a reason for hiding this comment

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

Lack of copy-constructor.

{
AllowAutoRedirect = allowAutoRedirect;
UseCookieContainer = useCookieContainer;
UseTracing = useTracing;
UseProxy = useProxy;
MaxConnectionsPerServer = maxConnectionsPerServer;
PooledConnectionLifeTime = pooledConnectionLifeTime;
EnableMultipleHttp2Connections = enableMultipleHttp2Connections;
}

/// <summary>
Expand Down Expand Up @@ -51,5 +52,11 @@ public HttpHandlerOptions(bool allowAutoRedirect, bool useCookieContainer, bool
/// </summary>
/// <value>PooledConnectionLifeTime.</value>
public TimeSpan PooledConnectionLifeTime { get; }

/// <summary>
/// Gets or sets a value that indicates whether additional HTTP/2 connections can be established to the same server.
/// </summary>
/// <value>EnableMultipleHttp2Connections.</value>
public bool EnableMultipleHttp2Connections { get; }
}
}
9 changes: 8 additions & 1 deletion src/Ocelot/Configuration/HttpHandlerOptionsBuilder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ public class HttpHandlerOptionsBuilder
private bool _useProxy;
private int _maxConnectionPerServer;
private TimeSpan _pooledConnectionLifetime = TimeSpan.FromSeconds(HttpHandlerOptionsCreator.DefaultPooledConnectionLifetimeSeconds);
private bool _enableMultipleHttp2Connections;

public HttpHandlerOptionsBuilder WithAllowAutoRedirect(bool input)
{
Expand Down Expand Up @@ -47,9 +48,15 @@ public HttpHandlerOptionsBuilder WithPooledConnectionLifetimeSeconds(TimeSpan po
return this;
}

public HttpHandlerOptionsBuilder WithEnableMultipleHttp2Connections(bool enableMultipleHttp2Connections)
{
_enableMultipleHttp2Connections = enableMultipleHttp2Connections;
return this;
}

public HttpHandlerOptions Build()
{
return new HttpHandlerOptions(_allowAutoRedirect, _useCookieContainer, _useTracing, _useProxy, _maxConnectionPerServer, _pooledConnectionLifetime);
return new HttpHandlerOptions(_allowAutoRedirect, _useCookieContainer, _useTracing, _useProxy, _maxConnectionPerServer, _pooledConnectionLifetime, _enableMultipleHttp2Connections);
}
}
}
5 changes: 3 additions & 2 deletions src/Ocelot/Requester/MessageInvokerPool.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,10 +73,11 @@ private HttpMessageHandler CreateHandler(DownstreamRoute downstreamRoute)
var handler = new SocketsHttpHandler
{
AllowAutoRedirect = downstreamRoute.HttpHandlerOptions.AllowAutoRedirect,
UseCookies = downstreamRoute.HttpHandlerOptions.UseCookieContainer,
UseProxy = downstreamRoute.HttpHandlerOptions.UseProxy,
EnableMultipleHttp2Connections = downstreamRoute.HttpHandlerOptions.EnableMultipleHttp2Connections,
MaxConnectionsPerServer = downstreamRoute.HttpHandlerOptions.MaxConnectionsPerServer,
PooledConnectionLifetime = downstreamRoute.HttpHandlerOptions.PooledConnectionLifeTime,
UseCookies = downstreamRoute.HttpHandlerOptions.UseCookieContainer,
UseProxy = downstreamRoute.HttpHandlerOptions.UseProxy,
};

if (downstreamRoute.HttpHandlerOptions.UseCookieContainer)
Expand Down
2 changes: 1 addition & 1 deletion test/Ocelot.UnitTests/Configuration/DownstreamRouteExtensionsTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ public DownstreamRouteExtensionsTests()
new List<DownstreamHostAndPort>(),
null,
null,
new HttpHandlerOptions(false, false, false, false, 0, TimeSpan.Zero),
new HttpHandlerOptions(false, false, false, false, 0, TimeSpan.Zero, false),
Copy link
Member

Choose a reason for hiding this comment

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

Why not to define default constructor with default initialization values including a value for EnableMultipleHttp2Connections which is false?

default,
default,
new QoSOptions(0, 0, 0, null),
Expand Down
38 changes: 29 additions & 9 deletions test/Ocelot.UnitTests/Configuration/HttpHandlerOptionsCreatorTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ public void should_not_use_tracing_if_fake_tracer_registered()
},
};

var expectedOptions = new HttpHandlerOptions(false, false, false, true, int.MaxValue, DefaultPooledConnectionLifeTime);
var expectedOptions = new HttpHandlerOptions(false, false, false, true, int.MaxValue, DefaultPooledConnectionLifeTime, false);

this.Given(x => GivenTheFollowing(fileRoute))
.When(x => WhenICreateHttpHandlerOptions())
Expand All @@ -52,7 +52,7 @@ public void should_use_tracing_if_real_tracer_registered()
},
};

var expectedOptions = new HttpHandlerOptions(false, false, true, true, int.MaxValue, DefaultPooledConnectionLifeTime);
var expectedOptions = new HttpHandlerOptions(false, false, true, true, int.MaxValue, DefaultPooledConnectionLifeTime, false);

this.Given(x => GivenTheFollowing(fileRoute))
.And(x => GivenARealTracer())
Expand All @@ -65,7 +65,7 @@ public void should_use_tracing_if_real_tracer_registered()
public void should_create_options_with_useCookie_false_and_allowAutoRedirect_true_as_default()
{
var fileRoute = new FileRoute();
var expectedOptions = new HttpHandlerOptions(false, false, false, true, int.MaxValue, DefaultPooledConnectionLifeTime);
var expectedOptions = new HttpHandlerOptions(false, false, false, true, int.MaxValue, DefaultPooledConnectionLifeTime, false);

this.Given(x => GivenTheFollowing(fileRoute))
.When(x => WhenICreateHttpHandlerOptions())
Expand All @@ -86,7 +86,7 @@ public void should_create_options_with_specified_useCookie_and_allowAutoRedirect
},
};

var expectedOptions = new HttpHandlerOptions(false, false, false, true, int.MaxValue, DefaultPooledConnectionLifeTime);
var expectedOptions = new HttpHandlerOptions(false, false, false, true, int.MaxValue, DefaultPooledConnectionLifeTime, false);

this.Given(x => GivenTheFollowing(fileRoute))
.When(x => WhenICreateHttpHandlerOptions())
Expand All @@ -102,7 +102,7 @@ public void should_create_options_with_useproxy_true_as_default()
HttpHandlerOptions = new FileHttpHandlerOptions(),
};

var expectedOptions = new HttpHandlerOptions(false, false, false, true, int.MaxValue, DefaultPooledConnectionLifeTime);
var expectedOptions = new HttpHandlerOptions(false, false, false, true, int.MaxValue, DefaultPooledConnectionLifeTime, false);

this.Given(x => GivenTheFollowing(fileRoute))
.When(x => WhenICreateHttpHandlerOptions())
Expand All @@ -121,7 +121,7 @@ public void should_create_options_with_specified_useproxy()
},
};

var expectedOptions = new HttpHandlerOptions(false, false, false, false, int.MaxValue, DefaultPooledConnectionLifeTime);
var expectedOptions = new HttpHandlerOptions(false, false, false, false, int.MaxValue, DefaultPooledConnectionLifeTime, false);

this.Given(x => GivenTheFollowing(fileRoute))
.When(x => WhenICreateHttpHandlerOptions())
Expand All @@ -140,7 +140,7 @@ public void should_create_options_with_specified_MaxConnectionsPerServer()
},
};

var expectedOptions = new HttpHandlerOptions(false, false, false, true, 10, DefaultPooledConnectionLifeTime);
var expectedOptions = new HttpHandlerOptions(false, false, false, true, 10, DefaultPooledConnectionLifeTime, false);

this.Given(x => GivenTheFollowing(fileRoute))
.When(x => WhenICreateHttpHandlerOptions())
Expand All @@ -159,7 +159,7 @@ public void should_create_options_fixing_specified_MaxConnectionsPerServer_range
},
};

var expectedOptions = new HttpHandlerOptions(false, false, false, true, int.MaxValue, DefaultPooledConnectionLifeTime);
var expectedOptions = new HttpHandlerOptions(false, false, false, true, int.MaxValue, DefaultPooledConnectionLifeTime, false);

this.Given(x => GivenTheFollowing(fileRoute))
.When(x => WhenICreateHttpHandlerOptions())
Expand All @@ -178,7 +178,26 @@ public void should_create_options_fixing_specified_MaxConnectionsPerServer_range
},
};

var expectedOptions = new HttpHandlerOptions(false, false, false, true, int.MaxValue, DefaultPooledConnectionLifeTime);
var expectedOptions = new HttpHandlerOptions(false, false, false, true, int.MaxValue, DefaultPooledConnectionLifeTime, false);

this.Given(x => GivenTheFollowing(fileRoute))
.When(x => WhenICreateHttpHandlerOptions())
.Then(x => ThenTheFollowingOptionsReturned(expectedOptions))
.BDDfy();
}

[Fact]
Copy link
Member

Choose a reason for hiding this comment

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

Lack of Traits

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did not understand what you mean

Copy link
Member

Choose a reason for hiding this comment

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

I mean this

Ocelot/test/Ocelot.UnitTests/Requester/MessageInvokerPoolTests.cs

Line 131 in ce0227c

[Trait("Issue", "1833")]

public void should_create_options_with_enableMultipleHttp2Connections()
{
var fileRoute = new FileRoute
{
HttpHandlerOptions = new FileHttpHandlerOptions
{
EnableMultipleHttp2Connections = true,
},
};

var expectedOptions = new HttpHandlerOptions(false, false, false, true, int.MaxValue, DefaultPooledConnectionLifeTime, true);

this.Given(x => GivenTheFollowing(fileRoute))
.When(x => WhenICreateHttpHandlerOptions())
Expand All @@ -204,6 +223,7 @@ private void ThenTheFollowingOptionsReturned(HttpHandlerOptions expected)
_httpHandlerOptions.UseTracing.ShouldBe(expected.UseTracing);
_httpHandlerOptions.UseProxy.ShouldBe(expected.UseProxy);
_httpHandlerOptions.MaxConnectionsPerServer.ShouldBe(expected.MaxConnectionsPerServer);
_httpHandlerOptions.EnableMultipleHttp2Connections.ShouldBe(expected.EnableMultipleHttp2Connections);
}

private void GivenARealTracer()
Expand Down
24 changes: 12 additions & 12 deletions test/Ocelot.UnitTests/Requester/DelegatingHandlerHandlerProviderFactoryTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public void should_follow_ordering_add_specifics()

var route = new DownstreamRouteBuilder()
.WithQosOptions(qosOptions)
.WithHttpHandlerOptions(new HttpHandlerOptions(true, true, true, true, int.MaxValue, DefaultPooledConnectionLifeTime))
.WithHttpHandlerOptions(new HttpHandlerOptions(true, true, true, true, int.MaxValue, DefaultPooledConnectionLifeTime, false))
.WithDelegatingHandlers(new List<string>
{
"FakeDelegatingHandler",
Expand Down Expand Up @@ -82,7 +82,7 @@ public void should_follow_ordering_order_specifics_and_globals()

var route = new DownstreamRouteBuilder()
.WithQosOptions(qosOptions)
.WithHttpHandlerOptions(new HttpHandlerOptions(true, true, true, true, int.MaxValue, DefaultPooledConnectionLifeTime))
.WithHttpHandlerOptions(new HttpHandlerOptions(true, true, true, true, int.MaxValue, DefaultPooledConnectionLifeTime, false))
.WithDelegatingHandlers(new List<string>
{
"FakeDelegatingHandlerTwo",
Expand Down Expand Up @@ -119,7 +119,7 @@ public void should_follow_ordering_order_specifics()

var route = new DownstreamRouteBuilder()
.WithQosOptions(qosOptions)
.WithHttpHandlerOptions(new HttpHandlerOptions(true, true, true, true, int.MaxValue, DefaultPooledConnectionLifeTime))
.WithHttpHandlerOptions(new HttpHandlerOptions(true, true, true, true, int.MaxValue, DefaultPooledConnectionLifeTime, false))
.WithDelegatingHandlers(new List<string>
{
"FakeDelegatingHandlerTwo",
Expand Down Expand Up @@ -155,7 +155,7 @@ public void should_follow_ordering_order_and_only_add_specifics_in_config()

var route = new DownstreamRouteBuilder()
.WithQosOptions(qosOptions)
.WithHttpHandlerOptions(new HttpHandlerOptions(true, true, true, true, int.MaxValue, DefaultPooledConnectionLifeTime))
.WithHttpHandlerOptions(new HttpHandlerOptions(true, true, true, true, int.MaxValue, DefaultPooledConnectionLifeTime, false))
.WithDelegatingHandlers(new List<string>
{
"FakeDelegatingHandler",
Expand Down Expand Up @@ -189,7 +189,7 @@ public void should_follow_ordering_dont_add_specifics()

var route = new DownstreamRouteBuilder()
.WithQosOptions(qosOptions)
.WithHttpHandlerOptions(new HttpHandlerOptions(true, true, true, true, int.MaxValue, DefaultPooledConnectionLifeTime))
.WithHttpHandlerOptions(new HttpHandlerOptions(true, true, true, true, int.MaxValue, DefaultPooledConnectionLifeTime, false))
.WithLoadBalancerKey(string.Empty)
.Build();

Expand All @@ -215,7 +215,7 @@ public void should_apply_re_route_specific()

var route = new DownstreamRouteBuilder()
.WithQosOptions(qosOptions)
.WithHttpHandlerOptions(new HttpHandlerOptions(true, true, false, true, int.MaxValue, DefaultPooledConnectionLifeTime))
.WithHttpHandlerOptions(new HttpHandlerOptions(true, true, false, true, int.MaxValue, DefaultPooledConnectionLifeTime, false))
.WithDelegatingHandlers(new List<string>
{
"FakeDelegatingHandler",
Expand Down Expand Up @@ -243,7 +243,7 @@ public void should_all_from_all_routes_provider_and_qos()

var route = new DownstreamRouteBuilder()
.WithQosOptions(qosOptions)
.WithHttpHandlerOptions(new HttpHandlerOptions(true, true, false, true, int.MaxValue, DefaultPooledConnectionLifeTime))
.WithHttpHandlerOptions(new HttpHandlerOptions(true, true, false, true, int.MaxValue, DefaultPooledConnectionLifeTime, false))
.WithLoadBalancerKey(string.Empty)
.Build();

Expand All @@ -265,7 +265,7 @@ public void should_return_provider_with_no_delegates()

var route = new DownstreamRouteBuilder()
.WithQosOptions(qosOptions)
.WithHttpHandlerOptions(new HttpHandlerOptions(true, true, false, true, int.MaxValue, DefaultPooledConnectionLifeTime))
.WithHttpHandlerOptions(new HttpHandlerOptions(true, true, false, true, int.MaxValue, DefaultPooledConnectionLifeTime, false))
.WithLoadBalancerKey(string.Empty)
.Build();

Expand All @@ -287,7 +287,7 @@ public void should_return_provider_with_qos_delegate()

var route = new DownstreamRouteBuilder()
.WithQosOptions(qosOptions)
.WithHttpHandlerOptions(new HttpHandlerOptions(true, true, false, true, int.MaxValue, DefaultPooledConnectionLifeTime))
.WithHttpHandlerOptions(new HttpHandlerOptions(true, true, false, true, int.MaxValue, DefaultPooledConnectionLifeTime, false))
.WithLoadBalancerKey(string.Empty)
.Build();

Expand All @@ -309,7 +309,7 @@ public void should_return_provider_with_qos_delegate_when_timeout_value_set()

var route = new DownstreamRouteBuilder()
.WithQosOptions(qosOptions)
.WithHttpHandlerOptions(new HttpHandlerOptions(true, true, false, true, int.MaxValue, DefaultPooledConnectionLifeTime))
.WithHttpHandlerOptions(new HttpHandlerOptions(true, true, false, true, int.MaxValue, DefaultPooledConnectionLifeTime, false))
.WithLoadBalancerKey(string.Empty)
.Build();

Expand All @@ -333,7 +333,7 @@ public void should_log_error_and_return_no_qos_provider_delegate_when_qos_factor

var route = new DownstreamRouteBuilder()
.WithQosOptions(qosOptions)
.WithHttpHandlerOptions(new HttpHandlerOptions(true, true, true, true, int.MaxValue, DefaultPooledConnectionLifeTime))
.WithHttpHandlerOptions(new HttpHandlerOptions(true, true, true, true, int.MaxValue, DefaultPooledConnectionLifeTime, false))
.WithLoadBalancerKey(string.Empty)
.Build();

Expand Down Expand Up @@ -363,7 +363,7 @@ public void should_log_error_and_return_no_qos_provider_delegate_when_qos_factor

var route = new DownstreamRouteBuilder()
.WithQosOptions(qosOptions)
.WithHttpHandlerOptions(new HttpHandlerOptions(true, true, true, true, int.MaxValue, DefaultPooledConnectionLifeTime))
.WithHttpHandlerOptions(new HttpHandlerOptions(true, true, true, true, int.MaxValue, DefaultPooledConnectionLifeTime, false))
.WithLoadBalancerKey(string.Empty)
.Build();

Expand Down
Loading