Skip to content

Commit e02f08c

Browse files
committed
Several fixes related to HttpClient + .NET Core + linux (#2785)
* ./build.sh now works on linux environments that are not Travis * ConnectionLimit is now variable when using CurlHandler based on logical processor count, remains 80 otherwise. * reversable commit that allows connectionreuse test + number of connections to be easily specified on the command line * Correct check for Mono * bump to fake 5 because latest 4.x changes System.Console.OutputEncoding in a way that breaks all .NET applications writing to console output afterwards Conflicts: .paket/Paket.Restore.targets paket.lock * make sure ExecProcess and all Fake Tasks that call it write their output on the trace * implemented nitpicks from PR review Conflicts: src/Tests/Framework/ManagedElasticsearch/Nodes/ElasticsearchNode.cs
1 parent fadeb30 commit e02f08c

File tree

19 files changed

+201
-94
lines changed

19 files changed

+201
-94
lines changed

build.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,4 +5,4 @@ BUILDSCRIPT="build/scripts/Targets.fsx"
55
mono .paket/paket.bootstrapper.exe
66
if [[ -f .paket.lock ]]; then mono .paket/paket.exe restore; fi
77
if [[ ! -f .paket.lock ]]; then mono .paket/paket.exe install; fi
8-
mono $FAKE $@ --fsiargs -d:MONO $BUILDSCRIPT "cmdline=$@"
8+
mono $FAKE $BUILDSCRIPT "cmdline=$*" --fsiargs -d:MONO

build/Clients.Common.targets

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,12 @@
55
<CurrentVersion>0.0.0-bad</CurrentVersion>
66
<CurrentAssemblyVersion>0.0.0</CurrentAssemblyVersion>
77
<CurrentAssemblyFileVersion>0.0.0.0</CurrentAssemblyFileVersion>
8-
<DotNetCoreOnly></DotNetCoreOnly>
8+
<!-- 'dotnet xunit' does a build but has no way to pass properties or prevent build so we snoop for TRAVIS here
9+
Ideally we handle this in the FAKE script (which we do for 'dotnet build')
10+
TODO too lazy to write and test a <CHOOSE>
11+
-->
12+
<DotNetCoreOnly Condition="'$(TRAVIS)'=='true'">1</DotNetCoreOnly>
13+
<DotNetCoreOnly Condition="'$(TRAVIS)'==''"></DotNetCoreOnly>
914
<DoSourceLink></DoSourceLink>
1015

1116
<!-- Version and Informational reflect actual version -->
@@ -16,20 +21,13 @@
1621
<!-- File version reflects actual version number without prelease since that not allowed in its struct -->
1722
<FileVersion>$(CurrentAssemblyFileVersion)</FileVersion>
1823

19-
<!-- 'dotnet xunit' does a build but has no way to pass properties or prevent build so we snoop for TRAVIS here
20-
Ideally we handle this in the FAKE script (which we do for 'dotnet build')
21-
TODO too lazy to write and test a <CHOOSE>
22-
-->
23-
<DotNetCoreOnly Condition="'$(TRAVIS)'=='true'">1</DotNetCoreOnly>
24-
<DotNetCoreOnly Condition="'$(TRAVIS)'==''"></DotNetCoreOnly>
25-
<DoSourceLink></DoSourceLink>
26-
<SignAssembly>true</SignAssembly>
24+
<SignAssembly Condition="'$(DotNetCoreOnly)'==''">true</SignAssembly>
2725
<AssemblyOriginatorKeyFile Condition="'$(DotNetCoreOnly)'==''">..\..\build\keys\keypair.snk</AssemblyOriginatorKeyFile>
2826
<GenerateDocumentationFile Condition="'$(DotNetCoreOnly)'==''">true</GenerateDocumentationFile>
2927
<NoWarn>1591,1572,1571,1573,1587,1570</NoWarn>
3028
<Prefer32Bit>false</Prefer32Bit>
3129
<DefineConstants Condition="'$(TargetFramework)'=='netstandard1.3' OR '$(DotNetCoreOnly)'=='1'">$(DefineConstants);DOTNETCORE</DefineConstants>
32-
<DebugType>embedded</DebugType>
30+
<DebugType Condition="'$(DotNetCoreOnly)'==''">embedded</DebugType>
3331
<SourceLink Condition="'$(DoSourceLink)'!=''">$(BaseIntermediateOutputPath)\sl-$(MsBuildProjectName)-$(TargetFramework).json</SourceLink>
3432
<RepoUri>https://raw.githubusercontent.com/elastic/elasticsearch-net</RepoUri>
3533
</PropertyGroup>

build/scripts/Building.fsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,8 @@ module Build =
2323

2424
type private GlobalJson = JsonProvider<"../../global.json">
2525
let private pinnedSdkVersion = GlobalJson.GetSample().Sdk.Version
26-
27-
let private buildingOnTravis = getEnvironmentVarAsBool "TRAVIS"
26+
if isMono then setProcessEnvironVar "TRAVIS" "true"
27+
let private buildingOnTravis = getEnvironmentVarAsBool "TRAVIS"
2828

2929
let private sln = sprintf "src/Elasticsearch%s.sln" (if buildingOnTravis then ".DotNetCoreOnly" else "")
3030

build/scripts/Commandline.fsx

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,6 @@ module Commandline =
5151
| _ -> "build"
5252

5353
let needsFullBuild =
54-
printfn "%s, %A" target skipTests
55-
5654
match (target, skipTests) with
5755
| (_, true) -> true
5856
//dotnet-xunit needs to a build of its own anyways
@@ -67,6 +65,7 @@ module Commandline =
6765

6866
let parse () =
6967
setEnvironVar "FAKEBUILD" "1"
68+
printfn "%A" arguments
7069
match arguments with
7170
| [] | ["build"] | ["test"] | ["clean"] -> ignore()
7271
| ["release"; version] -> setBuildParam "version" version
@@ -87,6 +86,14 @@ module Commandline =
8786
setBuildParam "clusterfilter" clusterFilter
8887
setBuildParam "testfilter" testFilter
8988

89+
| ["connectionreuse"; esVersions] ->
90+
setBuildParam "esversions" esVersions
91+
setBuildParam "clusterfilter" "ConnectionReuse"
92+
| ["connectionreuse"; esVersions; numberOfConnections] ->
93+
setBuildParam "esversions" esVersions
94+
setBuildParam "clusterfilter" "ConnectionReuse"
95+
setBuildParam "numberOfConnections" numberOfConnections
96+
9097
| ["canary"; ] -> ignore()
9198
| ["canary"; apiKey ] ->
9299
setBuildParam "apiKey" apiKey
@@ -98,5 +105,5 @@ module Commandline =
98105
traceError usage
99106
exit 2
100107

101-
setBuildParam "target" target
108+
setBuildParam "target" (if target = "connectionreuse" then "integrate" else target)
102109
traceHeader target

build/scripts/Testing.fsx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,10 @@ module Tests =
2121
let private setLocalEnvVars() =
2222
let clusterFilter = getBuildParamOrDefault "clusterfilter" ""
2323
let testFilter = getBuildParamOrDefault "testfilter" ""
24+
let numberOfConnections = getBuildParamOrDefault "numberOfConnections" ""
2425
setProcessEnvironVar "NEST_INTEGRATION_CLUSTER" clusterFilter
2526
setProcessEnvironVar "NEST_TEST_FILTER" testFilter
27+
setProcessEnvironVar "NEST_NUMBER_OF_CONNECTIONS" numberOfConnections
2628

2729
let private dotnetTest (target: Commandline.MultiTarget) =
2830
CreateDir Paths.BuildOutput

build/scripts/Tooling.fsx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ open System.Net
1111

1212
open Fake
1313

14+
Fake.ProcessHelper.redirectOutputToTrace <-true
15+
1416
module Tooling =
1517
open Paths
1618
open Projects

src/Elasticsearch.Net/Configuration/ConnectionConfiguration.cs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,16 @@ namespace Elasticsearch.Net
2121
/// </summary>
2222
public class ConnectionConfiguration : ConnectionConfiguration<ConnectionConfiguration>
2323
{
24+
internal static bool IsCurlHandler { get; } =
25+
#if DOTNETCORE
26+
typeof(HttpClientHandler).Assembly().GetType("System.Net.Http.CurlHandler") != null;
27+
#else
28+
false;
29+
#endif
2430
public static readonly TimeSpan DefaultTimeout = TimeSpan.FromMinutes(1);
2531
public static readonly TimeSpan DefaultPingTimeout = TimeSpan.FromSeconds(2);
2632
public static readonly TimeSpan DefaultPingTimeoutOnSSL = TimeSpan.FromSeconds(5);
27-
public static readonly int DefaultConnectionLimit = 80;
33+
public static readonly int DefaultConnectionLimit = IsCurlHandler ? Environment.ProcessorCount : 80;
2834

2935
/// <summary>
3036
/// ConnectionConfiguration allows you to control how ElasticLowLevelClient behaves and where/how it connects

src/Elasticsearch.Net/Connection/HttpConnection-CoreFx.cs

Lines changed: 63 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -35,47 +35,58 @@ public class HttpConnection : IConnection
3535

3636
protected readonly ConcurrentDictionary<int, HttpClient> Clients = new ConcurrentDictionary<int, HttpClient>();
3737

38+
private static readonly string CanNotUseStreamResponsesWithCurlHandler =
39+
"Using Stream as TReturn does not work as expected on .NET core linux, because we can no longer guarantee this works it will be removed from the client in our 6.0 release"
40+
;
41+
3842
private HttpClient GetClient(RequestData requestData)
3943
{
4044
var key = GetClientKey(requestData);
4145
HttpClient client;
42-
if (!this.Clients.TryGetValue(key, out client))
46+
if (this.Clients.TryGetValue(key, out client)) return client;
47+
lock (_lock)
4348
{
44-
lock (_lock)
49+
client = this.Clients.GetOrAdd(key, h =>
4550
{
46-
client = this.Clients.GetOrAdd(key, h =>
51+
var handler = CreateHttpClientHandler(requestData);
52+
var httpClient = new HttpClient(handler, false)
4753
{
48-
var handler = CreateHttpClientHandler(requestData);
49-
var httpClient = new HttpClient(handler, false)
50-
{
51-
Timeout = requestData.RequestTimeout
52-
};
53-
54-
httpClient.DefaultRequestHeaders.ExpectContinue = false;
55-
return httpClient;
56-
});
57-
}
54+
Timeout = requestData.RequestTimeout
55+
};
56+
57+
httpClient.DefaultRequestHeaders.ExpectContinue = false;
58+
return httpClient;
59+
});
5860
}
5961

6062
return client;
6163
}
6264

