Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SNOW-859635: Retry Strategy #671

Merged
merged 29 commits into from
Oct 27, 2023
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
1e5a951
Retry Strategy
sfc-gh-ext-simba-jy Oct 19, 2023
66fe71c
changed the Jitter to the original
sfc-gh-ext-simba-jy Oct 19, 2023
e3f6fb5
changed jitter
sfc-gh-ext-simba-jy Oct 19, 2023
a233abd
switched the point to add headers and make a function
sfc-gh-ext-simba-jy Oct 19, 2023
829edee
Merge branch 'master' into retrystrategy
sfc-gh-ext-simba-jy Oct 19, 2023
c2299fc
add testing cases
sfc-gh-ext-simba-jy Oct 20, 2023
11bc6a5
added testing for jitteredSleepTime and add comments
sfc-gh-ext-simba-jy Oct 20, 2023
43e8442
Removed spaces and formatted code
sfc-gh-ext-simba-jy Oct 20, 2023
e50573e
refactor isLoginRequest test
sfc-gh-ext-simba-jy Oct 20, 2023
15d7523
added max login time out, and refactored jittered time testing case
sfc-gh-ext-simba-jy Oct 23, 2023
aba19ae
Merge branch 'master' into retrystrategy
sfc-gh-ext-simba-jy Oct 23, 2023
9c7a7e4
edited the loginTimeout options
sfc-gh-ext-simba-jy Oct 23, 2023
1039fb8
add trailing comma in the default param
sfc-gh-ext-simba-jy Oct 23, 2023
3512bbc
fixed the format of jitter function
sfc-gh-ext-simba-jy Oct 23, 2023
1b4e12c
edited typos and fix some logics
sfc-gh-ext-simba-jy Oct 24, 2023
d6b82ca
added timeout in the ocsp login request test
sfc-gh-ext-simba-jy Oct 24, 2023
6f5df98
Merge branch 'master' into retrystrategy
sfc-gh-ext-simba-jy Oct 24, 2023
ab065b0
refactored chooseRandom code, and edited the comment
sfc-gh-ext-simba-jy Oct 24, 2023
0239e8f
Merge branch 'retrystrategy' of https://github.com/snowflakedb/snowfl…
sfc-gh-ext-simba-jy Oct 24, 2023
6dd5dcf
refactored chooseRandom function
sfc-gh-ext-simba-jy Oct 25, 2023
dc33cef
Changed the jitter rule, and comments
sfc-gh-ext-simba-jy Oct 26, 2023
9741f68
Merge branch 'master' into retrystrategy
sfc-gh-ext-simba-jy Oct 26, 2023
c7b7841
refactored jitter
sfc-gh-ext-simba-jy Oct 27, 2023
422854b
Merge branch 'retrystrategy' of https://github.com/snowflakedb/snowfl…
sfc-gh-ext-simba-jy Oct 27, 2023
717314d
removed unnecessary comments
sfc-gh-ext-simba-jy Oct 27, 2023
0688742
add getNextSleep testing
sfc-gh-ext-simba-jy Oct 27, 2023
7643c34
refactored loginTimeout in the connection config
sfc-gh-ext-simba-jy Oct 27, 2023
2d88fd0
removed console.log
sfc-gh-ext-simba-jy Oct 27, 2023
13b05b6
fixed typo
sfc-gh-ext-simba-jy Oct 27, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 21 additions & 3 deletions lib/connection/connection_config.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ const DEFAULT_PARAMS =
'forceStageBindError',
'includeRetryReason',
'disableQueryContextCache',
'loginTimeout',
];

function consolidateHostAndAccount(options)
Expand Down Expand Up @@ -483,6 +484,14 @@ function ConnectionConfig(options, validateCredentials, qaMode, clientInfo)
disableQueryContextCache = options.disableQueryContextCache;
}

let loginTimeout = 300;
if (Util.exists(options.loginTimeout)) {
Errors.checkArgumentValid(Util.isNumber(options.loginTimeout),
ErrorCodes.ERR_CONN_CREATE_INVALID_MAX_LOGIN_TIMEOUT);

loginTimeout = options.loginTimeout > 300 ? options.loginTimeout : 300;
sfc-gh-ext-simba-jy marked this conversation as resolved.
Show resolved Hide resolved
}

if (validateDefaultParameters)
{
for (const [key] of Object.entries(options))
Expand Down Expand Up @@ -793,6 +802,15 @@ function ConnectionConfig(options, validateCredentials, qaMode, clientInfo)
return clientConfigFile;
};

/**
* Returns the max login timeout
*
* @returns {Number}
*/
this.getLoginTimeout = function () {
return loginTimeout;
}

