From a6367ffc1f0447cea6a6f1a9cbfe097a4199563a Mon Sep 17 00:00:00 2001 From: Przemyslaw Motacki Date: Thu, 21 Sep 2023 14:58:29 +0200 Subject: [PATCH] SNOW-734920: Add possibility to configure keep alive value --- lib/constants/error_messages.js | 1 + lib/core.js | 9 +++ lib/errors.js | 1 + lib/global_config.js | 26 ++++++- lib/http/node.js | 13 ++-- test/integration/testKeepAlive.js | 125 ++++++++++++++++++------------ 6 files changed, 118 insertions(+), 57 deletions(-) diff --git a/lib/constants/error_messages.js b/lib/constants/error_messages.js index 0b2887103..a2fdb0e84 100644 --- a/lib/constants/error_messages.js +++ b/lib/constants/error_messages.js @@ -22,6 +22,7 @@ exports[403002] = 'Invalid insecureConnect option. The specified value must be a exports[403003] = 'Invalid OCSP mode. The specified value must be FAIL_CLOSED, FAIL_OPEN, or INSECURE_MODE.'; exports[403004] = 'Invalid custom JSON parser. The specified value must be a function.'; exports[403005] = 'Invalid custom XML parser. The specified value must be a function.'; +exports[403006] = 'Invalid keep alive value. The specified value must be a boolean.'; // 404001 exports[404001] = 'Connection options must be specified.'; diff --git a/lib/core.js b/lib/core.js index 3601c08b4..9dc26a2db 100644 --- a/lib/core.js +++ b/lib/core.js @@ -217,6 +217,15 @@ function Core(options) } else if (Util.exists(xmlParserConfig)) { GlobalConfig.createXmlColumnVariantParserWithParameters(xmlParserConfig); } + + let keepAlive = options.keepAlive; + if (Util.exists(keepAlive)) + { + Errors.checkArgumentValid(Util.isBoolean(keepAlive), + ErrorCodes.ERR_GLOBAL_CONFIGURE_INVALID_KEEP_ALIVE); + + GlobalConfig.setKeepAlive(keepAlive); + } } }; diff --git a/lib/errors.js b/lib/errors.js index 5361aedb0..44f1a6c2d 100644 --- a/lib/errors.js +++ b/lib/errors.js @@ -27,6 +27,7 @@ codes.ERR_GLOBAL_CONFIGURE_INVALID_INSECURE_CONNECT = 403002; codes.ERR_GLOBAL_CONFIGURE_INVALID_OCSP_MODE = 403003; codes.ERR_GLOBAL_CONFIGURE_INVALID_JSON_PARSER = 403004; codes.ERR_GLOBAL_CONFIGURE_INVALID_XML_PARSER = 403005; +codes.ERR_GLOBAL_CONFIGURE_INVALID_KEEP_ALIVE = 403006; // 404001 codes.ERR_CONN_CREATE_MISSING_OPTIONS = 404001; diff --git a/lib/global_config.js b/lib/global_config.js index ce347eea9..d9a4d4e87 100644 --- a/lib/global_config.js +++ b/lib/global_config.js @@ -255,5 +255,29 @@ function createXmlColumnVariantParser(config) { throw new Error(validateResult.err.msg); } }; - } + +let keepAlive = true; + +/** + * Updates the value of the 'keepAlive' parameter. + * + * @param {boolean} value + */ +exports.setKeepAlive = function (value) +{ + // validate input + Errors.assertInternal(Util.isBoolean(value)); + keepAlive = value; +}; + +/** + * Returns the overriden value of 'keepAlive' or default if not set. Default value is true + * + * @param {boolean} value + */ +exports.keepAlive = function (config) { + return keepAlive; +}; + + diff --git a/lib/http/node.js b/lib/http/node.js index 49cf8062a..d8d8ba8a6 100644 --- a/lib/http/node.js +++ b/lib/http/node.js @@ -8,6 +8,7 @@ const Base = require('./base'); const HttpsAgent = require('../agent/https_ocsp_agent'); const HttpsProxyAgent = require('../agent/https_proxy_ocsp_agent'); const HttpAgent = require('http').Agent; +const GlobalConfig = require('../../lib/global_config'); const Logger = require('../logger'); /** @@ -47,7 +48,7 @@ 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}` + 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); @@ -72,11 +73,11 @@ function prepareProxyAgentOptions(agentOptions, proxy) { function isBypassProxy(proxy, url, bypassProxy) { if (proxy && proxy.noProxy) { - var bypassList = proxy.noProxy.split("|"); + let bypassList = proxy.noProxy.split("|"); for (let i = 0; i < bypassList.length; i++) { host = bypassList[i].trim(); host = host.replace("*", ".*?"); - var matches = url.match(host); + let matches = url.match(host); if (matches) { Logger.getInstance().debug("bypassing proxy for %s", url); return true; @@ -89,9 +90,9 @@ function isBypassProxy(proxy, url, bypassProxy) { /** * @inheritDoc */ -NodeHttpClient.prototype.getAgent = function (url, proxy, mock) { +NodeHttpClient.prototype.getAgent = function (url, proxy, mock, defaultAgentOptions) { const isHttps = Url.parse(url).protocol === 'https:'; - let agentOptions = {keepAlive: false}; + let agentOptions = {keepAlive: GlobalConfig.keepAlive()}; var bypassProxy = isBypassProxy(proxy, url); if (mock) { @@ -106,7 +107,7 @@ NodeHttpClient.prototype.getAgent = function (url, proxy, mock) { return getFromCacheOrCreate(HttpsAgent, agentOptions, url); } } else if (proxy && !bypassProxy) { - let proxyAgentOptions = prepareProxyAgentOptions(); + const proxyAgentOptions = prepareProxyAgentOptions(agentOptions, proxy); return getFromCacheOrCreate(HttpAgent, proxyAgentOptions, url); } else { return getFromCacheOrCreate(HttpAgent, agentOptions, url); diff --git a/test/integration/testKeepAlive.js b/test/integration/testKeepAlive.js index cf5ef76e4..de643ae79 100644 --- a/test/integration/testKeepAlive.js +++ b/test/integration/testKeepAlive.js @@ -7,81 +7,106 @@ const connOption = require('./connectionOptions'); const async = require('async'); const testUtil = require('./testUtil'); const { performance } = require('perf_hooks'); +const assert = require("assert"); -describe.skip('keepAlive perf test', function () +describe('keepAlive test', function () { - this.timeout(1000000); + let connection; + const loopCount = 5; + const rowCount = 10; + const tableName = 'test_keepalive000'; - var connection; - const LOOP_COUNT = 100; + const createTableWithRandomStrings = `CREATE OR REPLACE TABLE ${tableName} (value string) + AS select randstr(200, random()) from table (generator(rowcount =>${rowCount}))`; - before(function (done) - { - console.time('execution'); - done(); - }); - after(function (done) - { - testUtil.destroyConnection(connection, done); + 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( [ - // Create the connection - function (callback) + // Run the query loopCount times + async function () { - console.time('connecting') connection = snowflake.createConnection(connOption.valid); - connection.connect(function (err, conn) + await testUtil.connectAsync(connection); + await testUtil.executeCmdAsync(connection, createTableWithRandomStrings); + for (let count = 1; count <= loopCount; count++) { - if (err) - { - console.error('Unable to connect: ' + err.message); - } else + await new Promise((resolve, reject) => { - console.timeEnd('connecting'); - callback(); - } - }); + 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(); + }); + } + } + }); + }); + } }, - // Run the query LOOP_COUNT times async function () { - var sum = 0; - for (var count = 1; count <= LOOP_COUNT; count++) + 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) => { - var start = performance.now(); - var statement = connection.execute({ - sqlText: "SELECT VALUE from VARIANT_TABLE2 limit 10000;", - complete: function (err, stmt, rows) - { - var stream = statement.streamRows(); - stream.on('error', function (err) - { + const start = performance.now(); + connection.execute({ + sqlText: `SELECT VALUE from ${tableName} limit ${rowCount};`, + streamResult: true, + complete: function (err, stmt) { + if (err) { done(err); - }); - stream.on('data', function (row) - { - return; - }); - stream.on('end', function (row) - { - var end = performance.now(); - var time = end - start; - console.log("query: " + time); - sum += time; - resolve(); - }); + } 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(); + }); + } } }); }); } - console.log("query average: " + (sum / LOOP_COUNT)); - console.timeEnd('execution'); + }, + async function () + { + assert.ok(sumWithoutKeepAlive/2 > sumWithKeepAlive, 'With keep alive queries should work more thdn two times faster'); } ], done