6365
public virtual ElasticsearchResponse<TReturn> Request<TReturn>(RequestData requestData) where TReturn : class
6466
{
67+
//TODO remove Stream response support in 6.0, closing the stream is sufficient on desktop/mono
68+
//but not on .NET core on linux HttpClient which proxies to curl.
69+
if (typeof(TReturn) == typeof(Stream) && ConnectionConfiguration.IsCurlHandler)
70+
throw new Exception(CanNotUseStreamResponsesWithCurlHandler);
71+
6572
var client = this.GetClient(requestData);
6673
var builder = new ResponseBuilder<TReturn>(requestData);
74+
HttpResponseMessage responseMessage = null;
6775
try
6876
{
6977
var requestMessage = CreateHttpRequestMessage(requestData);
70-
var response = client.SendAsync(requestMessage).GetAwaiter().GetResult();
78+
responseMessage = client.SendAsync(requestMessage).GetAwaiter().GetResult();
7179
requestData.MadeItToResponse = true;
72-
builder.StatusCode = (int)response.StatusCode;
80+
builder.StatusCode = (int)responseMessage.StatusCode;
7381
IEnumerable<string> warnings;
74-
if (response.Headers.TryGetValues("Warning", out warnings))
82+
if (responseMessage.Headers.TryGetValues("Warning", out warnings))
7583
builder.DeprecationWarnings = warnings;
7684

77-
if (response.Content != null)
78-
builder.Stream = response.Content.ReadAsStreamAsync().GetAwaiter().GetResult();
85+
if (responseMessage.Content != null)
86+
builder.Stream = responseMessage.Content.ReadAsStreamAsync().GetAwaiter().GetResult();
87+
// https://github.com/elastic/elasticsearch-net/issues/2311
88+
// if stream is null call dispose on response instead.
89+
if (builder.Stream == null || builder.Stream == Stream.Null) responseMessage.Dispose();
7990
}
8091
catch (TaskCanceledException e)
8192
{
@@ -85,26 +96,38 @@ public virtual ElasticsearchResponse<TReturn> Request<TReturn>(RequestData reque
8596
{
8697
builder.Exception = e;
8798
}
88-
89-
return builder.ToResponse();
99+
var response = builder.ToResponse();
100+
//explicit dispose of response not needed (as documented on MSDN) on desktop CLR
101+
//but we can not guarantee this is true for all HttpMessageHandler implementations
102+
if (typeof(TReturn) != typeof(Stream)) responseMessage?.Dispose();
103+
return response;
90104
}
91105

106+
92107
public virtual async Task<ElasticsearchResponse<TReturn>> RequestAsync<TReturn>(RequestData requestData, CancellationToken cancellationToken) where TReturn : class
93108
{
109+
//TODO remove Stream response support in 6.0, closing the stream is sufficient on desktop/mono
110+
//but not on .NET core on linux HttpClient which proxies to curl.
111+
if (typeof(TReturn) == typeof(Stream) && ConnectionConfiguration.IsCurlHandler)
112+
throw new Exception(CanNotUseStreamResponsesWithCurlHandler);
113+
94114
var client = this.GetClient(requestData);
95115
var builder = new ResponseBuilder<TReturn>(requestData, cancellationToken);
116+
HttpResponseMessage responseMessage = null;
96117
try
97118
{
98119
var requestMessage = CreateHttpRequestMessage(requestData);
99-
var response = await client.SendAsync(requestMessage, cancellationToken).ConfigureAwait(false);
100-
requestData.MadeItToResponse = true;
101-
builder.StatusCode = (int)response.StatusCode;
120+
responseMessage = await client.SendAsync(requestMessage, cancellationToken).ConfigureAwait(false);
121+
builder.StatusCode = (int)responseMessage.StatusCode;
102122
IEnumerable<string> warnings;
103-
if (response.Headers.TryGetValues("Warning", out warnings))
123+
if (responseMessage.Headers.TryGetValues("Warning", out warnings))
104124
builder.DeprecationWarnings = warnings;
105125

106-
if (response.Content != null)
107-
builder.Stream = await response.Content.ReadAsStreamAsync().ConfigureAwait(false);
126+
if (responseMessage.Content != null)
127+
builder.Stream = await responseMessage.Content.ReadAsStreamAsync().ConfigureAwait(false);
128+
// https://github.com/elastic/elasticsearch-net/issues/2311
129+
// if stream is null call dispose on response instead.
130+
if (builder.Stream == null || builder.Stream == Stream.Null) responseMessage.Dispose();
108131
}
109132
catch (TaskCanceledException e)
110133
{
@@ -114,8 +137,11 @@ public virtual async Task<ElasticsearchResponse<TReturn>> RequestAsync<TReturn>(
114137
{
115138
builder.Exception = e;
116139
}
117-
118-
return await builder.ToResponseAsync().ConfigureAwait(false);
140+
var response = await builder.ToResponseAsync().ConfigureAwait(false);
141+
//explicit dispose of response not needed (as documented on MSDN) on desktop CLR
142+
//but we can not guarantee this is true for all HttpMessageHandler implementations
143+
if (typeof(TReturn) != typeof(Stream)) responseMessage?.Dispose();
144+
return response;
119145
}
120146

121147
private static readonly string MissingConnectionLimitMethodError =
@@ -151,8 +177,11 @@ protected virtual HttpClientHandler CreateHttpClientHandler(RequestData requestD
151177
{
152178
var uri = new Uri(requestData.ProxyAddress);
153179
var proxy = new WebProxy(uri);
154-
var credentials = new NetworkCredential(requestData.ProxyUsername, requestData.ProxyPassword);
155-
proxy.Credentials = credentials;
180+
if (!string.IsNullOrEmpty(requestData.ProxyUsername))
181+
{
182+
var credentials = new NetworkCredential(requestData.ProxyUsername, requestData.ProxyPassword);
183+
proxy.Credentials = credentials;
184+
}
156185
handler.Proxy = proxy;
157186
}
158187

@@ -189,6 +218,10 @@ protected virtual HttpRequestMessage CreateRequestMessage(RequestData requestDat
189218
{
190219
requestMessage.Headers.TryAddWithoutValidation(key, requestData.Headers.GetValues(key));
191220
}
221+
requestMessage.Headers.Connection.Clear();
222+
requestMessage.Headers.ConnectionClose = false;
223+
requestMessage.Headers.Connection.Add("Keep-Alive");
224+
//requestMessage.Headers.Connection;
192225

193226
requestMessage.Headers.Accept.Add(new MediaTypeWithQualityHeaderValue(requestData.Accept));
194227

src/Elasticsearch.Net/Connection/HttpConnection.cs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -268,9 +268,7 @@ private static void HandleException<TReturn>(ResponseBuilder<TReturn> builder, W
268268

269269
void IDisposable.Dispose() => this.DisposeManagedResources();
270270

271-
protected virtual void DisposeManagedResources()
272-
{
273-
}
271+
protected virtual void DisposeManagedResources() { }
274272
}
275273
}
276274
#endif

src/Elasticsearch.Net/CrossPlatform/TypeExtensions.cs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,15 @@ internal static bool AssignableFrom(this Type type, Type from)
2525
return type.IsAssignableFrom(from);
2626
#endif
2727
}
28+
29+
internal static Assembly Assembly(this Type type)
30+
{
31+
#if DOTNETCORE
32+
return type.GetTypeInfo().Assembly;
33+
#else
34+
return type.Assembly;
35+
#endif
36+
}
2837

2938
internal static bool IsValue(this Type type)
3039
{

0 commit comments

Comments
 (0)