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

fix(util-retry): use different retry token instances per request #4755

Merged
merged 7 commits into from
May 25, 2023
6 changes: 3 additions & 3 deletions packages/middleware-retry/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { SdkError } from "@aws-sdk/types";
* Determines whether an error is retryable based on the number of retries
* already attempted, the HTTP status code, and the error received (if any).
*
* @param error The error encountered.
* @param error - The error encountered.
*/
export interface RetryDecider {
(error: SdkError): boolean;
Expand All @@ -13,8 +13,8 @@ export interface RetryDecider {
/**
* Determines the number of milliseconds to wait before retrying an action.
*
* @param delayBase The base delay (in milliseconds).
* @param attempts The number of times the action has already been tried.
* @param delayBase - The base delay (in milliseconds).
* @param attempts - The number of times the action has already been tried.
*/
export interface DelayDecider {
(delayBase: number, attempts: number): number;
Expand Down
21 changes: 2 additions & 19 deletions packages/types/src/retry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,26 +95,9 @@ export interface RetryToken {
*/
export interface StandardRetryToken extends RetryToken {
/**
* @returns wheather token has remaining tokens.
* @returns the cost of the last retry attempt.
*/
hasRetryTokens(errorType: RetryErrorType): boolean;

/**
* @returns the number of available tokens.
*/
getRetryTokenCount(errorInfo: RetryErrorInfo): number;

/**
* @returns the cost of the last retry attemp.
*/
getLastRetryCost(): number | undefined;

/**
* Releases a number of tokens.
*
* @param amount - of tokens to release.
*/
releaseRetryTokens(amount?: number): void;
getRetryCost(): number | undefined;
}

/**
Expand Down
7 changes: 2 additions & 5 deletions packages/util-retry/src/AdaptiveRetryStrategy.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { RetryErrorInfo, RetryErrorType, StandardRetryToken } from "@aws-sdk/types";
import { RetryErrorInfo, StandardRetryToken } from "@aws-sdk/types";

import { AdaptiveRetryStrategy } from "./AdaptiveRetryStrategy";
import { RETRY_MODES } from "./config";
Expand All @@ -17,10 +17,7 @@ describe(AdaptiveRetryStrategy.name, () => {
updateClientSendingRate: jest.fn(),
};
const mockRetryToken: StandardRetryToken = {
hasRetryTokens: (errorType: RetryErrorType) => true,
getLastRetryCost: () => 1,
getRetryTokenCount: (errorInfo: RetryErrorInfo) => 1,
releaseRetryTokens: (amount: number) => {},
getRetryCost: () => 1,
getRetryCount: () => 1,
getRetryDelay: () => 1,
};
Expand Down
4 changes: 2 additions & 2 deletions packages/util-retry/src/ConfiguredRetryStrategy.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ describe(ConfiguredRetryStrategy.name, () => {
errorType: "TRANSIENT",
});

expect(retryToken.getRetryCount()).toBe(4);
expect(retryToken.getRetryDelay()).toBe(4000);
expect(retryToken.getRetryCount()).toBe(5);
expect(retryToken.getRetryDelay()).toBe(5000);
});
});
57 changes: 15 additions & 42 deletions packages/util-retry/src/StandardRetryStrategy.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { RetryErrorInfo, RetryErrorType } from "@aws-sdk/types";

import { RETRY_MODES } from "./config";
import { DEFAULT_RETRY_DELAY_BASE, INITIAL_RETRY_TOKENS } from "./constants";
import { getDefaultRetryToken } from "./defaultRetryToken";
import { createDefaultRetryToken } from "./defaultRetryToken";
import { StandardRetryStrategy } from "./StandardRetryStrategy";

jest.mock("./defaultRetryToken");
Expand All @@ -18,7 +18,7 @@ describe(StandardRetryStrategy.name, () => {
const errorInfo = { errorType: "TRANSIENT" } as RetryErrorInfo;

beforeEach(() => {
(getDefaultRetryToken as jest.Mock).mockReturnValue(mockRetryToken);
(createDefaultRetryToken as jest.Mock).mockReturnValue(mockRetryToken);
});

afterEach(() => {
Expand All @@ -37,48 +37,40 @@ describe(StandardRetryStrategy.name, () => {
expect(retryStrategy.mode).toStrictEqual(RETRY_MODES.STANDARD);
});

describe("retryToken init", () => {
it("sets retryToken", () => {
const retryStrategy = new StandardRetryStrategy(() => Promise.resolve(maxAttempts));
expect(retryStrategy["retryToken"]).toBe(getDefaultRetryToken(INITIAL_RETRY_TOKENS, DEFAULT_RETRY_DELAY_BASE));
});
});

describe("acquireInitialRetryToken", () => {
it("returns default retryToken", async () => {
const retryStrategy = new StandardRetryStrategy(() => Promise.resolve(maxAttempts));
const retryToken = await retryStrategy.acquireInitialRetryToken(retryTokenScope);
expect(retryToken).toEqual(getDefaultRetryToken(INITIAL_RETRY_TOKENS, DEFAULT_RETRY_DELAY_BASE));
expect(retryToken).toEqual(
createDefaultRetryToken({
retryDelay: DEFAULT_RETRY_DELAY_BASE,
retryCount: 0,
})
);
});
});

describe("refreshRetryTokenForRetry", () => {
it("refreshes the token", async () => {
const getRetryTokenCount = jest.fn().mockReturnValue(1);
const getRetryCount = jest.fn().mockReturnValue(0);
const hasRetryTokens = jest.fn().mockReturnValue(true);
const mockRetryToken = {
getRetryCount,
getRetryTokenCount,
hasRetryTokens,
};
(getDefaultRetryToken as jest.Mock).mockReturnValue(mockRetryToken);
(createDefaultRetryToken as jest.Mock).mockReturnValue(mockRetryToken);
const retryStrategy = new StandardRetryStrategy(() => Promise.resolve(maxAttempts));
const token = await retryStrategy.acquireInitialRetryToken(retryTokenScope);
const refreshedToken = await retryStrategy.refreshRetryTokenForRetry(token, errorInfo);
expect(getRetryTokenCount).toHaveBeenCalledTimes(1);
expect(getRetryTokenCount).toHaveBeenCalledWith(errorInfo);
expect(getRetryCount).toHaveBeenCalledTimes(1);
expect(hasRetryTokens).toHaveBeenCalledTimes(1);
expect(hasRetryTokens).toHaveBeenCalledWith(errorInfo.errorType);
await retryStrategy.refreshRetryTokenForRetry(token, errorInfo);
expect(getRetryCount).toHaveBeenCalledTimes(3);
});

it("throws when attempts exceeds maxAttempts", async () => {
const mockRetryToken = {
getRetryCount: () => 2,
getRetryTokenCount: (errorInfo: any) => 1,
};
(getDefaultRetryToken as jest.Mock).mockReturnValue(mockRetryToken);
(createDefaultRetryToken as jest.Mock).mockReturnValue(mockRetryToken);
const retryStrategy = new StandardRetryStrategy(() => Promise.resolve(1));
const token = await retryStrategy.acquireInitialRetryToken(retryTokenScope);
try {
Expand All @@ -93,7 +85,7 @@ describe(StandardRetryStrategy.name, () => {
getRetryCount: () => 5,
getRetryTokenCount: (errorInfo: any) => 1,
};
(getDefaultRetryToken as jest.Mock).mockReturnValue(mockRetryToken);
(createDefaultRetryToken as jest.Mock).mockReturnValue(mockRetryToken);
const retryStrategy = new StandardRetryStrategy(() => Promise.resolve(5));
const token = await retryStrategy.acquireInitialRetryToken(retryTokenScope);
try {
Expand All @@ -109,7 +101,7 @@ describe(StandardRetryStrategy.name, () => {
getRetryTokenCount: (errorInfo: any) => 1,
hasRetryTokens: (errorType: RetryErrorType) => false,
};
(getDefaultRetryToken as jest.Mock).mockReturnValue(mockRetryToken);
(createDefaultRetryToken as jest.Mock).mockReturnValue(mockRetryToken);
const retryStrategy = new StandardRetryStrategy(() => Promise.resolve(maxAttempts));
const token = await retryStrategy.acquireInitialRetryToken(retryTokenScope);
try {
Expand All @@ -125,7 +117,7 @@ describe(StandardRetryStrategy.name, () => {
getRetryTokenCount: (errorInfo: any) => 1,
hasRetryTokens: (errorType: RetryErrorType) => true,
};
(getDefaultRetryToken as jest.Mock).mockReturnValue(mockRetryToken);
(createDefaultRetryToken as jest.Mock).mockReturnValue(mockRetryToken);
const retryStrategy = new StandardRetryStrategy(() => Promise.resolve(maxAttempts));
const token = await retryStrategy.acquireInitialRetryToken(retryTokenScope);
const errorInfo = {
Expand All @@ -137,24 +129,5 @@ describe(StandardRetryStrategy.name, () => {
expect(error).toStrictEqual(noRetryTokenAvailableError);
}
});

describe("recordSuccess", () => {
it("releases tokens", async () => {
const retryCost = 1;
const releaseRetryTokens = jest.fn();
const getLastRetryCost = jest.fn().mockReturnValue(retryCost);
const mockRetryToken = {
releaseRetryTokens,
getLastRetryCost,
};
(getDefaultRetryToken as jest.Mock).mockReturnValue(mockRetryToken);
const retryStrategy = new StandardRetryStrategy(() => Promise.resolve(maxAttempts));
const token = await retryStrategy.acquireInitialRetryToken(retryTokenScope);
retryStrategy.recordSuccess(token);
expect(releaseRetryTokens).toHaveBeenCalledTimes(1);
expect(releaseRetryTokens).toHaveBeenCalledWith(retryCost);
expect(getLastRetryCost).toHaveBeenCalledTimes(1);
});
});
});
});
64 changes: 52 additions & 12 deletions packages/util-retry/src/StandardRetryStrategy.ts
Original file line number Diff line number Diff line change
@@ -1,47 +1,82 @@
import { Provider, RetryErrorInfo, RetryErrorType, RetryStrategyV2, StandardRetryToken } from "@aws-sdk/types";

import { DEFAULT_MAX_ATTEMPTS, RETRY_MODES } from "./config";
import { DEFAULT_RETRY_DELAY_BASE, INITIAL_RETRY_TOKENS } from "./constants";
import { getDefaultRetryToken } from "./defaultRetryToken";
import {
DEFAULT_RETRY_DELAY_BASE,
INITIAL_RETRY_TOKENS,
NO_RETRY_INCREMENT,
RETRY_COST,
THROTTLING_RETRY_DELAY_BASE,
TIMEOUT_RETRY_COST,
} from "./constants";
import { getDefaultRetryBackoffStrategy } from "./defaultRetryBackoffStrategy";
import { createDefaultRetryToken } from "./defaultRetryToken";

/**
* @public
*/
export class StandardRetryStrategy implements RetryStrategyV2 {
public readonly mode: string = RETRY_MODES.STANDARD;
private retryToken: StandardRetryToken;
private capacity: number = INITIAL_RETRY_TOKENS;
private readonly retryBackoffStrategy = getDefaultRetryBackoffStrategy();
trivikr marked this conversation as resolved.
Show resolved Hide resolved
private readonly maxAttemptsProvider: Provider<number>;

constructor(maxAttempts: number);
constructor(maxAttemptsProvider: Provider<number>);
constructor(private readonly maxAttempts: number | Provider<number>) {
this.retryToken = getDefaultRetryToken(INITIAL_RETRY_TOKENS, DEFAULT_RETRY_DELAY_BASE);
this.maxAttemptsProvider = typeof maxAttempts === "function" ? maxAttempts : async () => maxAttempts;
}

public async acquireInitialRetryToken(retryTokenScope: string): Promise<StandardRetryToken> {
return this.retryToken;
return createDefaultRetryToken({
retryDelay: DEFAULT_RETRY_DELAY_BASE,
retryCount: 0,
});
}

public async refreshRetryTokenForRetry(
tokenToRenew: StandardRetryToken,
token: StandardRetryToken,
errorInfo: RetryErrorInfo
): Promise<StandardRetryToken> {
const maxAttempts = await this.getMaxAttempts();

if (this.shouldRetry(tokenToRenew, errorInfo, maxAttempts)) {
tokenToRenew.getRetryTokenCount(errorInfo);
return tokenToRenew;
if (this.shouldRetry(token, errorInfo, maxAttempts)) {
const errorType = errorInfo.errorType;
this.retryBackoffStrategy.setDelayBase(
errorType === "THROTTLING" ? THROTTLING_RETRY_DELAY_BASE : DEFAULT_RETRY_DELAY_BASE
);

const delayFromErrorType = this.retryBackoffStrategy.computeNextBackoffDelay(token.getRetryCount());
const retryDelay = errorInfo.retryAfterHint
? Math.max(errorInfo.retryAfterHint.getTime() - Date.now() || 0, delayFromErrorType)
: delayFromErrorType;

const capacityCost = this.getCapacityCost(errorType);
this.capacity -= capacityCost;
return createDefaultRetryToken({
retryDelay,
retryCount: token.getRetryCount() + 1,
retryCost: capacityCost,
});
}

throw new Error("No retry token available");
}

public recordSuccess(token: StandardRetryToken): void {
this.retryToken.releaseRetryTokens(token.getLastRetryCost());
this.capacity = Math.max(INITIAL_RETRY_TOKENS, this.capacity + (token.getRetryCost() ?? NO_RETRY_INCREMENT));
}

/**
* @returns the current available retry capacity.
*
* This number decreases when retries are executed and refills when requests or retries succeed.
*/
public getCapacity(): number {
return this.capacity;
}

private async getMaxAttempts() {
let maxAttempts: number;
try {
return await this.maxAttemptsProvider();
} catch (error) {
Expand All @@ -52,13 +87,18 @@ export class StandardRetryStrategy implements RetryStrategyV2 {

private shouldRetry(tokenToRenew: StandardRetryToken, errorInfo: RetryErrorInfo, maxAttempts: number): boolean {
const attempts = tokenToRenew.getRetryCount();

return (
attempts < maxAttempts &&
tokenToRenew.hasRetryTokens(errorInfo.errorType) &&
this.capacity >= this.getCapacityCost(errorInfo.errorType) &&
this.isRetryableError(errorInfo.errorType)
);
}

private getCapacityCost(errorType: RetryErrorType) {
return errorType === "TRANSIENT" ? TIMEOUT_RETRY_COST : RETRY_COST;
}

private isRetryableError(errorType: RetryErrorType): boolean {
return errorType === "THROTTLING" || errorType === "TRANSIENT";
}
Expand Down
Loading