Skip to content

Commit

Permalink
PR fixes
Browse files Browse the repository at this point in the history
Move the lambda config out of InRegion.

Get rid of the v1 config since it isn't set in stone yet. We will make
one in the future.

Make the numClients unique in the various tests.

Add a test showing numClients can be overridden.
  • Loading branch information
nand4011 committed Aug 11, 2023
1 parent ab2c0dd commit 010f15f
Show file tree
Hide file tree
Showing 5 changed files with 41 additions and 51 deletions.
60 changes: 20 additions & 40 deletions packages/client-sdk-nodejs/src/config/configurations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ export class Laptop extends CacheConfiguration {
const grpcConfig: GrpcConfiguration = new StaticGrpcConfiguration({
deadlineMillis: deadlineMillis,
maxSessionMemoryMb: defaultMaxSessionMemoryMb,
numClients: 6,
});
const transportStrategy: TransportStrategy = new StaticTransportStrategy({
grpcConfiguration: grpcConfig,
Expand All @@ -74,39 +73,27 @@ export class Laptop extends CacheConfiguration {
}
}

class InRegionDefault extends CacheConfiguration {
export class Lambda extends CacheConfiguration {
/**
* Provides the latest recommended configuration for a typical in-region environment. NOTE: this configuration may
* Provides the latest recommended configuration for a lambda environment. NOTE: this configuration may
* change in future releases to take advantage of improvements we identify for default configurations.
* @param {MomentoLoggerFactory} [loggerFactory=defaultLoggerFactory]
* @returns {CacheConfiguration}
*/
static latest(
loggerFactory: MomentoLoggerFactory = defaultLoggerFactory
): CacheConfiguration {
return InRegionDefault.v1(loggerFactory);
}

/**
* Provides v1 recommended configuration for a typical in-region environment. This configuration is guaranteed not
* to change in future releases of the Momento node.js SDK.
* @param {MomentoLoggerFactory} [loggerFactory=defaultLoggerFactory]
* @returns {CacheConfiguration}
*/
static v1(
loggerFactory: MomentoLoggerFactory = defaultLoggerFactory
): CacheConfiguration {
const deadlineMillis = 1100;
const grpcConfig: GrpcConfiguration = new StaticGrpcConfiguration({
deadlineMillis: deadlineMillis,
maxSessionMemoryMb: defaultMaxSessionMemoryMb,
numClients: 6,
numClients: 1,
});
const transportStrategy: TransportStrategy = new StaticTransportStrategy({
grpcConfiguration: grpcConfig,
maxIdleMillis: defaultMaxIdleMillis,
});
return new InRegionDefault({
return new Lambda({
loggerFactory: loggerFactory,
retryStrategy: defaultRetryStrategy(loggerFactory),
transportStrategy: transportStrategy,
Expand All @@ -115,34 +102,32 @@ class InRegionDefault extends CacheConfiguration {
}
}

class InRegionLowLatency extends CacheConfiguration {
class InRegionDefault extends CacheConfiguration {
/**
* Provides the latest recommended configuration for an in-region environment with aggressive low-latency requirements.
* NOTE: this configuration may change in future releases to take advantage of improvements we identify for default
* configurations.
* Provides the latest recommended configuration for a typical in-region environment. NOTE: this configuration may
* change in future releases to take advantage of improvements we identify for default configurations.
* @param {MomentoLoggerFactory} [loggerFactory=defaultLoggerFactory]
* @returns {CacheConfiguration}
*/
static latest(
loggerFactory: MomentoLoggerFactory = defaultLoggerFactory
): CacheConfiguration {
return InRegionLowLatency.v1(loggerFactory);
return InRegionDefault.v1(loggerFactory);
}

/**
* Provides v1 recommended configuration for an in-region environment with aggressive low-latency requirements.
* This configuration is guaranteed not to change in future releases of the Momento node.js SDK.
* Provides v1 recommended configuration for a typical in-region environment. This configuration is guaranteed not
* to change in future releases of the Momento node.js SDK.
* @param {MomentoLoggerFactory} [loggerFactory=defaultLoggerFactory]
* @returns {CacheConfiguration}
*/
static v1(
loggerFactory: MomentoLoggerFactory = defaultLoggerFactory
): CacheConfiguration {
const deadlineMillis = 500;
const deadlineMillis = 1100;
const grpcConfig: GrpcConfiguration = new StaticGrpcConfiguration({
deadlineMillis: deadlineMillis,
maxSessionMemoryMb: defaultMaxSessionMemoryMb,
numClients: 6,
});
const transportStrategy: TransportStrategy = new StaticTransportStrategy({
grpcConfiguration: grpcConfig,
Expand All @@ -157,39 +142,39 @@ class InRegionLowLatency extends CacheConfiguration {
}
}

class InRegionLambda extends CacheConfiguration {
class InRegionLowLatency extends CacheConfiguration {
/**
* Provides the latest recommended configuration for a in-region lambda environment. NOTE: this configuration may
* change in future releases to take advantage of improvements we identify for default configurations.
* Provides the latest recommended configuration for an in-region environment with aggressive low-latency requirements.
* NOTE: this configuration may change in future releases to take advantage of improvements we identify for default
* configurations.
* @param {MomentoLoggerFactory} [loggerFactory=defaultLoggerFactory]
* @returns {CacheConfiguration}
*/
static latest(
loggerFactory: MomentoLoggerFactory = defaultLoggerFactory
): CacheConfiguration {
return InRegionLambda.v1(loggerFactory);
return InRegionLowLatency.v1(loggerFactory);
}

/**
* Provides v1 recommended configuration for an in-region lambda environment. This configuration is guaranteed not
* to change in future releases of the Momento node.js SDK.
* Provides v1 recommended configuration for an in-region environment with aggressive low-latency requirements.
* This configuration is guaranteed not to change in future releases of the Momento node.js SDK.
* @param {MomentoLoggerFactory} [loggerFactory=defaultLoggerFactory]
* @returns {CacheConfiguration}
*/
static v1(
loggerFactory: MomentoLoggerFactory = defaultLoggerFactory
): CacheConfiguration {
const deadlineMillis = 1100;
const deadlineMillis = 500;
const grpcConfig: GrpcConfiguration = new StaticGrpcConfiguration({
deadlineMillis: deadlineMillis,
maxSessionMemoryMb: defaultMaxSessionMemoryMb,
numClients: 1,
});
const transportStrategy: TransportStrategy = new StaticTransportStrategy({
grpcConfiguration: grpcConfig,
maxIdleMillis: defaultMaxIdleMillis,
});
return new InRegionLambda({
return new InRegionDefault({
loggerFactory: loggerFactory,
retryStrategy: defaultRetryStrategy(loggerFactory),
transportStrategy: transportStrategy,
Expand Down Expand Up @@ -219,9 +204,4 @@ export class InRegion {
* @type {InRegionLowLatency}
*/
static LowLatency = InRegionLowLatency;

/**
* This config prioritizes quick setup time and consistent latency for use in lambdas.
*/
static Lambda = InRegionLambda;
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ export interface GrpcConfigurationProps {
* The number of internal clients a cache client will create to communicate with Momento. More of them allows
* more concurrent requests, at the cost of more open connections and the latency of setting up each client.
*/
numClients: number;
numClients?: number;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,12 @@ export class StaticGrpcConfiguration implements GrpcConfiguration {
constructor(props: GrpcConfigurationProps) {
this.deadlineMillis = props.deadlineMillis;
this.maxSessionMemoryMb = props.maxSessionMemoryMb;
this.numClients = props.numClients;
if (props.numClients !== undefined && props.numClients !== null) {
this.numClients = props.numClients;
} else {
// This is the previously hardcoded value and a safe default for most environments.
this.numClients = 6;
}
}

getDeadlineMillis(): number {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ describe('configuration.ts', () => {
const testGrpcConfiguration = new StaticGrpcConfiguration({
deadlineMillis: 90210,
maxSessionMemoryMb: 90211,
numClients: 6,
numClients: 2,
});
const testMaxIdleMillis = 90212;
const testTransportStrategy = new StaticTransportStrategy({
Expand Down Expand Up @@ -53,7 +53,7 @@ describe('configuration.ts', () => {
const newGrpcConfiguration = new StaticGrpcConfiguration({
deadlineMillis: 5000,
maxSessionMemoryMb: 5001,
numClients: 6,
numClients: 3,
});
const newMaxIdleMillis = 5002;
const newTransportStrategy = new StaticTransportStrategy({
Expand Down Expand Up @@ -109,9 +109,4 @@ describe('configuration.ts', () => {
Configurations.InRegion.LowLatency.v1()
);
});
it('should make v1 inregion lambda config available via latest alias', () => {
expect(Configurations.InRegion.Lambda.latest()).toEqual(
Configurations.InRegion.Lambda.v1()
);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
describe('StaticGrpcConfiguration', () => {
const testDeadlineMillis = 90210;
const testMaxSessionMemoryMb = 90211;
const testNumClients = 6;
const testNumClients = 4;
const testGrpcConfiguration = new StaticGrpcConfiguration({
deadlineMillis: testDeadlineMillis,
maxSessionMemoryMb: testMaxSessionMemoryMb,
Expand Down Expand Up @@ -36,12 +36,22 @@ describe('StaticGrpcConfiguration', () => {
newMaxSessionMemory
);
});

it('should support overriding num clients', () => {
const newNumClients = 9;
const configWithNewDeadline =
testGrpcConfiguration.withNumClients(newNumClients);
expect(configWithNewDeadline.getNumClients()).toEqual(newNumClients);
expect(configWithNewDeadline.getMaxSessionMemoryMb()).toEqual(
testMaxSessionMemoryMb
);
});
});

describe('StaticTransportStrategy', () => {
const testDeadlineMillis = 90210;
const testMaxSessionMemoryMb = 90211;
const testNumClients = 6;
const testNumClients = 5;
const testGrpcConfiguration = new StaticGrpcConfiguration({
deadlineMillis: testDeadlineMillis,
maxSessionMemoryMb: testMaxSessionMemoryMb,
Expand Down

0 comments on commit 010f15f

Please sign in to comment.