Skip to content

Commit

Permalink
feat: segregate retry decision for read and write APIs (#1465)
Browse files Browse the repository at this point in the history
* chore: add tests for retry strategies

* feat: segregate retry decision for read and write APIs

* chore: retry all idempotent operations on cancelled errors

* chore: add comment for why cancelled is retryable
  • Loading branch information
pratik151192 authored Nov 12, 2024
1 parent 3337744 commit 2e66ed6
Show file tree
Hide file tree
Showing 2 changed files with 111 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ const retryableGrpcStatusCodes: Array<Status> = [
// commenting out the ones we don't want to retry on for now.

// Status.OK,
// Status.CANCELLED,
// Idempotent operations can be safely retried for CANCELLED errors. These may pop us sometimes during
// client or server side deployments
Status.CANCELLED,
// Status.UNKNOWN,
// Status.INVALID_ARGUMENT,
// Status.DEADLINE_EXCEEDED,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
import {
DefaultEligibilityStrategy,
DefaultMomentoLoggerFactory,
} from '../../../../src';
import {Status} from '@grpc/grpc-js/build/src/constants';
import {Metadata, StatusObject} from '@grpc/grpc-js';
import {ClientMethodDefinition} from '@grpc/grpc-js/build/src/make-client';

describe('DefaultEligibilityStrategy', () => {
const testLoggerFactory = new DefaultMomentoLoggerFactory();
const eligibilityStrategy = new DefaultEligibilityStrategy(testLoggerFactory);

it('should return true for INTERNAL status code and GET request path', () => {
const grpcStatus = {code: Status.INTERNAL} as StatusObject;
const grpcRequest = {
path: '/cache_client.Scs/Get',
} as ClientMethodDefinition<unknown, unknown>;
const requestMetadata = new Metadata();

const isEligible = eligibilityStrategy.isEligibleForRetry({
grpcStatus,
grpcRequest,
requestMetadata,
});

expect(isEligible).toBe(true);
});

it('should return false for UNKNOWN status code and GET request path', () => {
const grpcStatus = {code: Status.UNKNOWN} as StatusObject;
const grpcRequest = {
path: '/cache_client.Scs/Get',
} as ClientMethodDefinition<unknown, unknown>;
const requestMetadata = new Metadata();

const isEligible = eligibilityStrategy.isEligibleForRetry({
grpcStatus,
grpcRequest,
requestMetadata,
});

expect(isEligible).toBe(false);
});

it('should return true for UNAVAILABLE status code and SET request path', () => {
const grpcStatus = {code: Status.UNAVAILABLE} as StatusObject;
const grpcRequest = {
path: '/cache_client.Scs/Set',
} as ClientMethodDefinition<unknown, unknown>;
const requestMetadata = new Metadata();

const isEligible = eligibilityStrategy.isEligibleForRetry({
grpcStatus,
grpcRequest,
requestMetadata,
});

expect(isEligible).toBe(true);
});

it('should return true for CANCELLED status code and GET request path', () => {
const grpcStatus = {code: Status.CANCELLED} as StatusObject;
const grpcRequest = {
path: '/cache_client.Scs/Get',
} as ClientMethodDefinition<unknown, unknown>;
const requestMetadata = new Metadata();

const isEligible = eligibilityStrategy.isEligibleForRetry({
grpcStatus,
grpcRequest,
requestMetadata,
});

expect(isEligible).toBe(true);
});

it('should return true for CANCELLED status code and SET request path', () => {
const grpcStatus = {code: Status.CANCELLED} as StatusObject;
const grpcRequest = {
path: '/cache_client.Scs/Set',
} as ClientMethodDefinition<unknown, unknown>;
const requestMetadata = new Metadata();

const isEligible = eligibilityStrategy.isEligibleForRetry({
grpcStatus,
grpcRequest,
requestMetadata,
});

expect(isEligible).toBe(true);
});

it('should return false for CANCELLED status code and dictionary increment request path', () => {
const grpcStatus = {code: Status.CANCELLED} as StatusObject;
const grpcRequest = {
path: '/cache_client.Scs/DictionaryIncrement',
} as ClientMethodDefinition<unknown, unknown>;
const requestMetadata = new Metadata();

const isEligible = eligibilityStrategy.isEligibleForRetry({
grpcStatus,
grpcRequest,
requestMetadata,
});

expect(isEligible).toBe(false);
});
});

0 comments on commit 2e66ed6

Please sign in to comment.