From 309a14b6ecce12cfcfb66e88028ccd64af52377c Mon Sep 17 00:00:00 2001 From: Jeff Fisher Date: Mon, 4 Jan 2021 12:38:47 -0800 Subject: [PATCH] [core-http(s)] Reuse DefaultHttpClient between ServiceClient instances (#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 --- sdk/core/core-client/src/httpClientCache.ts | 14 ++++++++++++++ sdk/core/core-client/src/serviceClient.ts | 4 ++-- sdk/core/core-client/test/serviceClient.spec.ts | 6 ++++++ sdk/core/core-http/CHANGELOG.md | 4 ++++ sdk/core/core-http/src/httpClientCache.ts | 15 +++++++++++++++ sdk/core/core-http/src/serviceClient.ts | 4 ++-- sdk/core/core-http/test/serviceClientTests.ts | 6 ++++++ 7 files changed, 49 insertions(+), 4 deletions(-) create mode 100644 sdk/core/core-client/src/httpClientCache.ts create mode 100644 sdk/core/core-http/src/httpClientCache.ts diff --git a/sdk/core/core-client/src/httpClientCache.ts b/sdk/core/core-client/src/httpClientCache.ts new file mode 100644 index 000000000000..1b97a2e8eb36 --- /dev/null +++ b/sdk/core/core-client/src/httpClientCache.ts @@ -0,0 +1,14 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +import { HttpsClient, DefaultHttpsClient } from "@azure/core-https"; + +let cachedHttpsClient: HttpsClient | undefined; + +export function getCachedDefaultHttpsClient(): HttpsClient { + if (!cachedHttpsClient) { + cachedHttpsClient = new DefaultHttpsClient(); + } + + return cachedHttpsClient; +} diff --git a/sdk/core/core-client/src/serviceClient.ts b/sdk/core/core-client/src/serviceClient.ts index 78425b9643fa..8b228e133276 100644 --- a/sdk/core/core-client/src/serviceClient.ts +++ b/sdk/core/core-client/src/serviceClient.ts @@ -3,7 +3,6 @@ import { TokenCredential } from "@azure/core-auth"; import { - DefaultHttpsClient, HttpsClient, PipelineRequest, PipelineResponse, @@ -29,6 +28,7 @@ import { isPrimitiveType } from "./utils"; import { deserializationPolicy, DeserializationPolicyOptions } from "./deserializationPolicy"; import { URL } from "./url"; import { serializationPolicy, serializationPolicyOptions } from "./serializationPolicy"; +import { getCachedDefaultHttpsClient } from "./httpClientCache"; /** * Options to be provided while creating the client. @@ -104,7 +104,7 @@ export class ServiceClient { constructor(options: ServiceClientOptions = {}) { this._requestContentType = options.requestContentType; this._baseUri = options.baseUri; - this._httpsClient = options.httpsClient || new DefaultHttpsClient(); + this._httpsClient = options.httpsClient || getCachedDefaultHttpsClient(); const credentialScopes = getCredentialScopes(options); this._pipeline = options.pipeline || diff --git a/sdk/core/core-client/test/serviceClient.spec.ts b/sdk/core/core-client/test/serviceClient.spec.ts index 8f54d666bee2..3fe1fd6f87a9 100644 --- a/sdk/core/core-client/test/serviceClient.spec.ts +++ b/sdk/core/core-client/test/serviceClient.spec.ts @@ -26,6 +26,7 @@ import { import { getOperationArgumentValueFromParameter } from "../src/operationHelpers"; import { deserializationPolicy } from "../src/deserializationPolicy"; import { TokenCredential } from "@azure/core-auth"; +import { getCachedDefaultHttpsClient } from "../src/httpClientCache"; describe("ServiceClient", function() { describe("Auth scopes", () => { @@ -801,6 +802,11 @@ describe("ServiceClient", function() { assert.strictEqual(ex.details.message, "InvalidResourceNameBody"); } }); + + it("should re-use the common instance of DefaultHttpClient", function() { + const client = new ServiceClient(); + assert.strictEqual((client as any)._httpsClient, getCachedDefaultHttpsClient()); + }); }); async function testSendOperationRequest( diff --git a/sdk/core/core-http/CHANGELOG.md b/sdk/core/core-http/CHANGELOG.md index 4ee9082fe45a..f9ed4036d918 100644 --- a/sdk/core/core-http/CHANGELOG.md +++ b/sdk/core/core-http/CHANGELOG.md @@ -1,5 +1,9 @@ # Release History +## 1.2.2 (Unreleased) + +- 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) + ## 1.2.1 (2020-12-09) - 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) diff --git a/sdk/core/core-http/src/httpClientCache.ts b/sdk/core/core-http/src/httpClientCache.ts new file mode 100644 index 000000000000..568ca65fbc54 --- /dev/null +++ b/sdk/core/core-http/src/httpClientCache.ts @@ -0,0 +1,15 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +import { HttpClient } from "./httpClient"; +import { DefaultHttpClient } from "./defaultHttpClient"; + +let cachedHttpClient: HttpClient | undefined; + +export function getCachedDefaultHttpClient(): HttpClient { + if (!cachedHttpClient) { + cachedHttpClient = new DefaultHttpClient(); + } + + return cachedHttpClient; +} diff --git a/sdk/core/core-http/src/serviceClient.ts b/sdk/core/core-http/src/serviceClient.ts index 4c003db73264..0d39af77898a 100644 --- a/sdk/core/core-http/src/serviceClient.ts +++ b/sdk/core/core-http/src/serviceClient.ts @@ -2,7 +2,6 @@ // Licensed under the MIT license. import { TokenCredential, isTokenCredential } from "@azure/core-auth"; -import { DefaultHttpClient } from "./defaultHttpClient"; import { HttpClient } from "./httpClient"; import { HttpOperationResponse, RestResponse } from "./httpOperationResponse"; import { HttpPipelineLogger } from "./httpPipelineLogger"; @@ -62,6 +61,7 @@ import { disableResponseDecompressionPolicy } from "./policies/disableResponseDe import { ndJsonPolicy } from "./policies/ndJsonPolicy"; import { XML_ATTRKEY, SerializerOptions, XML_CHARKEY } from "./util/serializer.common"; import { URL } from "./url"; +import { getCachedDefaultHttpClient } from "./httpClientCache"; /** * Options to configure a proxy for outgoing requests (Node.js only). @@ -197,7 +197,7 @@ export class ServiceClient { } this._withCredentials = options.withCredentials || false; - this._httpClient = options.httpClient || new DefaultHttpClient(); + this._httpClient = options.httpClient || getCachedDefaultHttpClient(); this._requestPolicyOptions = new RequestPolicyOptions(options.httpPipelineLogger); let requestPolicyFactories: RequestPolicyFactory[]; diff --git a/sdk/core/core-http/test/serviceClientTests.ts b/sdk/core/core-http/test/serviceClientTests.ts index 7b815fc06b49..1ce8529128f5 100644 --- a/sdk/core/core-http/test/serviceClientTests.ts +++ b/sdk/core/core-http/test/serviceClientTests.ts @@ -30,6 +30,7 @@ import { } from "../src/coreHttp"; import { ParameterPath } from "../src/operationParameter"; import * as Mappers from "./testMappers"; +import { getCachedDefaultHttpClient } from "../src/httpClientCache"; describe("ServiceClient", function() { describe("Auth scopes", () => { @@ -1993,6 +1994,11 @@ describe("ServiceClient", function() { assert.strictEqual(ex.details.message, "InvalidResourceNameBody"); } }); + + it("should re-use the common instance of DefaultHttpClient", function() { + const client = new ServiceClient(); + assert.strictEqual((client as any)._httpClient, getCachedDefaultHttpClient()); + }); }); function stringToByteArray(str: string): Uint8Array {