From 66f6923e479b2c262d6d8868e234fc1b326f58f6 Mon Sep 17 00:00:00 2001 From: Przemyslaw Motacki Date: Thu, 21 Sep 2023 15:48:50 +0200 Subject: [PATCH] SNOW-734920: Add possibility to configure keep alive value --- lib/global_config.js | 2 +- lib/http/base.js | 90 +++++++++++------------------- lib/http/node.js | 58 +++++++++---------- test/integration/testKeepAlive.js | 17 +++--- test/unit/snowflake_config_test.js | 22 ++++++++ 5 files changed, 94 insertions(+), 95 deletions(-) diff --git a/lib/global_config.js b/lib/global_config.js index d9a4d4e87..cb54b8ba7 100644 --- a/lib/global_config.js +++ b/lib/global_config.js @@ -276,7 +276,7 @@ exports.setKeepAlive = function (value) * * @param {boolean} value */ -exports.keepAlive = function (config) { +exports.getKeepAlive = function () { return keepAlive; }; diff --git a/lib/http/base.js b/lib/http/base.js index 8ff5036a7..1d1890034 100644 --- a/lib/http/base.js +++ b/lib/http/base.js @@ -16,14 +16,12 @@ const DEFAULT_REQUEST_TIMEOUT = 360000; * @param connectionConfig * @constructor */ -function HttpClient(connectionConfig) -{ +function HttpClient(connectionConfig) { // save the connection config this._connectionConfig = connectionConfig; } -HttpClient.prototype.getConnectionConfig = function () -{ +HttpClient.prototype.getConnectionConfig = function () { return this._connectionConfig; }; @@ -34,8 +32,7 @@ HttpClient.prototype.getConnectionConfig = function () * * @returns {Object} an object representing the request that was issued. */ -HttpClient.prototype.request = function (options) -{ +HttpClient.prototype.request = function (options) { // validate input Errors.assertInternal(Util.isObject(options)); Errors.assertInternal(Util.isString(options.method)); @@ -47,17 +44,16 @@ HttpClient.prototype.request = function (options) Errors.assertInternal(!Util.exists(options.callback) || Util.isFunction(options.callback)); - var headers; - var json; - var body; - var request; + let headers; + let json; + let body; + let request; // normalize the headers headers = normalizeHeaders(options.headers); // create a function to send the request - var sendRequest = async function sendRequest() - { + let sendRequest = async function sendRequest() { const url = options.url; const timeout = options.timeout || @@ -67,7 +63,7 @@ HttpClient.prototype.request = function (options) Logger.getInstance().trace(`CALL ${options.method} with timeout ${timeout}: ${url}`); // build the basic request options - var requestOptions = + const requestOptions = { method: options.method, url: url, @@ -90,7 +86,7 @@ HttpClient.prototype.request = function (options) if (this._connectionConfig.agentClass) { mock = { agentClass: this._connectionConfig.agentClass - } + }; } // add the agent and proxy options @@ -142,33 +138,25 @@ HttpClient.prototype.request = function (options) // if a request body is specified and compression is enabled, // try to compress the request body before sending the request json = body = options.json; - if (body) - { - var bufferUncompressed = Buffer.from(JSON.stringify(body), 'utf8'); - zlib.gzip(bufferUncompressed, null, function (err, bufferCompressed) - { + if (body) { + const bufferUncompressed = Buffer.from(JSON.stringify(body), 'utf8'); + zlib.gzip(bufferUncompressed, null, function (err, bufferCompressed) { // if the compression was successful - if (!err) - { + if (!err) { json = undefined; // don't specify the 'json' option // use the compressed buffer as the body and // set the appropriate content encoding body = bufferCompressed; headers['Content-Encoding'] = 'gzip'; - } - else - { + } else { Logger.getInstance().warn('Could not compress request body.'); } sendRequest(); }); - } - else - { - if (body) - { + } else { + if (body) { Logger.getInstance().trace('Request body compression disabled.'); } @@ -178,10 +166,8 @@ HttpClient.prototype.request = function (options) // return an externalized request object that only contains // methods we're comfortable exposing to the outside world return { - abort: function () - { - if (request) - { + abort: function () { + if (request) { request.abort(); } } @@ -195,8 +181,7 @@ HttpClient.prototype.request = function (options) * * @returns {*} */ -HttpClient.prototype.getRequestModule = function () -{ +HttpClient.prototype.getRequestModule = function () { return null; }; @@ -208,8 +193,7 @@ HttpClient.prototype.getRequestModule = function () * * @returns {*} */ -HttpClient.prototype.getAgent = function (url, proxy, mock) -{ +HttpClient.prototype.getAgent = function (url, proxy, mock) { return null; }; @@ -223,12 +207,10 @@ module.exports = HttpClient; * * @returns {Object} */ -function normalizeHeaders(headers) -{ - var ret = headers; +function normalizeHeaders(headers) { + let ret = headers; - if (Util.isObject(headers)) - { + if (Util.isObject(headers)) { ret = { 'user-agent': Util.userAgent }; @@ -242,19 +224,14 @@ function normalizeHeaders(headers) // browser-request will inject its own 'accept': 'application/json' header // and the browser XMLHttpRequest object will concatenate the two values and // send 'Accept': 'application/json, application/json' with the request - var headerNameLowerCase; - for (var headerName in headers) - { - if (headers.hasOwnProperty(headerName)) - { + let headerNameLowerCase; + for (const headerName in headers) { + if (headers.hasOwnProperty(headerName)) { headerNameLowerCase = headerName.toLowerCase(); if ((headerNameLowerCase === 'accept') || - (headerNameLowerCase === 'content-type')) - { + (headerNameLowerCase === 'content-type')) { ret[headerNameLowerCase] = headers[headerName]; - } - else - { + } else { ret[headerName] = headers[headerName]; } } @@ -273,13 +250,10 @@ function normalizeHeaders(headers) * * @return {Object} */ -function normalizeResponse(response) -{ +function normalizeResponse(response) { // if the response doesn't already have a getResponseHeader() method, add one - if (response && !response.getResponseHeader) - { - response.getResponseHeader = function (header) - { + if (response && !response.getResponseHeader) { + response.getResponseHeader = function (header) { return response.headers && response.headers[ Util.isString(header) ? header.toLowerCase() : header]; }; diff --git a/lib/http/node.js b/lib/http/node.js index d8d8ba8a6..c9a821213 100644 --- a/lib/http/node.js +++ b/lib/http/node.js @@ -22,12 +22,12 @@ const Logger = require('../logger'); * @return {Number} number of milliseconds to wait before retrying again the request. */ NodeHttpClient.prototype.constructExponentialBackoffStrategy = function () { - var sleep = this._connectionConfig.getRetrySfStartingSleepTime(); - var cap = this._connectionConfig.getRetrySfMaxSleepTime(); - var base = 1; + let sleep = this._connectionConfig.getRetrySfStartingSleepTime(); + const cap = this._connectionConfig.getRetrySfMaxSleepTime(); + const base = 1; sleep = Util.nextSleepTime(base, cap, sleep); return sleep * 1000; -} +}; /** * Creates a client that can be used to make requests in Node.js. @@ -35,8 +35,7 @@ NodeHttpClient.prototype.constructExponentialBackoffStrategy = function () { * @param {ConnectionConfig} connectionConfig * @constructor */ -function NodeHttpClient(connectionConfig) -{ +function NodeHttpClient(connectionConfig) { Base.apply(this, arguments); } @@ -44,16 +43,15 @@ Util.inherits(NodeHttpClient, Base); const httpsAgentCache = new Map(); -function getFromCacheOrCreate(agentClass, options, url) { - const parsed = Url.parse(url); - const protocol = parsed.protocol || 'http:'; - const port = parsed.port || (protocol === 'http:' ? '80' : '443'); - const agentId = `${protocol}//${parsed.hostname}:${port}/${options.keepAlive}` +function getFromCacheOrCreate(agentClass, options, parsedUrl) { + const protocol = parsedUrl.protocol || 'http:'; + const port = parsedUrl.port || (protocol === 'http:' ? '80' : '443'); + const agentId = `${protocol}//${parsedUrl.hostname}:${port}/${options.keepAlive}`; if (httpsAgentCache.has(agentId)) { Logger.getInstance().trace(`Get agent with id: ${agentId} from cache`); return httpsAgentCache.get(agentId); } else { - agent = agentClass(options) + const agent = agentClass(options); httpsAgentCache.set(agentId, agent); Logger.getInstance().trace(`Create and add to cache new agent ${agentId}`); return agent; @@ -61,7 +59,6 @@ function getFromCacheOrCreate(agentClass, options, url) { } function prepareProxyAgentOptions(agentOptions, proxy) { - Logger.getInstance().info(`Use proxy: ${JSON.stringify(proxy)}`); agentOptions.host = proxy.host; agentOptions.port = proxy.port; agentOptions.protocol = proxy.protocol; @@ -73,13 +70,13 @@ function prepareProxyAgentOptions(agentOptions, proxy) { function isBypassProxy(proxy, url, bypassProxy) { if (proxy && proxy.noProxy) { - let bypassList = proxy.noProxy.split("|"); + const bypassList = proxy.noProxy.split('|'); for (let i = 0; i < bypassList.length; i++) { host = bypassList[i].trim(); - host = host.replace("*", ".*?"); - let matches = url.match(host); + host = host.replace('*', '.*?'); + const matches = url.match(host); if (matches) { - Logger.getInstance().debug("bypassing proxy for %s", url); + Logger.getInstance().debug('bypassing proxy for %s', url); return true; } } @@ -90,28 +87,31 @@ function isBypassProxy(proxy, url, bypassProxy) { /** * @inheritDoc */ -NodeHttpClient.prototype.getAgent = function (url, proxy, mock, defaultAgentOptions) { - const isHttps = Url.parse(url).protocol === 'https:'; - let agentOptions = {keepAlive: GlobalConfig.keepAlive()}; - var bypassProxy = isBypassProxy(proxy, url); - +NodeHttpClient.prototype.getAgent = function (url, proxy, mock) { + const agentOptions = {keepAlive: GlobalConfig.getKeepAlive()}; if (mock) { - return mock.agentClass(agentOptions) + return mock.agentClass(agentOptions); } + const parsedUrl = Url.parse(url); + const isHttps = parsedUrl.protocol === 'https:'; + const bypassProxy = isBypassProxy(proxy, url); + let agent = {}; + if (isHttps) { if (proxy && !bypassProxy) { - const proxyAgentOptions = prepareProxyAgentOptions(agentOptions, proxy) - return getFromCacheOrCreate(HttpsProxyAgent, proxyAgentOptions, url); + prepareProxyAgentOptions(agentOptions, proxy); + agent = getFromCacheOrCreate(HttpsProxyAgent, agentOptions, parsedUrl); } else { - return getFromCacheOrCreate(HttpsAgent, agentOptions, url); + agent = getFromCacheOrCreate(HttpsAgent, agentOptions, parsedUrl); } } else if (proxy && !bypassProxy) { - const proxyAgentOptions = prepareProxyAgentOptions(agentOptions, proxy); - return getFromCacheOrCreate(HttpAgent, proxyAgentOptions, url); + prepareProxyAgentOptions(agentOptions, proxy); + agent = getFromCacheOrCreate(HttpAgent, agentOptions, url); } else { - return getFromCacheOrCreate(HttpAgent, agentOptions, url); + agent = getFromCacheOrCreate(HttpAgent, agentOptions, parsedUrl); } + return agent; }; module.exports = NodeHttpClient; diff --git a/test/integration/testKeepAlive.js b/test/integration/testKeepAlive.js index de643ae79..476e550b9 100644 --- a/test/integration/testKeepAlive.js +++ b/test/integration/testKeepAlive.js @@ -8,23 +8,29 @@ const async = require('async'); const testUtil = require('./testUtil'); const { performance } = require('perf_hooks'); const assert = require("assert"); +const {logger} = require("./sharedLogger"); describe('keepAlive test', function () { let connection; - const loopCount = 5; + const loopCount = 10; const rowCount = 10; const tableName = 'test_keepalive000'; const createTableWithRandomStrings = `CREATE OR REPLACE TABLE ${tableName} (value string) AS select randstr(200, random()) from table (generator(rowcount =>${rowCount}))`; + before(async () => { + connection = snowflake.createConnection(connOption.valid); + await testUtil.connectAsync(connection); + await testUtil.executeCmdAsync(connection, createTableWithRandomStrings); + }); after(async () => { await testUtil.dropTablesIgnoringErrorsAsync(connection, [tableName]); await testUtil.destroyConnectionAsync(connection); }); - it('Run query in loop', function (done) + it('Verify that requests working faster with keep alive', function (done) { let sumWithKeepAlive = 0; let sumWithoutKeepAlive = 0; @@ -33,9 +39,6 @@ describe('keepAlive test', function () // Run the query loopCount times async function () { - connection = snowflake.createConnection(connOption.valid); - await testUtil.connectAsync(connection); - await testUtil.executeCmdAsync(connection, createTableWithRandomStrings); for (let count = 1; count <= loopCount; count++) { await new Promise((resolve, reject) => @@ -72,7 +75,6 @@ describe('keepAlive test', function () snowflake.configure({keepAlive: false}); connection = snowflake.createConnection(connOption.valid); await testUtil.connectAsync(connection); - await testUtil.executeCmdAsync(connection, createTableWithRandomStrings); for (let count = 1; count <= loopCount; count++) { await new Promise((resolve, reject) => @@ -106,7 +108,8 @@ describe('keepAlive test', function () }, async function () { - assert.ok(sumWithoutKeepAlive/2 > sumWithKeepAlive, 'With keep alive queries should work more thdn two times faster'); + logger.debug(`sumWithoutKeepAlive: ${sumWithoutKeepAlive}, sumWithKeepAlive: ${sumWithKeepAlive}`) + assert.ok(sumWithoutKeepAlive * 0.75 > sumWithKeepAlive, 'With keep alive the queries should work faster'); } ], done diff --git a/test/unit/snowflake_config_test.js b/test/unit/snowflake_config_test.js index 87e0e9943..b92819af9 100644 --- a/test/unit/snowflake_config_test.js +++ b/test/unit/snowflake_config_test.js @@ -18,6 +18,7 @@ describe('Snowflake Configure Tests', function () { logLevel: Logger.getInstance().getLevelTag(), insecureConnect: GlobalConfig.isInsecureConnect(), ocspFailOpen: GlobalConfig.getOcspFailOpen(), + keepAlive: GlobalConfig.getKeepAlive(), jsonColumnVariantParser: GlobalConfig.jsonColumnVariantParser, xmlColumnVariantParser: GlobalConfig.xmlColumnVariantParser }; @@ -55,6 +56,11 @@ describe('Snowflake Configure Tests', function () { options: { xmlColumnVariantParser: 'unsupported' }, errorCode: ErrorCodes.ERR_GLOBAL_CONFIGURE_INVALID_XML_PARSER }, + { + name: 'invalid keep alive', + options: { keepAlive: 'unsupported' }, + errorCode: ErrorCodes.ERR_GLOBAL_CONFIGURE_INVALID_KEEP_ALIVE + } ]; negativeTestCases.forEach(testCase => { @@ -139,6 +145,20 @@ describe('Snowflake Configure Tests', function () { ocspFailOpen: true } }, + { + name: 'keepAlive false', + options: + { + keepAlive: false + } + }, + { + name: 'keepAlive true', + options: + { + keepAlive: true + } + }, { name: 'json parser', options: @@ -167,6 +187,8 @@ describe('Snowflake Configure Tests', function () { val = GlobalConfig.isInsecureConnect(); } else if (key == 'ocspFailOpen') { val = GlobalConfig.getOcspFailOpen(); + } else if (key == 'keepAlive') { + val = GlobalConfig.getKeepAlive(); } else { val = GlobalConfig[key]; }