From c489d5831e44035504a99f504e95844a99569a91 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 | 5 +- lib/http/base.js | 6 +- lib/http/node.js | 77 +++++++----- .../ocsp_mock/testConnectionOcspMock.js | 32 ++--- test/integration/testKeepAlive.js | 115 ------------------ test/integration/testLargeResultSet.js | 4 +- test/integration/testManualConnection.js | 66 ++++++++++ test/unit/snowflake_config_test.js | 22 ++++ 8 files changed, 151 insertions(+), 176 deletions(-) delete mode 100644 test/integration/testKeepAlive.js diff --git a/lib/global_config.js b/lib/global_config.js index d9a4d4e87..11f0b43c9 100644 --- a/lib/global_config.js +++ b/lib/global_config.js @@ -264,8 +264,7 @@ let keepAlive = true; * * @param {boolean} value */ -exports.setKeepAlive = function (value) -{ +exports.setKeepAlive = function (value) { // validate input Errors.assertInternal(Util.isBoolean(value)); keepAlive = value; @@ -276,7 +275,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 b5d154409..e4c0ebf5d 100644 --- a/lib/http/base.js +++ b/lib/http/base.js @@ -83,7 +83,8 @@ HttpClient.prototype.request = function (options) }; let mock; - if (this._connectionConfig.agentClass) { + if (this._connectionConfig.agentClass) + { mock = { agentClass: this._connectionConfig.agentClass } @@ -187,8 +188,7 @@ HttpClient.prototype.getRequestModule = function () * * @returns {*} */ -HttpClient.prototype.getAgent = function (url, proxy, mock) -{ +HttpClient.prototype.getAgent = function (url, proxy, mock) { return null; }; diff --git a/lib/http/node.js b/lib/http/node.js index d8d8ba8a6..7142e13c2 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,24 +43,33 @@ 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}` - if (httpsAgentCache.has(agentId)) { - Logger.getInstance().trace(`Get agent with id: ${agentId} from cache`); - return httpsAgentCache.get(agentId); - } else { - agent = agentClass(options) +function getFromCacheOrCreate(agentClass, options, parsedUrl) { + const protocol = parsedUrl.protocol; + const port = parsedUrl.port || (protocol === 'http:' ? '80' : '443'); + const agentId = `${protocol}//${parsedUrl.hostname}:${port}`; + + let agent ={} + function createAgent(agentClass, agentOptions) { + const agent = agentClass(agentOptions); httpsAgentCache.set(agentId, agent); Logger.getInstance().trace(`Create and add to cache new agent ${agentId}`); return agent; } + + if (httpsAgentCache.has(agentId)) { + Logger.getInstance().trace(`Get agent with id: ${agentId} from cache`); + agent = httpsAgentCache.get(agentId); + // recreate agent if keepAlive parameter was changed + if (agent.keepAlive !== options.keepAlive) { + agent = createAgent(agentClass, options); + } + } else { + agent = createAgent(agentClass, options); + } + return agent; } 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 +81,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 +98,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, parsedUrl); } else { - return getFromCacheOrCreate(HttpAgent, agentOptions, url); + agent = getFromCacheOrCreate(HttpAgent, agentOptions, parsedUrl); } + return agent; }; module.exports = NodeHttpClient; diff --git a/test/integration/ocsp_mock/testConnectionOcspMock.js b/test/integration/ocsp_mock/testConnectionOcspMock.js index ac88c771e..d9b78aef2 100644 --- a/test/integration/ocsp_mock/testConnectionOcspMock.js +++ b/test/integration/ocsp_mock/testConnectionOcspMock.js @@ -9,8 +9,6 @@ const connOption = require('../connectionOptions'); const Errors = require('../../../lib/errors'); const ErrorCodes = Errors.codes; const HttpsMockAgent = require('./https_ocsp_mock_agent'); -const {configureLogger} = require("../../configureLogger"); -const testUtil = require("../testUtil"); function cloneConnOption(connOption) { @@ -26,11 +24,6 @@ describe('Connection test with OCSP Mock', function () { const valid = cloneConnOption(connOption.valid); const isHttps = valid.accessUrl.startsWith("https"); - before(async () => - { - configureLogger('TRACE'); - }); - function connect(errcode, connection, callback) { @@ -83,20 +76,18 @@ describe('Connection test with OCSP Mock', function () ); }); - it('Connection failure with OCSP unknown error', function (done) { + it('Connection failure with OCSP unknown error', function (done) + { valid.agentClass = HttpsMockAgent.HttpsMockAgentOcspUnkwown; const connection = snowflake.createConnection(valid); async.series([ - function (callback) { - try { - connect(ErrorCodes.ERR_OCSP_UNKNOWN, connection, callback); - } catch (err){ - console.log(`err ${err}`); - } - + function (callback) + { + connect(ErrorCodes.ERR_OCSP_UNKNOWN, connection, callback); }, - function (callback) { + function (callback) + { destroy(connection, callback); } ], @@ -104,15 +95,18 @@ describe('Connection test with OCSP Mock', function () ); }); - it('Connection failure with invalid validity OCSP error', function (done) { + it('Connection failure with invalid validity OCSP error', function (done) + { valid.agentClass = HttpsMockAgent.HttpsMockAgentOcspInvalid; const connection = snowflake.createConnection(valid); async.series([ - function (callback) { + function (callback) + { connect(ErrorCodes.ERR_OCSP_INVALID_VALIDITY, connection, callback); }, - function (callback) { + function (callback) + { destroy(connection, callback); } ], diff --git a/test/integration/testKeepAlive.js b/test/integration/testKeepAlive.js deleted file mode 100644 index de643ae79..000000000 --- a/test/integration/testKeepAlive.js +++ /dev/null @@ -1,115 +0,0 @@ -/* - * Copyright (c) 2015-2021 Snowflake Computing Inc. All rights reserved. - */ - -const snowflake = require('./../../lib/snowflake'); -const connOption = require('./connectionOptions'); -const async = require('async'); -const testUtil = require('./testUtil'); -const { performance } = require('perf_hooks'); -const assert = require("assert"); - -describe('keepAlive test', function () -{ - let connection; - const loopCount = 5; - 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}))`; - - after(async () => { - await testUtil.dropTablesIgnoringErrorsAsync(connection, [tableName]); - await testUtil.destroyConnectionAsync(connection); - }); - - it('Run query in loop', function (done) - { - let sumWithKeepAlive = 0; - let sumWithoutKeepAlive = 0; - async.series( - [ - // 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) => - { - const start = performance.now(); - connection.execute({ - sqlText: `SELECT VALUE from ${tableName} limit ${rowCount};`, - streamResult: true, - complete: function (err, stmt) { - if (err) { - done(err); - } else { - stmt.streamRows() - .on('error', function (err) { - done(err); - }) - .on('data', function (row) { - return; - }) - .on('end', function (row) { - const end = performance.now(); - const time = end - start; - sumWithKeepAlive += time; - resolve(); - }); - } - } - }); - }); - } - }, - async 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) => - { - const start = performance.now(); - connection.execute({ - sqlText: `SELECT VALUE from ${tableName} limit ${rowCount};`, - streamResult: true, - complete: function (err, stmt) { - if (err) { - done(err); - } else { - stmt.streamRows() - .on('error', function (err) { - done(err); - }) - .on('data', function (row) { - return; - }) - .on('end', function (row) { - const end = performance.now(); - const time = end - start; - sumWithoutKeepAlive += time; - resolve(); - }); - } - } - }); - }); - } - }, - async function () - { - assert.ok(sumWithoutKeepAlive/2 > sumWithKeepAlive, 'With keep alive queries should work more thdn two times faster'); - } - ], - done - ); - }); -}); diff --git a/test/integration/testLargeResultSet.js b/test/integration/testLargeResultSet.js index 973a1163e..79d6194c9 100644 --- a/test/integration/testLargeResultSet.js +++ b/test/integration/testLargeResultSet.js @@ -252,7 +252,7 @@ describe('Large result Set Tests', function () }); }); -describe('SNOW-743920: Large result set with ~35 chunks', function () { +describe('SNOW-743920:Large result set with ~35 chunks', function () { let connection; const tableName = 'test_table'; const sourceRowCount = 251002; @@ -263,7 +263,6 @@ describe('SNOW-743920: Large result set with ~35 chunks', function () { before(async () => { connection = testUtil.createConnection(); - configureLogger('TRACE'); await testUtil.connectAsync(connection); // setting ROWS_PER_RESULTSET causes invalid, not encoded chunks from GCP // await testUtil.executeCmdAsync(connection, 'alter session set ROWS_PER_RESULTSET = 1000000'); @@ -273,7 +272,6 @@ describe('SNOW-743920: Large result set with ~35 chunks', function () { }); after(async () => { - configureLogger('ERROR'); await testUtil.dropTablesIgnoringErrorsAsync(connection, [tableName]); await testUtil.destroyConnectionAsync(connection); }); diff --git a/test/integration/testManualConnection.js b/test/integration/testManualConnection.js index ce05e7ab1..806db1bbb 100644 --- a/test/integration/testManualConnection.js +++ b/test/integration/testManualConnection.js @@ -259,4 +259,70 @@ if (process.env.RUN_MANUAL_TESTS_ONLY == "true") { }); }); }); + + describe.only('keepAlive test', function () { + let connection; + 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 () => { + snowflake.configure({keepAlive: true}); + await testUtil.dropTablesIgnoringErrorsAsync(connection, [tableName]); + await testUtil.destroyConnectionAsync(connection); + }); + + function executeSingleQuery() { + return new Promise((resolve, reject) => { + const start = Date.now(); + connection.execute({ + sqlText: `SELECT VALUE + from ${tableName} limit ${rowCount};`, + streamResult: true, + complete: function (err, stmt) { + if (err) { + throw err; + } else { + stmt.streamRows() + .on('error', function (err) { + throw err; + }) + .on('data', function (row) { + return; + }) + .on('end', function (row) { + const end = Date.now(); + const time = end - start; + resolve(time); + }); + } + } + }); + }); + } + + it('Verify that requests working faster with keep alive', async function () { + let sumWithKeepAlive = 0; + let sumWithoutKeepAlive = 0; + for (let count = 1; count <= loopCount; count++) { + let time = await executeSingleQuery(); + sumWithKeepAlive += time; + } + snowflake.configure({keepAlive: false}); + for (let count = 1; count <= loopCount; count++) { + let time = await executeSingleQuery(); + sumWithoutKeepAlive += time; + } + console.log(`Sum of time without keep alive: ${sumWithoutKeepAlive}. Sum of time with keep alive:: ${sumWithKeepAlive}`); + assert.ok(sumWithoutKeepAlive * 0.66 > sumWithKeepAlive, 'With keep alive the queries should work faster'); + }); + }); } 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]; }