// save config options
this.username = options.username;
this.password = options.password;
Expand Down Expand Up @@ -948,7 +966,7 @@ function createParameters()
},
{
name: PARAM_RETRY_SF_MAX_LOGIN_RETRIES,
defaultValue: 5,
defaultValue: 7,
sfc-gh-ext-simba-jy marked this conversation as resolved.
Show resolved Hide resolved
external: true,
validate: isNonNegativeInteger
},
Expand All @@ -959,12 +977,12 @@ function createParameters()
},
{
name: PARAM_RETRY_SF_STARTING_SLEEP_TIME,
defaultValue: 0.25,
defaultValue: 4,
validate: isNonNegativeNumber
},
{
name: PARAM_RETRY_SF_MAX_SLEEP_TIME,
defaultValue: 16,
defaultValue: 108,
validate: isNonNegativeNumber
}
];
Expand Down
1 change: 1 addition & 0 deletions lib/constants/error_messages.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 loginTimeout value. The specified value must be a number.';

// 405001
exports[405001] = 'Invalid callback. The specified value must be a function.';
Expand Down
1 change: 1 addition & 0 deletions lib/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -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_LOGIN_TIMEOUT = 404044;

// 405001
codes.ERR_CONN_CONNECT_INVALID_CALLBACK = 405001;
Expand Down
16 changes: 14 additions & 2 deletions lib/services/sf.js
Original file line number Diff line number Diff line change
Expand Up @@ -782,6 +782,13 @@ function StateAbstract(options)
requestOptions.headers =
Util.apply(this.getDefaultReqHeaders(), requestOptions.headers || {});

if (Util.isLoginRequest(requestOptions.url)) {
sfc-gh-dheyman marked this conversation as resolved.
Show resolved Hide resolved
Util.apply(requestOptions.headers, {
"CLIENT_APP_VERSION": requestOptions.json.data.CLIENT_APP_VERSION,
sfc-gh-ext-simba-lf marked this conversation as resolved.
Show resolved Hide resolved
"CLIENT_APP_ID": requestOptions.json.data.CLIENT_APP_ID,
})
}

// augment the options with the absolute url
requestOptions.absoluteUrl = this.buildFullUrl(requestOptions.url);
};
Expand Down Expand Up @@ -1172,7 +1179,9 @@ StateConnecting.prototype.continue = function ()
const startTime = connectionConfig.accessUrl.startsWith('https://') ?
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 parent = this;
Expand Down Expand Up @@ -1204,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.nextSleepTime(1, cap, sleep);
const jitter = Util.jitteredSleepTime(numRetries, sleep, totalTimeout, maxLoginTimeout);
sleep = jitter.sleep;
totalTimeout = jitter.totalTimeout;
setTimeout(sendRequest, sleep * 1000);
return;
}
Expand Down
70 changes: 69 additions & 1 deletion lib/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -405,7 +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
Expand All @@ -420,6 +420,74 @@ 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 {JSON} return next sleep Time and totalTime.
*/
exports.jitteredSleepTime = function (numofRetries, currentSleepTime, totalTimeout, maxLoginTimeout) {
const nextSleepTime = Math.min(maxLoginTimeout - totalTimeout, 4 * numofRetries);
sfc-gh-ext-simba-jy marked this conversation as resolved.
Show resolved Hide resolved
const sleep = nextSleepTime + getJitter(currentSleepTime);
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) {
sfc-gh-ext-simba-jy marked this conversation as resolved.
Show resolved Hide resolved
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.
sfc-gh-ext-simba-jy marked this conversation as resolved.
Show resolved Hide resolved
*
* @param {Number} curWaitTime
* @returns {Boolean} return jitter..
*/
function getJitter (curWaitTime) {
const multiplicationFactor = chooseRandom(1, -1);
const jitterAmount = 0.5 * curWaitTime * multiplicationFactor;
return jitterAmount;
}

/**
* 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
*
Expand Down
2 changes: 2 additions & 0 deletions test/integration/ocsp_mock/testConnectionOcspMock.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
31 changes: 29 additions & 2 deletions test/unit/connection/connection_config_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
loginTimeout: 'invalud'
},
errorCode: ErrorCodes.ERR_CONN_CREATE_INVALID_MAX_LOGIN_TIMEOUT
},
];

var createNegativeITCallback = function (testCase)
Expand Down Expand Up @@ -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',
}
},
{
Expand All @@ -914,6 +924,23 @@ describe('ConnectionConfig: basic', function ()
clientConfigFile: 'easy_logging_config.json'
}
},
{
name: 'login time out',
input:
{
account: 'account',
username: 'username',
password: 'password',
loginTimeout: 1234,
},
options:
{
accessUrl: 'https://account.snowflakecomputing.com',
username: 'username',
password: 'password',
account: 'account',
}
},
];

var createItCallback = function (testCase)
Expand Down
33 changes: 25 additions & 8 deletions test/unit/mock/mock_http_client.js
Original file line number Diff line number Diff line change
Expand Up @@ -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:
{
Expand Down Expand Up @@ -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:
{
Expand Down Expand Up @@ -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:
{
Expand Down Expand Up @@ -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:
{
Expand Down Expand Up @@ -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:
{
Expand Down Expand Up @@ -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:
{
Expand Down Expand Up @@ -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:
{
Expand Down Expand Up @@ -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:
{
Expand Down
Loading