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

feat: retry delay strategy #871

Merged
merged 13 commits into from
May 1, 2024
5 changes: 5 additions & 0 deletions docs/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,11 @@

### Changed

* fix: retry logic now includes delays with exponential backoff. Retries should no longer happen too quickly for the replica to catch up.

### Added

* new `HttpAgent` option: `backoffStrategy` - allows you to set a custom delay strategy for retries. The default is a newly exported `exponentialBackoff`, but you can pass your own function to customize the delay between retries.
- feat: make `IdbStorage` `get/set` methods generic

## [1.2.0] - 2024-03-25
Expand Down
9 changes: 6 additions & 3 deletions packages/agent/src/actor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ afterEach(() => {
global.Date.now = originalDateNowFn;
});


jest.setTimeout(30000);
describe('makeActor', () => {
// TODO: update tests to be compatible with changes to Certificate
it.skip('should encode calls', async () => {
Expand Down Expand Up @@ -133,7 +135,7 @@ describe('makeActor', () => {

const expectedCallRequestId = await requestIdOf(expectedCallRequest.content);

const httpAgent = new HttpAgent({ fetch: mockFetch });
const httpAgent = new HttpAgent({ fetch: mockFetch, retryTimes: 0 });

const actor = Actor.createActor(actorInterface, { canisterId, agent: httpAgent });
const reply = await actor.greet(argValue);
Expand Down Expand Up @@ -274,6 +276,7 @@ describe('makeActor', () => {
fetch: mockFetch,
host: 'http://127.0.0.1',
verifyQuerySignatures: false,
retryTimes: 0,
});
const canisterId = Principal.fromText('2chl6-4hpzw-vqaaa-aaaaa-c');
const actor = Actor.createActor(actorInterface, { canisterId, agent: httpAgent });
Expand Down Expand Up @@ -313,7 +316,7 @@ describe('makeActor', () => {
greet: IDL.Func([IDL.Text], [IDL.Text]),
});
};
const httpAgent = new HttpAgent({ fetch: mockFetch, host: 'http://127.0.0.1' });
const httpAgent = new HttpAgent({ fetch: mockFetch, host: 'http://127.0.0.1', retryTimes: 0 });
const canisterId = Principal.fromText('2chl6-4hpzw-vqaaa-aaaaa-c');
const actor = Actor.createActor(actorInterface, { canisterId, agent: httpAgent });

Expand All @@ -328,7 +331,7 @@ describe('makeActor', () => {
}
});
it('should throw a helpful error if the canisterId is not set', async () => {
const httpAgent = new HttpAgent({ host: 'http://127.0.0.1' });
const httpAgent = new HttpAgent({ host: 'http://127.0.0.1', retryTimes: 0 });
const actorInterface = () => {
return IDL.Service({
greet: IDL.Func([IDL.Text], [IDL.Text]),
Expand Down
15 changes: 14 additions & 1 deletion packages/agent/src/agent/http/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import { ExpirableMap } from '../../utils/expirableMap';
import { Ed25519PublicKey } from '../../public_key';
import { decodeTime } from '../../utils/leb';
import { ObservableLog } from '../../observable';
import { BackoffStrategy, delayWithStrategy, exponentialBackoff } from '../../polling/strategy';

export * from './transforms';
export { Nonce, makeNonce } from './types';
Expand Down Expand Up @@ -124,6 +125,10 @@ export interface HttpAgentOptions {
* @default 3
*/
retryTimes?: number;
/**
* The strategy to use for backoff when retrying requests
*/
backoffStrategy?: BackoffStrategy;
/**
* Whether the agent should verify signatures signed by node keys on query responses. Increases security, but adds overhead and must make a separate request to cache the node keys for the canister's subnet.
* @default true
Expand Down Expand Up @@ -190,6 +195,7 @@ export class HttpAgent implements Agent {
private readonly _credentials: string | undefined;
private _rootKeyFetched = false;
private readonly _retryTimes; // Retry requests N times before erroring by default
#backoffStrategy: BackoffStrategy;
public readonly _isAgent = true;

// The UTC time in milliseconds when the latest request was made
Expand Down Expand Up @@ -270,6 +276,8 @@ export class HttpAgent implements Agent {
}
// Default is 3
this._retryTimes = options.retryTimes ?? 3;
// Delay strategy for retries. Default is exponential backoff
this.#backoffStrategy = options.backoffStrategy ?? exponentialBackoff;
// Rewrite to avoid redirects
if (this._host.hostname.endsWith(IC0_SUB_DOMAIN)) {
this._host.hostname = IC0_DOMAIN;
Expand Down Expand Up @@ -439,6 +447,9 @@ export class HttpAgent implements Agent {
tries = 0,
): Promise<ApiQueryResponse> {
const { ecid, transformedRequest, body, requestId } = args;

await delayWithStrategy(tries, this.#backoffStrategy);

let response: ApiQueryResponse;
// Make the request and retry if it throws an error
try {
Expand Down Expand Up @@ -529,6 +540,9 @@ export class HttpAgent implements Agent {
}

private async _requestAndRetry(request: () => Promise<Response>, tries = 0): Promise<Response> {
// Delay the request by the exponential backoff strategy
await delayWithStrategy(tries, this.#backoffStrategy);

let response: Response;
try {
response = await request();
Expand Down Expand Up @@ -641,7 +655,6 @@ export class HttpAgent implements Agent {
return this.#subnetKeys.get(ecid.toString());
};
// Attempt to make the query i=retryTimes times
// eslint-disable-next-line @typescript-eslint/no-unused-vars
// Make query and fetch subnet keys in parallel
const [query, subnetStatus] = await Promise.all([makeQuery(), getSubnetStatus()]);

Expand Down
1 change: 1 addition & 0 deletions packages/agent/src/fetch_candid.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ test('simulate fetching a Candid interface', async () => {
fetch: mockFetch,
host: 'http://127.0.0.1',
verifyQuerySignatures: false,
retryTimes: 0,
});

const candid = await fetchCandid('ryjl3-tyaaa-aaaaa-aaaba-cai', agent);
Expand Down
1 change: 1 addition & 0 deletions packages/agent/src/polling/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ export type PollStrategyFactory = () => PollStrategy;
* @param requestId The Request ID to poll status for.
* @param strategy A polling strategy.
* @param request Request for the readState call.
* @param blsVerify - optional replacement function that verifies the BLS signature of a certificate.
*/
export async function pollForResponse(
agent: Agent,
Expand Down
24 changes: 24 additions & 0 deletions packages/agent/src/polling/strategy.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { delayWithStrategy, exponentialBackoff } from './strategy';

describe('delayWithStrategy', () => {
it('should do exponential backoff', async () => {
const iterations = 0;
expect(exponentialBackoff(iterations)).toBe(0);
expect(exponentialBackoff(1)).toBe(300);
expect(exponentialBackoff(2)).toBe(600);
expect(exponentialBackoff(3)).toBe(1200);
});

it('should delay the execution of the function by a certain amount of time', async () => {
jest.useRealTimers();
const iterations = 0;
const strategy = jest.fn().mockReturnValue(0);
await delayWithStrategy(iterations, strategy);
expect(strategy).toHaveBeenCalledWith(iterations);

const iterations2 = 1;
const strategy2 = jest.fn().mockReturnValue(400);
await delayWithStrategy(iterations2, strategy2);
expect(strategy2).toHaveBeenCalledWith(iterations2);
});
});
30 changes: 30 additions & 0 deletions packages/agent/src/polling/strategy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,3 +135,33 @@ export function chain(...strategies: PollStrategy[]): PollStrategy {
}
};
}


export type Delay = number;
export type BackoffStrategy = (retryCount: number) => Delay;

/**
* An exponential backoff strategy.
* @param iterations The number of retries that have been done.
*/
export function exponentialBackoff(iterations: number): Delay {
if (iterations === 0) {
return 0;
}
return 2 ** iterations * 150;
krpeacock marked this conversation as resolved.
Show resolved Hide resolved
}

/**
* Delays the execution of the function by a certain amount of time.
* @param iterations The number of retries that have been done.
* @param delayStrategy The backoff strategy to use.
*/
export async function delayWithStrategy(
iterations = 0,
delayStrategy: BackoffStrategy = exponentialBackoff,
): Promise<void> {
const delay = delayStrategy(iterations);
if (delay > 0) {
await new Promise(resolve => setTimeout(resolve, delay));
}
}
Loading