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

[core-amqp] Refactor time units to milliseconds #4401

Merged
merged 14 commits into from
Jul 31, 2019
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
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
10 changes: 6 additions & 4 deletions sdk/core/core-amqp/src/ConnectionContextBase.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,11 +117,11 @@ export interface CreateConnectionContextBaseParameters {
*/
isEntityPathRequired?: boolean;
/**
* @property {number} [operationTimeoutInSeconds] - The duration in which the promise should
* @property {number} [operationTimeoutInMs] - The duration in which the promise should
* complete (resolve/reject). If it is not completed, then the Promise will be rejected after
* timeout occurs. Default: `60 seconds`.
* timeout occurs. Default: `60000 milliseconds`.
*/
operationTimeoutInSeconds?: number;
operationTimeoutInMs?: number;
}

export module ConnectionContextBase {
Expand Down Expand Up @@ -156,7 +156,9 @@ export module ConnectionContextBase {
platform: `(${os.arch()}-${os.type()}-${os.release()})`,
framework: `Node/${process.version}`
},
operationTimeoutInSeconds: parameters.operationTimeoutInSeconds
operationTimeoutInSeconds: parameters.operationTimeoutInMs
? parameters.operationTimeoutInMs / 1000
: undefined
};

if (
Expand Down
26 changes: 13 additions & 13 deletions sdk/core/core-amqp/src/requestResponseLink.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,20 +31,20 @@ export interface SendRequestOptions {
*/
abortSignal?: AbortSignalLike;
/**
* @property {number} [timeoutInSeconds] Max time to wait for the operation to complete.
* Default: `10 seconds`.
* @property {number} [timeoutInMs] Max time to wait for the operation to complete.
* Default: `10000 milliseconds`.
*/
timeoutInSeconds?: number;
timeoutInMs?: number;
/**
* @property {number} [maxRetries] Number of times the operation needs to be retried in case
* of error. Default: 3.
*/
maxRetries?: number;
/**
* @property {number} [delayInSeconds] Amount of time to wait in seconds before making the
* next attempt. Default: 15.
* @property {number} [delayInMs] Amount of time to wait in milliseconds before making the
* next attempt. Default: `15000 milliseconds`.
*/
delayInSeconds?: number;
delayInMs?: number;
/**
* @property {string} [requestName] Name of the request being performed.
*/
Expand Down Expand Up @@ -86,9 +86,9 @@ export class RequestResponseLink implements ReqResLink {

/**
* Sends the given request message and returns the received response. If the operation is not
* completed in the provided timeout in seconds `default: 10`, then the request will be retried
* linearly for the provided number of times `default: 3` with the provided delay in seconds
* `default: 15` between each attempt.
* completed in the provided timeout in milliseconds `default: 10000`, then the request will be retried
* linearly for the provided number of times `default: 3` with the provided delay in milliseconds
* `default: 15000` between each attempt.
*
* @param {Message} request The AMQP (request) message.
* @param {SendRequestOptions} [options] Options that can be provided while sending a request.
Expand All @@ -97,8 +97,8 @@ export class RequestResponseLink implements ReqResLink {
sendRequest(request: AmqpMessage, options?: SendRequestOptions): Promise<AmqpMessage> {
if (!options) options = {};

if (!options.timeoutInSeconds) {
options.timeoutInSeconds = 10;
if (!options.timeoutInMs) {
options.timeoutInMs = 10000;
}

let count: number = 0;
Expand Down Expand Up @@ -240,7 +240,7 @@ export class RequestResponseLink implements ReqResLink {
};

this.receiver.on(ReceiverEvents.message, messageCallback);
waitTimer = setTimeout(actionAfterTimeout, options!.timeoutInSeconds! * 1000);
waitTimer = setTimeout(actionAfterTimeout, options!.timeoutInMs!);
log.reqres(
"[%s] %s request sent: %O",
this.connection.id,
Expand All @@ -256,7 +256,7 @@ export class RequestResponseLink implements ReqResLink {
request.to && request.to === Constants.cbsEndpoint
? RetryOperationType.cbsAuth
: RetryOperationType.management,
delayInSeconds: options.delayInSeconds,
delayInMs: options.delayInMs,
maxRetries: options.maxRetries
};
return retry<AmqpMessage>(config);
Expand Down
20 changes: 10 additions & 10 deletions sdk/core/core-amqp/src/retry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { delay, isNode } from "./util/utils";
import * as log from "./log";
import {
defaultMaxRetries,
defaultDelayBetweenOperationRetriesInSeconds,
defaultDelayBetweenOperationRetriesInMs,
defaultMaxDelayForExponentialRetryInMs,
defaultMinDelayForExponentialRetryInMs
} from "./util/constants";
Expand Down Expand Up @@ -79,12 +79,12 @@ export interface RetryConfig<T> {
*/
maxRetries?: number;
/**
* @property {number} [delayInSeconds] Amount of time to wait in seconds before making the
* next attempt. Default: 30.
* @property {number} [delayInMs] Amount of time to wait in milliseconds before making the
* next attempt. Default: `30000 milliseconds`.
* When `retryPolicy` option is set to `ExponentialRetryPolicy`, \
* this is used to compute the exponentially increasing delays between retries.
*/
delayInSeconds?: number;
delayInMs?: number;
/**
* @property {string} connectionHost The host "<yournamespace>.servicebus.windows.net".
* Used to check network connectivity.
Expand Down Expand Up @@ -160,8 +160,8 @@ export async function retry<T>(config: RetryConfig<T>): Promise<T> {
if (config.maxRetries == undefined || config.maxRetries < 0) {
config.maxRetries = defaultMaxRetries;
}
if (config.delayInSeconds == undefined || config.delayInSeconds < 0) {
config.delayInSeconds = defaultDelayBetweenOperationRetriesInSeconds;
if (config.delayInMs == undefined || config.delayInMs < 0) {
config.delayInMs = defaultDelayBetweenOperationRetriesInMs;
}
if (config.maxExponentialRetryDelayInMs == undefined || config.maxExponentialRetryDelayInMs < 0) {
config.maxExponentialRetryDelayInMs = defaultMaxDelayForExponentialRetryInMs;
Expand Down Expand Up @@ -213,12 +213,12 @@ export async function retry<T>(config: RetryConfig<T>): Promise<T> {
i,
err
);
let targetDelayInMs = config.delayInSeconds;
let targetDelayInMs = config.delayInMs;
if (config.retryPolicy === RetryPolicy.ExponentialRetryPolicy) {
let incrementDelta = Math.pow(2, i) - 1;
const boundedRandDelta =
config.delayInSeconds * 0.8 +
Math.floor(Math.random() * (config.delayInSeconds * 1.2 - config.delayInSeconds * 0.8));
config.delayInMs * 0.8 +
Math.floor(Math.random() * (config.delayInMs * 1.2 - config.delayInMs * 0.8));
incrementDelta *= boundedRandDelta;

targetDelayInMs = Math.min(
Expand All @@ -229,7 +229,7 @@ export async function retry<T>(config: RetryConfig<T>): Promise<T> {

if (lastError && lastError.retryable) {
log.error(
"[%s] Sleeping for %d seconds for '%s'.",
"[%s] Sleeping for %d milliseconds for '%s'.",
config.connectionId,
targetDelayInMs / 1000,
config.operationType
Expand Down
10 changes: 5 additions & 5 deletions sdk/core/core-amqp/src/util/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ export const receiverError = "receiver_error";
export const senderError = "sender_error";
export const sessionError = "session_error";
export const connectionError = "connection_error";
export const defaultOperationTimeoutInSeconds = 60;
export const defaultOperationTimeoutInMs = 60000;
export const managementRequestKey = "managementRequest";
export const negotiateCbsKey = "negotiateCbs";
export const negotiateClaim = "negotiateClaim";
Expand All @@ -71,13 +71,13 @@ export const maxDurationValue = 922337203685477;
export const minDurationValue = -922337203685477;
// https://github.com/Azure/azure-amqp/blob/master/Microsoft.Azure.Amqp/Amqp/AmqpConstants.cs#L47
export const maxAbsoluteExpiryTime = new Date("9999-12-31T07:59:59.000Z").getTime();
export const aadTokenValidityMarginSeconds = 5;
export const aadTokenValidityMarginInMs = 5000;
export const connectionReconnectDelay = 300;
export const defaultMaxRetries = 3;
export const defaultMaxRetriesForConnection = 150;
export const defaultDelayBetweenOperationRetriesInSeconds = 30;
export const defaultMaxDelayForExponentialRetryInMs = 1000 * 90;
export const defaultMinDelayForExponentialRetryInMs = 1000 * 3;
export const defaultDelayBetweenOperationRetriesInMs = 30000;
export const defaultMaxDelayForExponentialRetryInMs = 90000;
export const defaultMinDelayForExponentialRetryInMs = 3000;
export const receiverSettleMode = "receiver-settle-mode";
export const dispositionStatus = "disposition-status";
export const fromSequenceNumber = "from-sequence-number";
Expand Down
8 changes: 4 additions & 4 deletions sdk/core/core-amqp/test/requestResponse.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,8 @@ describe("RequestResponseLink", function() {
});
}, 4000);
const response = await link.sendRequest(request, {
delayInSeconds: 1,
timeoutInSeconds: 5
delayInMs: 1000,
timeoutInMs: 5000
});
assert.equal(response.correlation_id, messageId);
});
Expand Down Expand Up @@ -240,8 +240,8 @@ describe("RequestResponseLink", function() {
const signal = controller.signal;
setTimeout(controller.abort.bind(controller), 100);
await link.sendRequest(request, {
delayInSeconds: 1,
timeoutInSeconds: 5,
delayInMs: 1000,
timeoutInMs: 5000,
abortSignal: signal // cancel between request attempts
});
throw new Error(`Test failure`);
Expand Down
22 changes: 11 additions & 11 deletions sdk/core/core-amqp/test/retry.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ dotenv.config();
},
connectionId: "connection-1",
operationType: RetryOperationType.cbsAuth,
delayInSeconds: 15,
delayInMs: 15000,
minExponentialRetryDelayInMs: 0,
retryPolicy: retryPolicy
};
Expand Down Expand Up @@ -66,7 +66,7 @@ dotenv.config();
},
connectionId: "connection-1",
operationType: RetryOperationType.management,
delayInSeconds: 15,
delayInMs: 15000,
minExponentialRetryDelayInMs: 0,
retryPolicy: retryPolicy
};
Expand Down Expand Up @@ -101,7 +101,7 @@ dotenv.config();
connectionId: "connection-1",
operationType: RetryOperationType.receiverLink,
maxRetries: 2,
delayInSeconds: 0.5,
delayInMs: 500,
minExponentialRetryDelayInMs: 0,
retryPolicy: retryPolicy
};
Expand Down Expand Up @@ -140,7 +140,7 @@ dotenv.config();
connectionId: "connection-1",
operationType: RetryOperationType.senderLink,
maxRetries: 2,
delayInSeconds: 0.5,
delayInMs: 500,
minExponentialRetryDelayInMs: 0,
retryPolicy: retryPolicy
};
Expand Down Expand Up @@ -180,7 +180,7 @@ dotenv.config();
connectionId: "connection-1",
operationType: RetryOperationType.sendMessage,
maxRetries: 2,
delayInSeconds: 0.5,
delayInMs: 500,
minExponentialRetryDelayInMs: 0,
retryPolicy: retryPolicy
};
Expand All @@ -207,7 +207,7 @@ dotenv.config();
connectionId: "connection-1",
operationType: RetryOperationType.session,
maxRetries: 4,
delayInSeconds: 0.5,
delayInMs: 500,
minExponentialRetryDelayInMs: 0,
retryPolicy: retryPolicy
};
Expand Down Expand Up @@ -236,7 +236,7 @@ dotenv.config();
},
connectionId: "connection-1",
operationType: RetryOperationType.cbsAuth,
delayInSeconds: 0.5,
delayInMs: 500,
minExponentialRetryDelayInMs: 0,
retryPolicy: retryPolicy
};
Expand Down Expand Up @@ -265,7 +265,7 @@ dotenv.config();
connectionId: "connection-1",
operationType: RetryOperationType.management,
maxRetries: Infinity,
delayInSeconds: 0.5,
delayInMs: 500,
minExponentialRetryDelayInMs: 0,
retryPolicy: retryPolicy
};
Expand Down Expand Up @@ -300,7 +300,7 @@ dotenv.config();
connectionId: "connection-1",
operationType: RetryOperationType.receiverLink,
maxRetries: Infinity,
delayInSeconds: 0.5,
delayInMs: 500,
minExponentialRetryDelayInMs: 0,
retryPolicy: retryPolicy
};
Expand Down Expand Up @@ -339,7 +339,7 @@ dotenv.config();
connectionId: "connection-1",
operationType: RetryOperationType.senderLink,
maxRetries: Infinity,
delayInSeconds: 0.5,
delayInMs: 500,
minExponentialRetryDelayInMs: 0,
retryPolicy: retryPolicy
};
Expand Down Expand Up @@ -379,7 +379,7 @@ dotenv.config();
connectionId: "connection-1",
operationType: RetryOperationType.sendMessage,
maxRetries: Constants.defaultMaxRetriesForConnection,
delayInSeconds: 0.001,
delayInMs: 1,
minExponentialRetryDelayInMs: 0,
retryPolicy: retryPolicy
};
Expand Down