From 1e5a951455784a251aa9757ccb861eebe32b5710 Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-jy Date: Thu, 19 Oct 2023 01:12:35 -0700 Subject: [PATCH 01/23] Retry Strategy --- lib/connection/connection_config.js | 6 +++--- lib/services/sf.js | 9 +++++++- lib/util.js | 16 ++++++++++++++ test/unit/mock/mock_http_client.js | 33 ++++++++++++++++++++++------- 4 files changed, 52 insertions(+), 12 deletions(-) diff --git a/lib/connection/connection_config.js b/lib/connection/connection_config.js index 7fc76fa5d..ceef29f12 100644 --- a/lib/connection/connection_config.js +++ b/lib/connection/connection_config.js @@ -948,7 +948,7 @@ function createParameters() }, { name: PARAM_RETRY_SF_MAX_LOGIN_RETRIES, - defaultValue: 5, + defaultValue: 7, external: true, validate: isNonNegativeInteger }, @@ -959,12 +959,12 @@ function createParameters() }, { name: PARAM_RETRY_SF_STARTING_SLEEP_TIME, - defaultValue: 0.25, + defaultValue: 1, validate: isNonNegativeNumber }, { name: PARAM_RETRY_SF_MAX_SLEEP_TIME, - defaultValue: 16, + defaultValue: 300, validate: isNonNegativeNumber } ]; diff --git a/lib/services/sf.js b/lib/services/sf.js index e0f714842..4f611d2ad 100644 --- a/lib/services/sf.js +++ b/lib/services/sf.js @@ -1173,6 +1173,7 @@ StateConnecting.prototype.continue = function () const maxLoginRetries = connectionConfig.getRetrySfMaxLoginRetries(); let sleep = connectionConfig.getRetrySfStartingSleepTime(); const cap = connectionConfig.getRetrySfMaxSleepTime(); + // const isLoginRequest = Logger.getInstance().debug("Retry Max sleep time = " + cap); const parent = this; const requestCallback = function (err, body) @@ -1206,7 +1207,8 @@ StateConnecting.prototype.continue = function () isRetryableNetworkError(err) || isRetryableHttpError(err))) { numRetries++; - sleep = Util.nextSleepTime(1, cap, sleep); + // sleep = Util.nextSleepTime(1, cap, sleep); + sleep = Util.jitteredSleepTime(numRetries, cap); setTimeout(sendRequest, sleep * 1000); return; } @@ -1239,6 +1241,10 @@ StateConnecting.prototype.continue = function () const sendRequest = function () { const targetUrl = buildLoginUrl(connectionConfig); + const headers = { + "CLIENT_APP_VERSION": connectionConfig.getClientVersion(), + "CLIENT_APP_ID": "JavaScript", + } Logger.getInstance().debug( "Contacting SF: %s, (%s/%s)", targetUrl, numRetries, maxLoginRetries); const request = parent.createUnauthenticatedRequest({ @@ -1248,6 +1254,7 @@ StateConnecting.prototype.continue = function () scope: this, startTime: startTime, retry: numRetries, + headers: headers, callback: requestCallback }); request.send(); diff --git a/lib/util.js b/lib/util.js index 8c822b4f1..5dab8aefd 100644 --- a/lib/util.js +++ b/lib/util.js @@ -420,6 +420,22 @@ exports.nextSleepTime = function ( Math.min(base, previousSleep * 3)); }; +exports.jitteredSleepTime = function (numofRetries,maxSleepTime) { + const temp = Math.min(maxSleepTime, 2** numofRetries); + return temp / 2 + getJitter(temp); + +} + +function getJitter (curWaitTime) { + const multiplicationFactor = chooseRandom(1, -1); + const jitterAmmount = 0.5 * curWaitTime * multiplicationFactor; + return jitterAmmount; +} + +function chooseRandom (firstNumber, secondNumber) { + const random = Math.floor(Math.random()); + return random % 2 === 0 ? firstNumber : secondNumber; +} /** * Checks if the HTTP response code is retryable * diff --git a/test/unit/mock/mock_http_client.js b/test/unit/mock/mock_http_client.js index 05d3c346b..551733cf7 100644 --- a/test/unit/mock/mock_http_client.js +++ b/test/unit/mock/mock_http_client.js @@ -174,7 +174,9 @@ function buildRequestOutputMappings(clientInfo) headers: { 'Accept': 'application/json', - 'Content-Type': 'application/json' + 'Content-Type': 'application/json', + "CLIENT_APP_VERSION": clientInfo.version, + "CLIENT_APP_ID": "JavaScript", }, json: { @@ -1158,7 +1160,9 @@ function buildRequestOutputMappings(clientInfo) headers: { 'Accept': 'application/json', - 'Content-Type': 'application/json' + 'Content-Type': 'application/json', + "CLIENT_APP_VERSION": clientInfo.version, + "CLIENT_APP_ID": "JavaScript" }, json: { @@ -1373,7 +1377,9 @@ function buildRequestOutputMappings(clientInfo) headers: { 'Accept': 'application/json', - 'Content-Type': 'application/json' + 'Content-Type': 'application/json', + "CLIENT_APP_VERSION": clientInfo.version, + "CLIENT_APP_ID": "JavaScript" }, json: { @@ -1478,7 +1484,9 @@ function buildRequestOutputMappings(clientInfo) headers: { 'Accept': 'application/json', - 'Content-Type': 'application/json' + 'Content-Type': 'application/json', + "CLIENT_APP_VERSION": clientInfo.version, + "CLIENT_APP_ID": "JavaScript" }, json: { @@ -1583,7 +1591,9 @@ function buildRequestOutputMappings(clientInfo) headers: { 'Accept': 'application/json', - 'Content-Type': 'application/json' + 'Content-Type': 'application/json', + "CLIENT_APP_VERSION": clientInfo.version, + "CLIENT_APP_ID": "JavaScript" }, json: { @@ -1670,7 +1680,9 @@ function buildRequestOutputMappings(clientInfo) headers: { 'Accept': 'application/json', - 'Content-Type': 'application/json' + 'Content-Type': 'application/json', + "CLIENT_APP_VERSION": clientInfo.version, + "CLIENT_APP_ID": "JavaScript" }, json: { @@ -1706,7 +1718,10 @@ function buildRequestOutputMappings(clientInfo) headers: { 'Accept': 'application/json', - 'Content-Type': 'application/json' + 'Content-Type': 'application/json', + "CLIENT_APP_VERSION": clientInfo.version, + "CLIENT_APP_ID": "JavaScript" + }, json: { @@ -1742,7 +1757,9 @@ function buildRequestOutputMappings(clientInfo) headers: { 'Accept': 'application/json', - 'Content-Type': 'application/json' + 'Content-Type': 'application/json', + "CLIENT_APP_VERSION": clientInfo.version, + "CLIENT_APP_ID": "JavaScript" }, json: { From 66fe71ce5419dc0c9d67ba12988dd1e0dd337c63 Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-jy Date: Thu, 19 Oct 2023 09:10:11 -0700 Subject: [PATCH 02/23] changed the Jitter to the original --- lib/services/sf.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/services/sf.js b/lib/services/sf.js index 4f611d2ad..c106f598f 100644 --- a/lib/services/sf.js +++ b/lib/services/sf.js @@ -1207,8 +1207,8 @@ StateConnecting.prototype.continue = function () isRetryableNetworkError(err) || isRetryableHttpError(err))) { numRetries++; - // sleep = Util.nextSleepTime(1, cap, sleep); - sleep = Util.jitteredSleepTime(numRetries, cap); + sleep = Util.nextSleepTime(1, cap, sleep); + // sleep = Util.jitteredSleepTime(numRetries, cap); setTimeout(sendRequest, sleep * 1000); return; } From e3f6fb5005fa146b713d88435ab46cb106021060 Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-jy Date: Thu, 19 Oct 2023 14:16:46 -0700 Subject: [PATCH 03/23] changed jitter --- lib/connection/connection_config.js | 2 +- lib/services/sf.js | 5 +++-- lib/util.js | 9 +++++---- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/lib/connection/connection_config.js b/lib/connection/connection_config.js index ceef29f12..4b37a62b9 100644 --- a/lib/connection/connection_config.js +++ b/lib/connection/connection_config.js @@ -964,7 +964,7 @@ function createParameters() }, { name: PARAM_RETRY_SF_MAX_SLEEP_TIME, - defaultValue: 300, + defaultValue: 16, validate: isNonNegativeNumber } ]; diff --git a/lib/services/sf.js b/lib/services/sf.js index c106f598f..1c89a5d74 100644 --- a/lib/services/sf.js +++ b/lib/services/sf.js @@ -1207,8 +1207,9 @@ StateConnecting.prototype.continue = function () isRetryableNetworkError(err) || isRetryableHttpError(err))) { numRetries++; - sleep = Util.nextSleepTime(1, cap, sleep); - // sleep = Util.jitteredSleepTime(numRetries, cap); + // sleep = Util.nextSleepTime(1, cap, sleep); + sleep = Util.jitteredSleepTime(numRetries, sleep, cap); + console.log(sleep); setTimeout(sendRequest, sleep * 1000); return; } diff --git a/lib/util.js b/lib/util.js index 5dab8aefd..52dc92cd6 100644 --- a/lib/util.js +++ b/lib/util.js @@ -406,6 +406,7 @@ exports.isNode = function () * Returns the next sleep time calculated by exponential backoff with * decorrelated jitter. * sleep = min(cap, random_between(base, sleep * 3)) + * * for more details, check out: * http://www.awsarchitectureblog.com/2015/03/backoff.html * @param base minimum seconds @@ -420,9 +421,9 @@ exports.nextSleepTime = function ( Math.min(base, previousSleep * 3)); }; -exports.jitteredSleepTime = function (numofRetries,maxSleepTime) { - const temp = Math.min(maxSleepTime, 2** numofRetries); - return temp / 2 + getJitter(temp); +exports.jitteredSleepTime = function (numofRetries, currentSleepTime, maxSleepTime) { + const nextSleepTime = Math.min(maxSleepTime, 2 ** numofRetries); + return nextSleepTime + getJitter(currentSleepTime); } @@ -433,7 +434,7 @@ function getJitter (curWaitTime) { } function chooseRandom (firstNumber, secondNumber) { - const random = Math.floor(Math.random()); + const random = Math.floor(Math.random()*1000); return random % 2 === 0 ? firstNumber : secondNumber; } /** From a233abdf28a802e17fc208d3fa0519ca5258ec2c Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-jy Date: Thu, 19 Oct 2023 16:12:49 -0700 Subject: [PATCH 04/23] switched the point to add headers and make a function --- lib/services/sf.js | 13 ++++++++----- lib/util.js | 7 ++++++- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/lib/services/sf.js b/lib/services/sf.js index 1c89a5d74..554ef643f 100644 --- a/lib/services/sf.js +++ b/lib/services/sf.js @@ -781,6 +781,14 @@ function StateAbstract(options) requestOptions.headers = Util.apply(this.getDefaultReqHeaders(), requestOptions.headers || {}); + if (Util.isLoginRequest(requestOptions.url)) { + Util.apply(requestOptions.headers, { + "CLIENT_APP_VERSION": requestOptions.json.data.CLIENT_APP_VERSION, + "CLIENT_APP_ID": requestOptions.json.data.CLIENT_APP_ID, + }) + + } + // augment the options with the absolute url requestOptions.absoluteUrl = this.buildFullUrl(requestOptions.url); }; @@ -1242,10 +1250,6 @@ StateConnecting.prototype.continue = function () const sendRequest = function () { const targetUrl = buildLoginUrl(connectionConfig); - const headers = { - "CLIENT_APP_VERSION": connectionConfig.getClientVersion(), - "CLIENT_APP_ID": "JavaScript", - } Logger.getInstance().debug( "Contacting SF: %s, (%s/%s)", targetUrl, numRetries, maxLoginRetries); const request = parent.createUnauthenticatedRequest({ @@ -1255,7 +1259,6 @@ StateConnecting.prototype.continue = function () scope: this, startTime: startTime, retry: numRetries, - headers: headers, callback: requestCallback }); request.send(); diff --git a/lib/util.js b/lib/util.js index 52dc92cd6..0833d33c6 100644 --- a/lib/util.js +++ b/lib/util.js @@ -434,9 +434,14 @@ function getJitter (curWaitTime) { } function chooseRandom (firstNumber, secondNumber) { - const random = Math.floor(Math.random()*1000); + const random = Math.floor(Math.random() * 1000); return random % 2 === 0 ? firstNumber : secondNumber; } + +exports.isLoginRequest = function (loginUrl) { + const endPoints = ['/v1/login-request', '/authenticator-request', '/token-request',]; + return endPoints.some((endPoint) => loginUrl.includes(endPoint)); +} /** * Checks if the HTTP response code is retryable * From c2299fc84a96f4accc58f76b0523348d4df7975c Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-jy Date: Fri, 20 Oct 2023 00:21:10 -0700 Subject: [PATCH 05/23] add testing cases --- test/unit/util_test.js | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/test/unit/util_test.js b/test/unit/util_test.js index 6a0e11e57..92c0d5f05 100644 --- a/test/unit/util_test.js +++ b/test/unit/util_test.js @@ -522,6 +522,42 @@ describe('Util', function () } }) + it("Util.isLoginRequest Test", function () { + const baseUrl = 'wwww.test.com'; + const testCases = + [ + { + endPoint: '/v1/login-request', + result: true, + }, + { + endPoint: '/login-request', + result: false, + }, + { + endPoint: '/authenticator-request', + result:true, + }, + { + endPoint: '/authenticator-requ', + result:false, + }, + { + endPoint: '/token-request', + result:true, + }, + { + endPoint: '/tokenRequest', + result:false, + } + ]; + for (const {endPoint,result} of testCases) + { + const isLoginRequest = Util.isLoginRequest(baseUrl+endPoint); + assert.strictEqual(isLoginRequest,result); + } + }) + it('Util.apply()', function () { assert.strictEqual(Util.apply(null, null), null); From 11bc6a5bc12eaa795c9b0ad433c495ad0edbe7cd Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-jy Date: Fri, 20 Oct 2023 11:00:06 -0700 Subject: [PATCH 06/23] added testing for jitteredSleepTime and add comments --- lib/util.js | 31 ++++++++++++++++++++++++++++++- test/unit/util_test.js | 11 +++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/lib/util.js b/lib/util.js index 0833d33c6..21cebd904 100644 --- a/lib/util.js +++ b/lib/util.js @@ -421,27 +421,56 @@ exports.nextSleepTime = function ( Math.min(base, previousSleep * 3)); }; + +/** + * Return next sleep time calculated by the jitter rule. + * + * @param {Number} numofRetries + * @param {Number} currentSleepTime + * @param {Number} maxSleepTime + * @returns {Number} return next sleep Time.. + */ exports.jitteredSleepTime = function (numofRetries, currentSleepTime, maxSleepTime) { const nextSleepTime = Math.min(maxSleepTime, 2 ** numofRetries); return nextSleepTime + getJitter(currentSleepTime); - } +/** + * Check whether the request is the login-request or not. + * + * @param {Number} curWaitTime + * @returns {Boolean} return jitter.. + */ function getJitter (curWaitTime) { const multiplicationFactor = chooseRandom(1, -1); const jitterAmmount = 0.5 * curWaitTime * multiplicationFactor; return jitterAmmount; } +/** + * Choose one of the number between two numbers. + * + * @param {Number} firstNumber + * @param {Number} secondNumber + * @returns {Boolean} return one of the argument randomly. + */ + function chooseRandom (firstNumber, secondNumber) { const random = Math.floor(Math.random() * 1000); return random % 2 === 0 ? firstNumber : secondNumber; } +/** + * Check whether the request is the login-request or not. + * + * @param loginurl HTTP request url + * @returns {Boolean} true if it is loginRequest, otherwise false. + */ exports.isLoginRequest = function (loginUrl) { const endPoints = ['/v1/login-request', '/authenticator-request', '/token-request',]; return endPoints.some((endPoint) => loginUrl.includes(endPoint)); } + /** * Checks if the HTTP response code is retryable * diff --git a/test/unit/util_test.js b/test/unit/util_test.js index 92c0d5f05..a12d4aef6 100644 --- a/test/unit/util_test.js +++ b/test/unit/util_test.js @@ -558,6 +558,17 @@ describe('Util', function () } }) + it("Util.jitterSleepTime Test", function () { + const maxSleepTime = 16; + const currentSleepTime = Math.min(maxSleepTime, (Math.random()*15) + 1); + const numRetries = Math.min(4, (Math.random() * 3) + 1); + const result = Util.jitteredSleepTime(numRetries, currentSleepTime, maxSleepTime); + const jitter = currentSleepTime / 2 + const nextSleep = 2 ** numRetries; + + assert.ok(result === nextSleep + jitter || result === nextSleep - jitter) + }) + it('Util.apply()', function () { assert.strictEqual(Util.apply(null, null), null); From 43e8442e73a0893cc667a32c146cab1f41dd7406 Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-jy Date: Fri, 20 Oct 2023 11:24:12 -0700 Subject: [PATCH 07/23] Removed spaces and formatted code --- lib/services/sf.js | 4 ---- lib/util.js | 4 +--- test/unit/util_test.js | 13 +++++++------ 3 files changed, 8 insertions(+), 13 deletions(-) diff --git a/lib/services/sf.js b/lib/services/sf.js index bcec47fac..2f55c0390 100644 --- a/lib/services/sf.js +++ b/lib/services/sf.js @@ -787,7 +787,6 @@ function StateAbstract(options) "CLIENT_APP_VERSION": requestOptions.json.data.CLIENT_APP_VERSION, "CLIENT_APP_ID": requestOptions.json.data.CLIENT_APP_ID, }) - } // augment the options with the absolute url @@ -1182,7 +1181,6 @@ StateConnecting.prototype.continue = function () const maxLoginRetries = connectionConfig.getRetrySfMaxLoginRetries(); let sleep = connectionConfig.getRetrySfStartingSleepTime(); const cap = connectionConfig.getRetrySfMaxSleepTime(); - // const isLoginRequest = Logger.getInstance().debug("Retry Max sleep time = " + cap); const parent = this; const requestCallback = function (err, body) @@ -1216,9 +1214,7 @@ StateConnecting.prototype.continue = function () isRetryableNetworkError(err) || isRetryableHttpError(err))) { numRetries++; - // sleep = Util.nextSleepTime(1, cap, sleep); sleep = Util.jitteredSleepTime(numRetries, sleep, cap); - console.log(sleep); setTimeout(sendRequest, sleep * 1000); return; } diff --git a/lib/util.js b/lib/util.js index 21cebd904..29d6a1a82 100644 --- a/lib/util.js +++ b/lib/util.js @@ -405,8 +405,7 @@ exports.isNode = function () /** * Returns the next sleep time calculated by exponential backoff with * decorrelated jitter. - * sleep = min(cap, random_between(base, sleep * 3)) - * + * sleep = min(cap, random_between(base, sleep * 3)) * for more details, check out: * http://www.awsarchitectureblog.com/2015/03/backoff.html * @param base minimum seconds @@ -454,7 +453,6 @@ function getJitter (curWaitTime) { * @param {Number} secondNumber * @returns {Boolean} return one of the argument randomly. */ - function chooseRandom (firstNumber, secondNumber) { const random = Math.floor(Math.random() * 1000); return random % 2 === 0 ? firstNumber : secondNumber; diff --git a/test/unit/util_test.js b/test/unit/util_test.js index a12d4aef6..70000555d 100644 --- a/test/unit/util_test.js +++ b/test/unit/util_test.js @@ -520,7 +520,7 @@ describe('Util', function () assert.strictEqual(url, testCase.result); }) } - }) + }); it("Util.isLoginRequest Test", function () { const baseUrl = 'wwww.test.com'; @@ -549,14 +549,15 @@ describe('Util', function () { endPoint: '/tokenRequest', result:false, - } + }, ]; + for (const {endPoint,result} of testCases) { - const isLoginRequest = Util.isLoginRequest(baseUrl+endPoint); - assert.strictEqual(isLoginRequest,result); + const isLoginRequest = Util.isLoginRequest(baseUrl + endPoint); + assert.strictEqual(isLoginRequest, result); } - }) + }); it("Util.jitterSleepTime Test", function () { const maxSleepTime = 16; @@ -567,7 +568,7 @@ describe('Util', function () const nextSleep = 2 ** numRetries; assert.ok(result === nextSleep + jitter || result === nextSleep - jitter) - }) + }); it('Util.apply()', function () { From e50573ee43ec5395229afd92834837ab2580b6f0 Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-jy Date: Fri, 20 Oct 2023 12:56:08 -0700 Subject: [PATCH 08/23] refactor isLoginRequest test --- test/unit/util_test.js | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/test/unit/util_test.js b/test/unit/util_test.js index 70000555d..74299be91 100644 --- a/test/unit/util_test.js +++ b/test/unit/util_test.js @@ -522,40 +522,48 @@ describe('Util', function () } }); - it("Util.isLoginRequest Test", function () { + describe("Util.isLoginRequest Test", function () { const baseUrl = 'wwww.test.com'; const testCases = [ { + testName: 'test URL with a right login end point', endPoint: '/v1/login-request', result: true, }, { + testName: 'test URL with a wrong login end point', endPoint: '/login-request', result: false, }, { + testName: 'test URL with a right authenticator-request point', endPoint: '/authenticator-request', result:true, }, { + testName: 'test URL with a wrong authenticator-request point', endPoint: '/authenticator-requ', result:false, }, { + testName: 'test URL with a right token point', endPoint: '/token-request', result:true, }, { + testName: 'test URL with a wrong token point', endPoint: '/tokenRequest', result:false, }, ]; - for (const {endPoint,result} of testCases) + for (const {testName, endPoint,result} of testCases) { - const isLoginRequest = Util.isLoginRequest(baseUrl + endPoint); - assert.strictEqual(isLoginRequest, result); + it(testName, function () { + const isLoginRequest = Util.isLoginRequest(baseUrl + endPoint); + assert.strictEqual(isLoginRequest, result); + }) } }); From 15d7523a1694b09a899af0e88fff65f894c6914b Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-jy Date: Mon, 23 Oct 2023 00:28:51 -0700 Subject: [PATCH 09/23] added max login time out, and refactored jittered time testing case --- lib/connection/connection_config.js | 17 +++++ lib/constants/error_messages.js | 1 + lib/errors.js | 1 + lib/services/sf.js | 9 ++- lib/util.js | 53 +++++++++---- .../unit/connection/connection_config_test.js | 31 +++++++- test/unit/util_test.js | 74 +++++++++++++++++-- 7 files changed, 160 insertions(+), 26 deletions(-) diff --git a/lib/connection/connection_config.js b/lib/connection/connection_config.js index 4b37a62b9..41ab172ee 100644 --- a/lib/connection/connection_config.js +++ b/lib/connection/connection_config.js @@ -483,6 +483,14 @@ function ConnectionConfig(options, validateCredentials, qaMode, clientInfo) disableQueryContextCache = options.disableQueryContextCache; } + let maxLoginTimeout = 300; + if (Util.exists(options.maxLoginTimeout)) { + Errors.checkArgumentValid(Util.isNumber(options.maxLoginTimeout), + ErrorCodes.ERR_CONN_CREATE_INVALID_MAX_LOGIN_TIMEOUT); + + maxLoginTimeout = options.maxLoginTimeout; + } + if (validateDefaultParameters) { for (const [key] of Object.entries(options)) @@ -793,6 +801,15 @@ function ConnectionConfig(options, validateCredentials, qaMode, clientInfo) return clientConfigFile; }; + /** + * Returns the max login timeout + * + * @returns {Number} + */ + this.getMaxLoginTimeout = function () { + return maxLoginTimeout; + } + // save config options this.username = options.username; this.password = options.password; diff --git a/lib/constants/error_messages.js b/lib/constants/error_messages.js index ad6b407c2..1618bd5f9 100644 --- a/lib/constants/error_messages.js +++ b/lib/constants/error_messages.js @@ -68,6 +68,7 @@ exports[404040] = 'Invalid browser timeout value. The specified value must be a exports[404041] = 'Invalid disablQueryContextCache. The specified value must be a boolean.'; exports[404042] = 'Invalid includeRetryReason. The specified value must be a boolean.' exports[404043] = 'Invalid clientConfigFile value. The specified value must be a string.'; +exports[404044] = 'Invalid loginMaxTimeout value. The specified value must be a number.'; // 405001 exports[405001] = 'Invalid callback. The specified value must be a function.'; diff --git a/lib/errors.js b/lib/errors.js index c04fe7b5a..4806617b1 100644 --- a/lib/errors.js +++ b/lib/errors.js @@ -73,6 +73,7 @@ codes.ERR_CONN_CREATE_INVALID_BROWSER_TIMEOUT = 404040; codes.ERR_CONN_CREATE_INVALID_DISABLED_QUERY_CONTEXT_CACHE = 404041 codes.ERR_CONN_CREATE_INVALID_INCLUDE_RETRY_REASON =404042 codes.ERR_CONN_CREATE_INVALID_CLIENT_CONFIG_FILE = 404043; +codes.ERR_CONN_CREATE_INVALID_MAX_LOGIN_TIMEOUT = 404044; // 405001 codes.ERR_CONN_CONNECT_INVALID_CALLBACK = 405001; diff --git a/lib/services/sf.js b/lib/services/sf.js index 2f55c0390..144196bc5 100644 --- a/lib/services/sf.js +++ b/lib/services/sf.js @@ -1179,7 +1179,9 @@ StateConnecting.prototype.continue = function () const startTime = connectionConfig.accessUrl.startsWith('https://') ? Date.now() : 'FIXEDTIMESTAMP'; const maxLoginRetries = connectionConfig.getRetrySfMaxLoginRetries(); + const maxLoginTimeout = connectionConfig.getMaxLoginTimeout(); let sleep = connectionConfig.getRetrySfStartingSleepTime(); + let totalTimeout = sleep; const cap = connectionConfig.getRetrySfMaxSleepTime(); Logger.getInstance().debug("Retry Max sleep time = " + cap); const parent = this; @@ -1211,10 +1213,13 @@ StateConnecting.prototype.continue = function () if (Errors.isNetworkError(err) || Errors.isRequestFailedError(err)) { if (numRetries < maxLoginRetries && ( - isRetryableNetworkError(err) || isRetryableHttpError(err))) + isRetryableNetworkError(err) || isRetryableHttpError(err)) && + totalTimeout < maxLoginTimeout) { numRetries++; - sleep = Util.jitteredSleepTime(numRetries, sleep, cap); + const jitter = Util.jitteredSleepTime(numRetries, sleep, cap, totalTimeout, maxLoginTimeout); + sleep = jitter.sleep; + totalTimeout = jitter.totalTimeout; setTimeout(sendRequest, sleep * 1000); return; } diff --git a/lib/util.js b/lib/util.js index 29d6a1a82..f0cf17060 100644 --- a/lib/util.js +++ b/lib/util.js @@ -427,13 +427,48 @@ exports.nextSleepTime = function ( * @param {Number} numofRetries * @param {Number} currentSleepTime * @param {Number} maxSleepTime - * @returns {Number} return next sleep Time.. + * @returns {JSON} return next sleep Time and totalTime. */ -exports.jitteredSleepTime = function (numofRetries, currentSleepTime, maxSleepTime) { +exports.jitteredSleepTime = function (numofRetries, currentSleepTime, maxSleepTime, totalTimeout = 0, maxLoginTimeout = 300) { const nextSleepTime = Math.min(maxSleepTime, 2 ** numofRetries); - return nextSleepTime + getJitter(currentSleepTime); + let sleep = nextSleepTime + getJitter(currentSleepTime); + if (totalTimeout + sleep > maxLoginTimeout) { + sleep = maxLoginTimeout - totalTimeout; + totalTimeout = maxLoginTimeout; + } + else { + totalTimeout += sleep + } + return {sleep ,totalTimeout} } +/** + * Choose one of the number between two numbers. + * + * @param {Number} firstNumber + * @param {Number} secondNumber + * @returns {Boolean} return a random number between two numbers. + */ +function chooseRandom (firstNumber, secondNumber) { + let bigNumber; + let smallNumber; + if (firstNumber > secondNumber) { + bigNumber = firstNumber; + smallNumber = secondNumber; + } else if (firstNumber === secondNumber) { + bigNumber = 1; + smallNumber = 0; + } + else { + bigNumber = secondNumber; + smallNumber = firstNumber; + } + + return (Math.random() * (bigNumber - smallNumber)) + smallNumber; +} + +exports.chooseRandom = chooseRandom; + /** * Check whether the request is the login-request or not. * @@ -446,18 +481,6 @@ function getJitter (curWaitTime) { return jitterAmmount; } -/** - * Choose one of the number between two numbers. - * - * @param {Number} firstNumber - * @param {Number} secondNumber - * @returns {Boolean} return one of the argument randomly. - */ -function chooseRandom (firstNumber, secondNumber) { - const random = Math.floor(Math.random() * 1000); - return random % 2 === 0 ? firstNumber : secondNumber; -} - /** * Check whether the request is the login-request or not. * diff --git a/test/unit/connection/connection_config_test.js b/test/unit/connection/connection_config_test.js index e331a579a..c1f1baedb 100644 --- a/test/unit/connection/connection_config_test.js +++ b/test/unit/connection/connection_config_test.js @@ -546,6 +546,16 @@ describe('ConnectionConfig: basic', function () }, errorCode: ErrorCodes.ERR_CONN_CREATE_INVALID_CLIENT_CONFIG_FILE }, + { + name: 'invalid maxLoginTimeout', + options: { + account: 'account', + username: 'username', + password: 'password', + maxLoginTimeout: 'invalud' + }, + errorCode: ErrorCodes.ERR_CONN_CREATE_INVALID_MAX_LOGIN_TIMEOUT + }, ]; var createNegativeITCallback = function (testCase) @@ -887,13 +897,13 @@ describe('ConnectionConfig: basic', function () username: 'username', password: 'password', account: 'account', - disableQueryContextCache: true + disableQueryContextCache: true, }, options: { accessUrl: 'https://account.snowflakecomputing.com', username: 'username', - password: 'password' + password: 'password', } }, { @@ -914,6 +924,23 @@ describe('ConnectionConfig: basic', function () clientConfigFile: 'easy_logging_config.json' } }, + { + name: 'max login time out', + input: + { + account: 'account', + username: 'username', + password: 'password', + maxLoginTimeout: 100, + }, + options: + { + accessUrl: 'https://account.snowflakecomputing.com', + username: 'username', + password: 'password', + account: 'account', + } + }, ]; var createItCallback = function (testCase) diff --git a/test/unit/util_test.js b/test/unit/util_test.js index 74299be91..eace486bd 100644 --- a/test/unit/util_test.js +++ b/test/unit/util_test.js @@ -568,16 +568,76 @@ describe('Util', function () }); it("Util.jitterSleepTime Test", function () { + const errorCodes = + [ + { + statusCode: 403, + retry403: true, + isRetryable: true, + }, + { + statusCode: 408, + retry403: false, + isRetryable: true, + }, + { + statusCode: 429, + retry403: false, + isRetryable: true, + }, + { + statusCode: 500, + retry403: false, + isRetryable: true, + }, + { + statusCode: 503, + retry403: false, + isRetryable: true, + }, + { + statusCode: 538, + retry403: false, + isRetryable: true, + }, + { + statusCode: 525, + retry403: false, + isRetryable: true, + }, + ]; + const maxSleepTime = 16; - const currentSleepTime = Math.min(maxSleepTime, (Math.random()*15) + 1); - const numRetries = Math.min(4, (Math.random() * 3) + 1); - const result = Util.jitteredSleepTime(numRetries, currentSleepTime, maxSleepTime); - const jitter = currentSleepTime / 2 - const nextSleep = 2 ** numRetries; - - assert.ok(result === nextSleep + jitter || result === nextSleep - jitter) + const maxLoginTimeout = 300; + let currentSleepTime = 1; + let retryCount = 1; + let totalTimeout = 1; + for (const response of errorCodes) { + assert.strictEqual(Util.isRetryableHttpError(response,true), true); + + const result = Util.jitteredSleepTime(retryCount, currentSleepTime, maxSleepTime, totalTimeout, maxLoginTimeout); + const jitter = currentSleepTime / 2 + const nextSleep = 2 ** retryCount; + currentSleepTime = result.sleep; + totalTimeout = result.totalTimeout; + + assert.ok(currentSleepTime <= nextSleep + jitter || currentSleepTime >= nextSleep - jitter) + retryCount++; + } + assert.strictEqual(retryCount, 8); + assert.ok(totalTimeout <= maxLoginTimeout); }); + it("Util.chooseRandom Test", function () { + const positiveInteger = Util.chooseRandom(1, 5); + const negativeInteger = Util.chooseRandom(-1, -5); + assert.ok(1 <= positiveInteger && positiveInteger <= 5); + assert.ok(-5 <= negativeInteger && negativeInteger <= -1); + + const randomNumber = Util.chooseRandom(positiveInteger, negativeInteger); + assert.ok(negativeInteger <= randomNumber && randomNumber <= positiveInteger) + }) + it('Util.apply()', function () { assert.strictEqual(Util.apply(null, null), null); From 9c7a7e4fdca5882aeeba940ef075574440bff492 Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-jy Date: Mon, 23 Oct 2023 13:16:55 -0700 Subject: [PATCH 10/23] edited the loginTimeout options --- lib/connection/connection_config.js | 15 ++++++++------- lib/constants/error_messages.js | 2 +- lib/errors.js | 2 +- lib/services/sf.js | 2 +- test/unit/connection/connection_config_test.js | 6 +++--- 5 files changed, 14 insertions(+), 13 deletions(-) diff --git a/lib/connection/connection_config.js b/lib/connection/connection_config.js index 41ab172ee..fc8811f85 100644 --- a/lib/connection/connection_config.js +++ b/lib/connection/connection_config.js @@ -51,6 +51,7 @@ const DEFAULT_PARAMS = 'forceStageBindError', 'includeRetryReason', 'disableQueryContextCache', + 'loginTimeout' ]; function consolidateHostAndAccount(options) @@ -483,12 +484,12 @@ function ConnectionConfig(options, validateCredentials, qaMode, clientInfo) disableQueryContextCache = options.disableQueryContextCache; } - let maxLoginTimeout = 300; - if (Util.exists(options.maxLoginTimeout)) { - Errors.checkArgumentValid(Util.isNumber(options.maxLoginTimeout), + let loginTimeout = 300; + if (Util.exists(options.loginTimeout)) { + Errors.checkArgumentValid(Util.isNumber(options.loginTimeout), ErrorCodes.ERR_CONN_CREATE_INVALID_MAX_LOGIN_TIMEOUT); - maxLoginTimeout = options.maxLoginTimeout; + loginTimeout = options.loginTimeout > 300 ? options.loginTimeout : 300; } if (validateDefaultParameters) @@ -806,8 +807,8 @@ function ConnectionConfig(options, validateCredentials, qaMode, clientInfo) * * @returns {Number} */ - this.getMaxLoginTimeout = function () { - return maxLoginTimeout; + this.getLoginTimeout = function () { + return loginTimeout; } // save config options @@ -976,7 +977,7 @@ function createParameters() }, { name: PARAM_RETRY_SF_STARTING_SLEEP_TIME, - defaultValue: 1, + defaultValue: 4, validate: isNonNegativeNumber }, { diff --git a/lib/constants/error_messages.js b/lib/constants/error_messages.js index 1618bd5f9..fe7157917 100644 --- a/lib/constants/error_messages.js +++ b/lib/constants/error_messages.js @@ -68,7 +68,7 @@ exports[404040] = 'Invalid browser timeout value. The specified value must be a exports[404041] = 'Invalid disablQueryContextCache. The specified value must be a boolean.'; exports[404042] = 'Invalid includeRetryReason. The specified value must be a boolean.' exports[404043] = 'Invalid clientConfigFile value. The specified value must be a string.'; -exports[404044] = 'Invalid loginMaxTimeout value. The specified value must be a number.'; +exports[404044] = 'Invalid loginTimeout value. The specified value must be a number.'; // 405001 exports[405001] = 'Invalid callback. The specified value must be a function.'; diff --git a/lib/errors.js b/lib/errors.js index 4806617b1..3b08a888b 100644 --- a/lib/errors.js +++ b/lib/errors.js @@ -73,7 +73,7 @@ codes.ERR_CONN_CREATE_INVALID_BROWSER_TIMEOUT = 404040; codes.ERR_CONN_CREATE_INVALID_DISABLED_QUERY_CONTEXT_CACHE = 404041 codes.ERR_CONN_CREATE_INVALID_INCLUDE_RETRY_REASON =404042 codes.ERR_CONN_CREATE_INVALID_CLIENT_CONFIG_FILE = 404043; -codes.ERR_CONN_CREATE_INVALID_MAX_LOGIN_TIMEOUT = 404044; +codes.ERR_CONN_CREATE_INVALID_LOGIN_TIMEOUT = 404044; // 405001 codes.ERR_CONN_CONNECT_INVALID_CALLBACK = 405001; diff --git a/lib/services/sf.js b/lib/services/sf.js index 144196bc5..ec3ee4564 100644 --- a/lib/services/sf.js +++ b/lib/services/sf.js @@ -1179,7 +1179,7 @@ StateConnecting.prototype.continue = function () const startTime = connectionConfig.accessUrl.startsWith('https://') ? Date.now() : 'FIXEDTIMESTAMP'; const maxLoginRetries = connectionConfig.getRetrySfMaxLoginRetries(); - const maxLoginTimeout = connectionConfig.getMaxLoginTimeout(); + const maxLoginTimeout = connectionConfig.getLoginTimeout(); let sleep = connectionConfig.getRetrySfStartingSleepTime(); let totalTimeout = sleep; const cap = connectionConfig.getRetrySfMaxSleepTime(); diff --git a/test/unit/connection/connection_config_test.js b/test/unit/connection/connection_config_test.js index c1f1baedb..d21250fd1 100644 --- a/test/unit/connection/connection_config_test.js +++ b/test/unit/connection/connection_config_test.js @@ -552,7 +552,7 @@ describe('ConnectionConfig: basic', function () account: 'account', username: 'username', password: 'password', - maxLoginTimeout: 'invalud' + loginTimeout: 'invalud' }, errorCode: ErrorCodes.ERR_CONN_CREATE_INVALID_MAX_LOGIN_TIMEOUT }, @@ -925,13 +925,13 @@ describe('ConnectionConfig: basic', function () } }, { - name: 'max login time out', + name: 'login time out', input: { account: 'account', username: 'username', password: 'password', - maxLoginTimeout: 100, + loginTimeout: 1234, }, options: { From 1039fb8523eebdc0023e165ef40873ded9517cb2 Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-jy Date: Mon, 23 Oct 2023 13:53:12 -0700 Subject: [PATCH 11/23] add trailing comma in the default param --- lib/connection/connection_config.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/connection/connection_config.js b/lib/connection/connection_config.js index fc8811f85..3adb3eaa2 100644 --- a/lib/connection/connection_config.js +++ b/lib/connection/connection_config.js @@ -51,7 +51,7 @@ const DEFAULT_PARAMS = 'forceStageBindError', 'includeRetryReason', 'disableQueryContextCache', - 'loginTimeout' + 'loginTimeout', ]; function consolidateHostAndAccount(options) From 3512bbce96a439d59fcfb768c373981e4244d018 Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-jy Date: Mon, 23 Oct 2023 16:45:33 -0700 Subject: [PATCH 12/23] fixed the format of jitter function --- lib/util.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/util.js b/lib/util.js index f0cf17060..284310f17 100644 --- a/lib/util.js +++ b/lib/util.js @@ -439,7 +439,8 @@ exports.jitteredSleepTime = function (numofRetries, currentSleepTime, maxSleepTi else { totalTimeout += sleep } - return {sleep ,totalTimeout} + + return {sleep, totalTimeout} } /** @@ -455,7 +456,8 @@ function chooseRandom (firstNumber, secondNumber) { if (firstNumber > secondNumber) { bigNumber = firstNumber; smallNumber = secondNumber; - } else if (firstNumber === secondNumber) { + } + else if (firstNumber === secondNumber) { bigNumber = 1; smallNumber = 0; } From 1b4e12c9800551fc5b0dcc887a1c43f2733fc333 Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-jy Date: Mon, 23 Oct 2023 18:29:00 -0700 Subject: [PATCH 13/23] edited typos and fix some logics --- lib/connection/connection_config.js | 2 +- lib/services/sf.js | 2 +- lib/util.js | 18 ++++++------------ test/unit/util_test.js | 6 +++--- 4 files changed, 11 insertions(+), 17 deletions(-) diff --git a/lib/connection/connection_config.js b/lib/connection/connection_config.js index 3adb3eaa2..d8280c007 100644 --- a/lib/connection/connection_config.js +++ b/lib/connection/connection_config.js @@ -982,7 +982,7 @@ function createParameters() }, { name: PARAM_RETRY_SF_MAX_SLEEP_TIME, - defaultValue: 16, + defaultValue: 108, validate: isNonNegativeNumber } ]; diff --git a/lib/services/sf.js b/lib/services/sf.js index ec3ee4564..6334b86b6 100644 --- a/lib/services/sf.js +++ b/lib/services/sf.js @@ -1217,7 +1217,7 @@ StateConnecting.prototype.continue = function () totalTimeout < maxLoginTimeout) { numRetries++; - const jitter = Util.jitteredSleepTime(numRetries, sleep, cap, totalTimeout, maxLoginTimeout); + const jitter = Util.jitteredSleepTime(numRetries, sleep, totalTimeout, maxLoginTimeout); sleep = jitter.sleep; totalTimeout = jitter.totalTimeout; setTimeout(sendRequest, sleep * 1000); diff --git a/lib/util.js b/lib/util.js index 284310f17..c0ec546e3 100644 --- a/lib/util.js +++ b/lib/util.js @@ -429,16 +429,10 @@ exports.nextSleepTime = function ( * @param {Number} maxSleepTime * @returns {JSON} return next sleep Time and totalTime. */ -exports.jitteredSleepTime = function (numofRetries, currentSleepTime, maxSleepTime, totalTimeout = 0, maxLoginTimeout = 300) { - const nextSleepTime = Math.min(maxSleepTime, 2 ** numofRetries); - let sleep = nextSleepTime + getJitter(currentSleepTime); - if (totalTimeout + sleep > maxLoginTimeout) { - sleep = maxLoginTimeout - totalTimeout; - totalTimeout = maxLoginTimeout; - } - else { - totalTimeout += sleep - } +exports.jitteredSleepTime = function (numofRetries, currentSleepTime, totalTimeout, maxLoginTimeout) { + const nextSleepTime = Math.min(maxLoginTimeout - totalTimeout, 4 * numofRetries); + const sleep = nextSleepTime + getJitter(currentSleepTime); + totalTimeout += sleep return {sleep, totalTimeout} } @@ -479,8 +473,8 @@ exports.chooseRandom = chooseRandom; */ function getJitter (curWaitTime) { const multiplicationFactor = chooseRandom(1, -1); - const jitterAmmount = 0.5 * curWaitTime * multiplicationFactor; - return jitterAmmount; + const jitterAmount = 0.5 * curWaitTime * multiplicationFactor; + return jitterAmount; } /** diff --git a/test/unit/util_test.js b/test/unit/util_test.js index eace486bd..7ef205aaa 100644 --- a/test/unit/util_test.js +++ b/test/unit/util_test.js @@ -607,15 +607,14 @@ describe('Util', function () }, ]; - const maxSleepTime = 16; const maxLoginTimeout = 300; - let currentSleepTime = 1; + let currentSleepTime = 4; let retryCount = 1; let totalTimeout = 1; for (const response of errorCodes) { assert.strictEqual(Util.isRetryableHttpError(response,true), true); - const result = Util.jitteredSleepTime(retryCount, currentSleepTime, maxSleepTime, totalTimeout, maxLoginTimeout); + const result = Util.jitteredSleepTime(retryCount, currentSleepTime, totalTimeout, maxLoginTimeout); const jitter = currentSleepTime / 2 const nextSleep = 2 ** retryCount; currentSleepTime = result.sleep; @@ -624,6 +623,7 @@ describe('Util', function () assert.ok(currentSleepTime <= nextSleep + jitter || currentSleepTime >= nextSleep - jitter) retryCount++; } + assert.strictEqual(retryCount, 8); assert.ok(totalTimeout <= maxLoginTimeout); }); From d6b82cae06d63554a571ff18a67299746cec534b Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-jy Date: Mon, 23 Oct 2023 21:22:02 -0700 Subject: [PATCH 14/23] added timeout in the ocsp login request test --- test/integration/ocsp_mock/testConnectionOcspMock.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/integration/ocsp_mock/testConnectionOcspMock.js b/test/integration/ocsp_mock/testConnectionOcspMock.js index 24e02830a..9a99c6533 100644 --- a/test/integration/ocsp_mock/testConnectionOcspMock.js +++ b/test/integration/ocsp_mock/testConnectionOcspMock.js @@ -23,6 +23,8 @@ function cloneConnOption(connOption) describe('Connection test with OCSP Mock', function () { + this.timeout(300000); + const valid = cloneConnOption(connOption.valid); console.log(connOption.valid); From ab065b01c105400158bfda90dd1915da61c17c20 Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-jy Date: Tue, 24 Oct 2023 11:02:35 -0700 Subject: [PATCH 15/23] refactored chooseRandom code, and edited the comment --- lib/util.js | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/lib/util.js b/lib/util.js index c0ec546e3..a59770271 100644 --- a/lib/util.js +++ b/lib/util.js @@ -445,31 +445,23 @@ exports.jitteredSleepTime = function (numofRetries, currentSleepTime, totalTimeo * @returns {Boolean} return a random number between two numbers. */ function chooseRandom (firstNumber, secondNumber) { - let bigNumber; - let smallNumber; - if (firstNumber > secondNumber) { - bigNumber = firstNumber; - smallNumber = secondNumber; - } - else if (firstNumber === secondNumber) { - bigNumber = 1; - smallNumber = 0; + let bigNumber = Math.max(firstNumber, secondNumber); + let smallNumber = Math.min(firstNumber, secondNumber); + + if (bigNumber === smallNumber) { + return Math.random() * bigNumber; } - else { - bigNumber = secondNumber; - smallNumber = firstNumber; - } - + return (Math.random() * (bigNumber - smallNumber)) + smallNumber; } exports.chooseRandom = chooseRandom; /** - * Check whether the request is the login-request or not. + * return the jitter value. * * @param {Number} curWaitTime - * @returns {Boolean} return jitter.. + * @returns {Boolean} return jitter. */ function getJitter (curWaitTime) { const multiplicationFactor = chooseRandom(1, -1); From 6dd5dcf9c875360e980993af53210e2e279d6529 Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-jy Date: Wed, 25 Oct 2023 08:26:46 -0700 Subject: [PATCH 16/23] refactored chooseRandom function --- lib/util.js | 4 ---- 1 file changed, 4 deletions(-) diff --git a/lib/util.js b/lib/util.js index a59770271..01b6b62e3 100644 --- a/lib/util.js +++ b/lib/util.js @@ -448,10 +448,6 @@ function chooseRandom (firstNumber, secondNumber) { let bigNumber = Math.max(firstNumber, secondNumber); let smallNumber = Math.min(firstNumber, secondNumber); - if (bigNumber === smallNumber) { - return Math.random() * bigNumber; - } - return (Math.random() * (bigNumber - smallNumber)) + smallNumber; } From dc33cefa3364a3a1e2b6d1c96dd86289c4a484b6 Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-jy Date: Thu, 26 Oct 2023 10:51:48 -0700 Subject: [PATCH 17/23] Changed the jitter rule, and comments --- lib/connection/connection_config.js | 2 +- lib/services/sf.js | 15 ++++++++++----- lib/util.js | 23 +++++++++++++---------- test/unit/util_test.js | 18 +++++++----------- 4 files changed, 31 insertions(+), 27 deletions(-) diff --git a/lib/connection/connection_config.js b/lib/connection/connection_config.js index d8280c007..3adb3eaa2 100644 --- a/lib/connection/connection_config.js +++ b/lib/connection/connection_config.js @@ -982,7 +982,7 @@ function createParameters() }, { name: PARAM_RETRY_SF_MAX_SLEEP_TIME, - defaultValue: 108, + defaultValue: 16, validate: isNonNegativeNumber } ]; diff --git a/lib/services/sf.js b/lib/services/sf.js index 6334b86b6..2aa63103c 100644 --- a/lib/services/sf.js +++ b/lib/services/sf.js @@ -1180,10 +1180,10 @@ StateConnecting.prototype.continue = function () Date.now() : 'FIXEDTIMESTAMP'; const maxLoginRetries = connectionConfig.getRetrySfMaxLoginRetries(); const maxLoginTimeout = connectionConfig.getLoginTimeout(); - let sleep = connectionConfig.getRetrySfStartingSleepTime(); - let totalTimeout = sleep; - const cap = connectionConfig.getRetrySfMaxSleepTime(); - Logger.getInstance().debug("Retry Max sleep time = " + cap); + const base = connectionConfig.getRetrySfStartingSleepTime(); + let sleep = base; + let totalTimeout = base; + Logger.getInstance().debug("Total loginTimeout is for the retries = " + maxLoginTimeout); const parent = this; const requestCallback = function (err, body) { @@ -1217,9 +1217,14 @@ StateConnecting.prototype.continue = function () totalTimeout < maxLoginTimeout) { numRetries++; - const jitter = Util.jitteredSleepTime(numRetries, sleep, totalTimeout, maxLoginTimeout); + const jitter = Util.jitteredSleepTime(numRetries, sleep, totalTimeout, base, maxLoginTimeout); sleep = jitter.sleep; totalTimeout = jitter.totalTimeout; + + if(sleep <= 0) { + Logger.getInstance().debug("Reached out to the Login Timeout"); + parent.snowflakeService.transitionToDisconnected(); + } setTimeout(sendRequest, sleep * 1000); return; } diff --git a/lib/util.js b/lib/util.js index 01b6b62e3..dc7921fd5 100644 --- a/lib/util.js +++ b/lib/util.js @@ -426,14 +426,14 @@ exports.nextSleepTime = function ( * * @param {Number} numofRetries * @param {Number} currentSleepTime - * @param {Number} maxSleepTime + * @param {Number} totalTimeout + * @param {Number} base + * @param {Number} maxLoginTimeout * @returns {JSON} return next sleep Time and totalTime. */ -exports.jitteredSleepTime = function (numofRetries, currentSleepTime, totalTimeout, maxLoginTimeout) { - const nextSleepTime = Math.min(maxLoginTimeout - totalTimeout, 4 * numofRetries); - const sleep = nextSleepTime + getJitter(currentSleepTime); +exports.jitteredSleepTime = function (numofRetries, currentSleepTime, totalTimeout, base, maxLoginTimeout) { + const sleep = Math.min((maxLoginTimeout - totalTimeout), getNextSleepTime(numofRetries, base, currentSleepTime) ); totalTimeout += sleep - return {sleep, totalTimeout} } @@ -455,14 +455,17 @@ exports.chooseRandom = chooseRandom; /** * return the jitter value. - * - * @param {Number} curWaitTime + * @param {Number} numofRetries + * @param {Number} base + * @param {Number} currentSleepTime * @returns {Boolean} return jitter. */ -function getJitter (curWaitTime) { +function getNextSleepTime (numofRetries, base, currentSleepTime) { const multiplicationFactor = chooseRandom(1, -1); - const jitterAmount = 0.5 * curWaitTime * multiplicationFactor; - return jitterAmount; + const nextSleep = (base * (2 ** (numofRetries - 1))); + const jitterAmount = 0.5 * currentSleepTime * multiplicationFactor; + + return nextSleep + jitterAmount; } /** diff --git a/test/unit/util_test.js b/test/unit/util_test.js index 7ef205aaa..8c9941aca 100644 --- a/test/unit/util_test.js +++ b/test/unit/util_test.js @@ -600,31 +600,27 @@ describe('Util', function () retry403: false, isRetryable: true, }, - { - statusCode: 525, - retry403: false, - isRetryable: true, - }, ]; const maxLoginTimeout = 300; - let currentSleepTime = 4; + const base = 4; + let currentSleepTime = base; let retryCount = 1; - let totalTimeout = 1; + let totalTimeout = base; for (const response of errorCodes) { + retryCount++; assert.strictEqual(Util.isRetryableHttpError(response,true), true); - const result = Util.jitteredSleepTime(retryCount, currentSleepTime, totalTimeout, maxLoginTimeout); + const result = Util.jitteredSleepTime(retryCount, currentSleepTime, totalTimeout, base, maxLoginTimeout); const jitter = currentSleepTime / 2 - const nextSleep = 2 ** retryCount; + const nextSleep = base * (2 ** (retryCount-1)); currentSleepTime = result.sleep; totalTimeout = result.totalTimeout; assert.ok(currentSleepTime <= nextSleep + jitter || currentSleepTime >= nextSleep - jitter) - retryCount++; } - assert.strictEqual(retryCount, 8); + assert.strictEqual(retryCount, 7); assert.ok(totalTimeout <= maxLoginTimeout); }); From c7b78410c21560f1ed031bf7446b661ddaa93692 Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-jy Date: Thu, 26 Oct 2023 22:16:29 -0700 Subject: [PATCH 18/23] refactored jitter --- lib/services/sf.js | 7 +++---- lib/util.js | 9 +++++---- test/unit/util_test.js | 9 ++++----- 3 files changed, 12 insertions(+), 13 deletions(-) diff --git a/lib/services/sf.js b/lib/services/sf.js index 2aa63103c..2ebfa0664 100644 --- a/lib/services/sf.js +++ b/lib/services/sf.js @@ -1180,9 +1180,8 @@ StateConnecting.prototype.continue = function () Date.now() : 'FIXEDTIMESTAMP'; const maxLoginRetries = connectionConfig.getRetrySfMaxLoginRetries(); const maxLoginTimeout = connectionConfig.getLoginTimeout(); - const base = connectionConfig.getRetrySfStartingSleepTime(); - let sleep = base; - let totalTimeout = base; + let sleep = connectionConfig.getRetrySfStartingSleepTime();; + let totalTimeout = sleep; Logger.getInstance().debug("Total loginTimeout is for the retries = " + maxLoginTimeout); const parent = this; const requestCallback = function (err, body) @@ -1217,7 +1216,7 @@ StateConnecting.prototype.continue = function () totalTimeout < maxLoginTimeout) { numRetries++; - const jitter = Util.jitteredSleepTime(numRetries, sleep, totalTimeout, base, maxLoginTimeout); + const jitter = Util.jitteredSleepTime(numRetries, sleep, totalTimeout, maxLoginTimeout); sleep = jitter.sleep; totalTimeout = jitter.totalTimeout; diff --git a/lib/util.js b/lib/util.js index dc7921fd5..857cd7fa0 100644 --- a/lib/util.js +++ b/lib/util.js @@ -431,9 +431,10 @@ exports.nextSleepTime = function ( * @param {Number} maxLoginTimeout * @returns {JSON} return next sleep Time and totalTime. */ -exports.jitteredSleepTime = function (numofRetries, currentSleepTime, totalTimeout, base, maxLoginTimeout) { - const sleep = Math.min((maxLoginTimeout - totalTimeout), getNextSleepTime(numofRetries, base, currentSleepTime) ); +exports.jitteredSleepTime = function (numofRetries, currentSleepTime, totalTimeout, maxLoginTimeout) { + const sleep = Math.min((maxLoginTimeout - totalTimeout), getNextSleepTime(numofRetries, currentSleepTime) ); totalTimeout += sleep + console.log(totalTimeout); return {sleep, totalTimeout} } @@ -460,9 +461,9 @@ exports.chooseRandom = chooseRandom; * @param {Number} currentSleepTime * @returns {Boolean} return jitter. */ -function getNextSleepTime (numofRetries, base, currentSleepTime) { +function getNextSleepTime (numofRetries, currentSleepTime) { const multiplicationFactor = chooseRandom(1, -1); - const nextSleep = (base * (2 ** (numofRetries - 1))); + const nextSleep = (2 ** (numofRetries)); const jitterAmount = 0.5 * currentSleepTime * multiplicationFactor; return nextSleep + jitterAmount; diff --git a/test/unit/util_test.js b/test/unit/util_test.js index 8c9941aca..f44f85ad5 100644 --- a/test/unit/util_test.js +++ b/test/unit/util_test.js @@ -603,17 +603,16 @@ describe('Util', function () ]; const maxLoginTimeout = 300; - const base = 4; - let currentSleepTime = base; + let currentSleepTime = 4; let retryCount = 1; - let totalTimeout = base; + let totalTimeout = currentSleepTime; for (const response of errorCodes) { retryCount++; assert.strictEqual(Util.isRetryableHttpError(response,true), true); - const result = Util.jitteredSleepTime(retryCount, currentSleepTime, totalTimeout, base, maxLoginTimeout); + const result = Util.jitteredSleepTime(retryCount, currentSleepTime, totalTimeout, maxLoginTimeout); const jitter = currentSleepTime / 2 - const nextSleep = base * (2 ** (retryCount-1)); + const nextSleep = 2 ** retryCount; currentSleepTime = result.sleep; totalTimeout = result.totalTimeout; From 717314dea1cbbd916fcbaa8517f8925cc70490db Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-jy Date: Fri, 27 Oct 2023 09:02:32 -0700 Subject: [PATCH 19/23] removed unnecessary comments --- lib/util.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/util.js b/lib/util.js index 857cd7fa0..72d07b433 100644 --- a/lib/util.js +++ b/lib/util.js @@ -427,7 +427,6 @@ exports.nextSleepTime = function ( * @param {Number} numofRetries * @param {Number} currentSleepTime * @param {Number} totalTimeout - * @param {Number} base * @param {Number} maxLoginTimeout * @returns {JSON} return next sleep Time and totalTime. */ @@ -457,7 +456,6 @@ exports.chooseRandom = chooseRandom; /** * return the jitter value. * @param {Number} numofRetries - * @param {Number} base * @param {Number} currentSleepTime * @returns {Boolean} return jitter. */ From 06887426f7637d883c692de9f58af2cf9c62f6d1 Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-jy Date: Fri, 27 Oct 2023 09:26:35 -0700 Subject: [PATCH 20/23] add getNextSleep testing --- lib/util.js | 2 +- test/unit/util_test.js | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/lib/util.js b/lib/util.js index 72d07b433..22f27003c 100644 --- a/lib/util.js +++ b/lib/util.js @@ -466,7 +466,7 @@ function getNextSleepTime (numofRetries, currentSleepTime) { return nextSleep + jitterAmount; } - +exports.getNextSleepTime = getNextSleepTime; /** * Check whether the request is the login-request or not. * diff --git a/test/unit/util_test.js b/test/unit/util_test.js index f44f85ad5..8a029c411 100644 --- a/test/unit/util_test.js +++ b/test/unit/util_test.js @@ -623,6 +623,20 @@ describe('Util', function () assert.ok(totalTimeout <= maxLoginTimeout); }); + it(" test Exonential Jitter Back Off", function () { + const numofRetries = 10; + let sleep = 1; + let retries = []; + for(let i = 0; i < numofRetries; i++) { + retries.push(Util.getNextSleepTime(i + 1, sleep)); + sleep = retries[i]; + } + + for(let i = 0; i< numofRetries -1; i++) { + assert.ok(retries[i] < retries[i + 1]); + } + }) + it("Util.chooseRandom Test", function () { const positiveInteger = Util.chooseRandom(1, 5); const negativeInteger = Util.chooseRandom(-1, -5); From 7643c343be611cf2ccd0c5faa3f89b0ca58780bf Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-jy Date: Fri, 27 Oct 2023 09:53:11 -0700 Subject: [PATCH 21/23] refactored loginTimeout in the connection config --- lib/connection/connection_config.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/connection/connection_config.js b/lib/connection/connection_config.js index 3adb3eaa2..d207a87fd 100644 --- a/lib/connection/connection_config.js +++ b/lib/connection/connection_config.js @@ -489,7 +489,7 @@ function ConnectionConfig(options, validateCredentials, qaMode, clientInfo) Errors.checkArgumentValid(Util.isNumber(options.loginTimeout), ErrorCodes.ERR_CONN_CREATE_INVALID_MAX_LOGIN_TIMEOUT); - loginTimeout = options.loginTimeout > 300 ? options.loginTimeout : 300; + loginTimeout = Math.max(loginTimeout,options.loginTimeout); } if (validateDefaultParameters) From 2d88fd0cf87a31581a7e5fd8de8b8b408a14c874 Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-jy Date: Fri, 27 Oct 2023 10:29:41 -0700 Subject: [PATCH 22/23] removed console.log --- lib/util.js | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/util.js b/lib/util.js index 22f27003c..4deb72c7b 100644 --- a/lib/util.js +++ b/lib/util.js @@ -433,7 +433,6 @@ exports.nextSleepTime = function ( exports.jitteredSleepTime = function (numofRetries, currentSleepTime, totalTimeout, maxLoginTimeout) { const sleep = Math.min((maxLoginTimeout - totalTimeout), getNextSleepTime(numofRetries, currentSleepTime) ); totalTimeout += sleep - console.log(totalTimeout); return {sleep, totalTimeout} } From 13b05b61f95d839ce16daa73c3772f5b80cdcc27 Mon Sep 17 00:00:00 2001 From: sfc-gh-ext-simba-jy Date: Fri, 27 Oct 2023 10:46:23 -0700 Subject: [PATCH 23/23] fixed typo --- test/unit/util_test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/util_test.js b/test/unit/util_test.js index 8a029c411..8b90a4683 100644 --- a/test/unit/util_test.js +++ b/test/unit/util_test.js @@ -623,7 +623,7 @@ describe('Util', function () assert.ok(totalTimeout <= maxLoginTimeout); }); - it(" test Exonential Jitter Back Off", function () { + it("test exponential jitter back off", function () { const numofRetries = 10; let sleep = 1; let retries = [];