Skip to content

Commit 309a14b

Browse files
authored
[core-http(s)] Reuse DefaultHttpClient between ServiceClient instances (Azure#12966)
Today `ServiceClient` ends up creating a new instance of `DefaultHttpClient` each time one is not passed in via `ServiceClientOptions`. Since it is unusual for a custom `HttpClient` to be passed in, this means we recreate the `HttpClient` each time a someone creates a convenience client that wraps a generated client. Normally, this isn't a big deal since clients are heavy enough to be cached/re-used by customer code. However, a KeyVault customer ran into a scenario where they had to recreate `CryptographyClient` many times (i.e. 5 times per second) and they noticed some pretty bad performance characteristics (out of memory, out of sockets, etc.) While there is still more we can do with improving `CryptographyClient` to perhaps not need to be constructed so often (or making it cache its own copy of `KeyVaultClient` internally), this PR aims to solve the biggest issue with the above scenario: many copies of `DefaultHttpClient` each allocating NodeJS `http.Agent`s that then are tied to real OS resources that take a while to cleanup. Another related note is that the Storage packages already do this today in their custom Pipeline implementation: https://github.com/Azure/azure-sdk-for-js/blob/master/sdk/storage/storage-blob/src/utils/cache.ts
1 parent b83b905 commit 309a14b

File tree

7 files changed

+49
-4
lines changed

7 files changed

+49
-4
lines changed
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// Copyright (c) Microsoft Corporation.
2+
// Licensed under the MIT license.
3+
4+
import { HttpsClient, DefaultHttpsClient } from "@azure/core-https";
5+
6+
let cachedHttpsClient: HttpsClient | undefined;
7+
8+
export function getCachedDefaultHttpsClient(): HttpsClient {
9+
if (!cachedHttpsClient) {
10+
cachedHttpsClient = new DefaultHttpsClient();
11+
}
12+
13+
return cachedHttpsClient;
14+
}

sdk/core/core-client/src/serviceClient.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33

44
import { TokenCredential } from "@azure/core-auth";
55
import {
6-
DefaultHttpsClient,
76
HttpsClient,
87
PipelineRequest,
98
PipelineResponse,
@@ -29,6 +28,7 @@ import { isPrimitiveType } from "./utils";
2928
import { deserializationPolicy, DeserializationPolicyOptions } from "./deserializationPolicy";
3029
import { URL } from "./url";
3130
import { serializationPolicy, serializationPolicyOptions } from "./serializationPolicy";
31+
import { getCachedDefaultHttpsClient } from "./httpClientCache";
3232

3333
/**
3434
* Options to be provided while creating the client.
@@ -104,7 +104,7 @@ export class ServiceClient {
104104
constructor(options: ServiceClientOptions = {}) {
105105
this._requestContentType = options.requestContentType;
106106
this._baseUri = options.baseUri;
107-
this._httpsClient = options.httpsClient || new DefaultHttpsClient();
107+
this._httpsClient = options.httpsClient || getCachedDefaultHttpsClient();
108108
const credentialScopes = getCredentialScopes(options);
109109
this._pipeline =
110110
options.pipeline ||

sdk/core/core-client/test/serviceClient.spec.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import {
2626
import { getOperationArgumentValueFromParameter } from "../src/operationHelpers";
2727
import { deserializationPolicy } from "../src/deserializationPolicy";
2828
import { TokenCredential } from "@azure/core-auth";
29+
import { getCachedDefaultHttpsClient } from "../src/httpClientCache";
2930

3031
describe("ServiceClient", function() {
3132
describe("Auth scopes", () => {
@@ -801,6 +802,11 @@ describe("ServiceClient", function() {
801802
assert.strictEqual(ex.details.message, "InvalidResourceNameBody");
802803
}
803804
});
805+
806+
it("should re-use the common instance of DefaultHttpClient", function() {
807+
const client = new ServiceClient();
808+
assert.strictEqual((client as any)._httpsClient, getCachedDefaultHttpsClient());
809+
});
804810
});
805811

806812
async function testSendOperationRequest(

sdk/core/core-http/CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
# Release History
22

3+
## 1.2.2 (Unreleased)
4+
5+
- Cache the default `HttpClient` used when one isn't passed in to `ServiceClient`. This means that for most packages we will only create a single `HttpClient`, which will improve platform resource usage by reducing overhead. [PR 12966](https://github.com/Azure/azure-sdk-for-js/pull/12966)
6+
37
## 1.2.1 (2020-12-09)
48

59
- Support custom auth scopes. Scope is a mechanism in OAuth 2.0 to limit an application's access to a user's account [PR 12752](https://github.com/Azure/azure-sdk-for-js/pull/12752)
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
// Copyright (c) Microsoft Corporation.
2+
// Licensed under the MIT license.
3+
4+
import { HttpClient } from "./httpClient";
5+
import { DefaultHttpClient } from "./defaultHttpClient";
6+
7+
let cachedHttpClient: HttpClient | undefined;
8+
9+
export function getCachedDefaultHttpClient(): HttpClient {
10+
if (!cachedHttpClient) {
11+
cachedHttpClient = new DefaultHttpClient();
12+
}
13+
14+
return cachedHttpClient;
15+
}

sdk/core/core-http/src/serviceClient.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
// Licensed under the MIT license.
33

44
import { TokenCredential, isTokenCredential } from "@azure/core-auth";
5-
import { DefaultHttpClient } from "./defaultHttpClient";
65
import { HttpClient } from "./httpClient";
76
import { HttpOperationResponse, RestResponse } from "./httpOperationResponse";
87
import { HttpPipelineLogger } from "./httpPipelineLogger";
@@ -62,6 +61,7 @@ import { disableResponseDecompressionPolicy } from "./policies/disableResponseDe
6261
import { ndJsonPolicy } from "./policies/ndJsonPolicy";
6362
import { XML_ATTRKEY, SerializerOptions, XML_CHARKEY } from "./util/serializer.common";
6463
import { URL } from "./url";
64+
import { getCachedDefaultHttpClient } from "./httpClientCache";
6565

6666
/**
6767
* Options to configure a proxy for outgoing requests (Node.js only).
@@ -197,7 +197,7 @@ export class ServiceClient {
197197
}
198198

199199
this._withCredentials = options.withCredentials || false;
200-
this._httpClient = options.httpClient || new DefaultHttpClient();
200+
this._httpClient = options.httpClient || getCachedDefaultHttpClient();
201201
this._requestPolicyOptions = new RequestPolicyOptions(options.httpPipelineLogger);
202202

203203
let requestPolicyFactories: RequestPolicyFactory[];

sdk/core/core-http/test/serviceClientTests.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import {
3030
} from "../src/coreHttp";
3131
import { ParameterPath } from "../src/operationParameter";
3232
import * as Mappers from "./testMappers";
33+
import { getCachedDefaultHttpClient } from "../src/httpClientCache";
3334

3435
describe("ServiceClient", function() {
3536
describe("Auth scopes", () => {
@@ -1993,6 +1994,11 @@ describe("ServiceClient", function() {
19931994
assert.strictEqual(ex.details.message, "InvalidResourceNameBody");
19941995
}
19951996
});
1997+
1998+
it("should re-use the common instance of DefaultHttpClient", function() {
1999+
const client = new ServiceClient();
2000+
assert.strictEqual((client as any)._httpClient, getCachedDefaultHttpClient());
2001+
});
19962002
});
19972003

19982004
function stringToByteArray(str: string): Uint8Array {

0 commit comments

Comments
 (0)