From d0093fb15fb34cd50a25cc5833b18259f05f8d3a Mon Sep 17 00:00:00 2001 From: selki Date: Fri, 8 Nov 2024 16:42:25 +0000 Subject: [PATCH 01/13] Added Expiring LRU Cache implementation --- src/cache/abstract-assignment-cache.ts | 4 +- ...ing-lru-in-memory-assignment-cache.spec.ts | 37 +++++++++++++++++++ ...expiring-lru-in-memory-assignment-cache.ts | 20 ++++++++++ src/cache/lru-cache.spec.ts | 27 +++++++++++++- src/cache/lru-cache.ts | 14 +++++++ 5 files changed, 100 insertions(+), 2 deletions(-) create mode 100644 src/cache/expiring-lru-in-memory-assignment-cache.spec.ts create mode 100644 src/cache/expiring-lru-in-memory-assignment-cache.ts diff --git a/src/cache/abstract-assignment-cache.ts b/src/cache/abstract-assignment-cache.ts index d5bb9d7..6cd2ffb 100644 --- a/src/cache/abstract-assignment-cache.ts +++ b/src/cache/abstract-assignment-cache.ts @@ -1,6 +1,7 @@ import { getMD5Hash } from '../obfuscation'; -import { LRUCache } from './lru-cache'; +import {ExpiringLRUCache, LRUCache} from './lru-cache'; +import {max} from "lodash"; /** * Assignment cache keys are only on the subject and flag level, while the entire value is used @@ -103,6 +104,7 @@ export class NonExpiringInMemoryAssignmentCache extends AbstractAssignmentCache< * The primary use case is for server-side SDKs, where the cache is shared across * multiple users. In this case, the cache size should be set to the maximum number * of users that can be active at the same time. + * @param {number} maxSize - Maximum cache size */ export class LRUInMemoryAssignmentCache extends AbstractAssignmentCache { constructor(maxSize: number) { diff --git a/src/cache/expiring-lru-in-memory-assignment-cache.spec.ts b/src/cache/expiring-lru-in-memory-assignment-cache.spec.ts new file mode 100644 index 0000000..3648884 --- /dev/null +++ b/src/cache/expiring-lru-in-memory-assignment-cache.spec.ts @@ -0,0 +1,37 @@ +import { ExpiringLRUInMemoryAssignmentCache } from './expiring-lru-in-memory-assignment-cache'; + +describe('ExpiringLRUInMemoryAssignmentCache', () => { + let cache: ExpiringLRUInMemoryAssignmentCache; + const defaultTimout = 60_000; // 10 minutes + + beforeAll(() => { + jest.useFakeTimers(); + cache = new ExpiringLRUInMemoryAssignmentCache(2); + }); + + it(`assignment cache's timeout should default to 10 minutes `, () => { + const key1 = { subjectKey: 'a', flagKey: 'b', banditKey: 'c', actionKey: 'd' }; + cache.set(key1); + jest.advanceTimersByTime(defaultTimout); + expect(cache.has(key1)).toBeFalsy(); + }); + + it(`assignment cache's timeout value is used on construction`, () => { + const expectedTimout = 88; + cache = new ExpiringLRUInMemoryAssignmentCache(2, expectedTimout); + const key1 = { subjectKey: 'a', flagKey: 'b', banditKey: 'c', actionKey: 'd' }; + cache.set(key1); + jest.advanceTimersByTime(expectedTimout); + expect(cache.has(key1)).toBeFalsy(); + }); + + it(`cache shouldn't be invalidated before timeout`, () => { + const key1 = { subjectKey: 'a', flagKey: 'b', banditKey: 'c', actionKey: 'd' }; + cache.set(key1); + + expect(cache.has(key1)).toBeTruthy(); + + jest.advanceTimersByTime(defaultTimout); + expect(cache.has(key1)).toBeFalsy(); + }); +}); diff --git a/src/cache/expiring-lru-in-memory-assignment-cache.ts b/src/cache/expiring-lru-in-memory-assignment-cache.ts new file mode 100644 index 0000000..a378c45 --- /dev/null +++ b/src/cache/expiring-lru-in-memory-assignment-cache.ts @@ -0,0 +1,20 @@ +import { AbstractAssignmentCache } from './abstract-assignment-cache'; +import { ExpiringLRUCache } from './lru-cache'; + +/** + * Variation of LRU caching mechanism that will automatically evict items after + * set time of milliseconds. + * + * It is used to limit the size of the cache. + * + * The primary use case is for server-side SDKs, where the cache is shared across + * multiple users. In this case, the cache size should be set to the maximum number + * of users that can be active at the same time. + * @param {number} maxSize - Maximum cache size + * @param {number} timeout - Time in milliseconds after cache will expire. + */ +export class ExpiringLRUInMemoryAssignmentCache extends AbstractAssignmentCache { + constructor(maxSize: number, timeout = 60_000) { + super(new ExpiringLRUCache(maxSize, timeout)); + } +} diff --git a/src/cache/lru-cache.spec.ts b/src/cache/lru-cache.spec.ts index 9812a5e..9e9c5bb 100644 --- a/src/cache/lru-cache.spec.ts +++ b/src/cache/lru-cache.spec.ts @@ -1,4 +1,4 @@ -import { LRUCache } from './lru-cache'; +import {ExpiringLRUCache, LRUCache} from './lru-cache'; describe('LRUCache', () => { let cache: LRUCache; @@ -62,3 +62,28 @@ describe('LRUCache', () => { expect(oneCache.get('b')).toBe('banana'); }); }); + +describe('Expiring LRU Cache', () => { + let cache: ExpiringLRUCache; + const expectedCacheTimeoutMs = 50; + + beforeEach(async () => { + cache = new ExpiringLRUCache(2, expectedCacheTimeoutMs); + }); + + afterAll(async () => { + jest.restoreAllMocks(); + }); + + it('should evict cache after timeout', async () => { + jest.useFakeTimers(); + jest.spyOn(global, 'setTimeout'); + + cache.set('a', 'apple'); + jest.advanceTimersByTime(expectedCacheTimeoutMs); + + expect(setTimeout).toHaveBeenCalledTimes(1); + expect(setTimeout).toHaveBeenLastCalledWith(expect.any(Function), expectedCacheTimeoutMs); + expect(cache.get('a')).toBeUndefined(); + }); +}); diff --git a/src/cache/lru-cache.ts b/src/cache/lru-cache.ts index d87d29c..2bc31d6 100644 --- a/src/cache/lru-cache.ts +++ b/src/cache/lru-cache.ts @@ -97,3 +97,17 @@ export class LRUCache implements Map { return this; } } + +export class ExpiringLRUCache extends LRUCache { + constructor(readonly maxSize: number, readonly timeout: number) { + super(maxSize); + } + + set(key: string, value: string): this { + const cache = super.set(key, value); + setTimeout(() => { + this.delete(key); + }, this.timeout); + return cache; + } +} From ffc0c590ad2634264f56cdc27866967aa62ee776 Mon Sep 17 00:00:00 2001 From: selki Date: Fri, 8 Nov 2024 16:43:24 +0000 Subject: [PATCH 02/13] Added Expiring LRU Cache implementation --- src/cache/lru-cache.spec.ts | 2 +- src/cache/lru-cache.ts | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/cache/lru-cache.spec.ts b/src/cache/lru-cache.spec.ts index 9e9c5bb..0623109 100644 --- a/src/cache/lru-cache.spec.ts +++ b/src/cache/lru-cache.spec.ts @@ -1,4 +1,4 @@ -import {ExpiringLRUCache, LRUCache} from './lru-cache'; +import { ExpiringLRUCache, LRUCache } from './lru-cache'; describe('LRUCache', () => { let cache: LRUCache; diff --git a/src/cache/lru-cache.ts b/src/cache/lru-cache.ts index 2bc31d6..ed197f2 100644 --- a/src/cache/lru-cache.ts +++ b/src/cache/lru-cache.ts @@ -97,7 +97,11 @@ export class LRUCache implements Map { return this; } } - +/** + * Variation of LRUCache that expires after set time in milliseconds + * @param {number} maxSize - Maximum cache size + * @param {number} timeout - Time in milliseconds after which cache entry will evict itself + **/ export class ExpiringLRUCache extends LRUCache { constructor(readonly maxSize: number, readonly timeout: number) { super(maxSize); From 90429c1ec21c15ebe1606e935093ff99a6ba2f55 Mon Sep 17 00:00:00 2001 From: selki Date: Fri, 8 Nov 2024 16:59:01 +0000 Subject: [PATCH 03/13] Reorganized caches into their own files --- src/cache/abstract-assignment-cache.spec.ts | 2 +- src/cache/abstract-assignment-cache.ts | 33 ------------- src/cache/lru-cache.ts | 1 + src/cache/lru-in-memory-assignment-cache.ts | 18 ++++++++ ...non-expiring-in-memory-cache-assignment.ts | 15 ++++++ src/client/eppo-client.ts | 46 +++++++++---------- src/index.ts | 4 +- 7 files changed, 59 insertions(+), 60 deletions(-) create mode 100644 src/cache/lru-in-memory-assignment-cache.ts create mode 100644 src/cache/non-expiring-in-memory-cache-assignment.ts diff --git a/src/cache/abstract-assignment-cache.spec.ts b/src/cache/abstract-assignment-cache.spec.ts index 6482dd7..4668ade 100644 --- a/src/cache/abstract-assignment-cache.spec.ts +++ b/src/cache/abstract-assignment-cache.spec.ts @@ -1,8 +1,8 @@ import { assignmentCacheKeyToString, assignmentCacheValueToString, - NonExpiringInMemoryAssignmentCache, } from './abstract-assignment-cache'; +import { NonExpiringInMemoryAssignmentCache } from './non-expiring-in-memory-cache-assignment'; describe('NonExpiringInMemoryAssignmentCache', () => { it('read and write variation entries', () => { diff --git a/src/cache/abstract-assignment-cache.ts b/src/cache/abstract-assignment-cache.ts index 6cd2ffb..91298fd 100644 --- a/src/cache/abstract-assignment-cache.ts +++ b/src/cache/abstract-assignment-cache.ts @@ -1,8 +1,5 @@ import { getMD5Hash } from '../obfuscation'; -import {ExpiringLRUCache, LRUCache} from './lru-cache'; -import {max} from "lodash"; - /** * Assignment cache keys are only on the subject and flag level, while the entire value is used * for uniqueness checking. This way that if an assigned variation or bandit action changes for a @@ -81,33 +78,3 @@ export abstract class AbstractAssignmentCache> return this.delegate.entries(); } } - -/** - * A cache that never expires. - * - * The primary use case is for client-side SDKs, where the cache is only used - * for a single user. - */ -export class NonExpiringInMemoryAssignmentCache extends AbstractAssignmentCache< - Map -> { - constructor(store = new Map()) { - super(store); - } -} - -/** - * A cache that uses the LRU algorithm to evict the least recently used items. - * - * It is used to limit the size of the cache. - * - * The primary use case is for server-side SDKs, where the cache is shared across - * multiple users. In this case, the cache size should be set to the maximum number - * of users that can be active at the same time. - * @param {number} maxSize - Maximum cache size - */ -export class LRUInMemoryAssignmentCache extends AbstractAssignmentCache { - constructor(maxSize: number) { - super(new LRUCache(maxSize)); - } -} diff --git a/src/cache/lru-cache.ts b/src/cache/lru-cache.ts index ed197f2..7103f89 100644 --- a/src/cache/lru-cache.ts +++ b/src/cache/lru-cache.ts @@ -97,6 +97,7 @@ export class LRUCache implements Map { return this; } } + /** * Variation of LRUCache that expires after set time in milliseconds * @param {number} maxSize - Maximum cache size diff --git a/src/cache/lru-in-memory-assignment-cache.ts b/src/cache/lru-in-memory-assignment-cache.ts new file mode 100644 index 0000000..2b51b72 --- /dev/null +++ b/src/cache/lru-in-memory-assignment-cache.ts @@ -0,0 +1,18 @@ +import { AbstractAssignmentCache } from './abstract-assignment-cache'; +import { LRUCache } from './lru-cache'; + +/** + * A cache that uses the LRU algorithm to evict the least recently used items. + * + * It is used to limit the size of the cache. + * + * The primary use case is for server-side SDKs, where the cache is shared across + * multiple users. In this case, the cache size should be set to the maximum number + * of users that can be active at the same time. + * @param {number} maxSize - Maximum cache size + */ +export class LRUInMemoryAssignmentCache extends AbstractAssignmentCache { + constructor(maxSize: number) { + super(new LRUCache(maxSize)); + } +} diff --git a/src/cache/non-expiring-in-memory-cache-assignment.ts b/src/cache/non-expiring-in-memory-cache-assignment.ts new file mode 100644 index 0000000..10120f5 --- /dev/null +++ b/src/cache/non-expiring-in-memory-cache-assignment.ts @@ -0,0 +1,15 @@ +import { AbstractAssignmentCache } from './abstract-assignment-cache'; + +/** + * A cache that never expires. + * + * The primary use case is for client-side SDKs, where the cache is only used + * for a single user. + */ +export class NonExpiringInMemoryAssignmentCache extends AbstractAssignmentCache< + Map +> { + constructor(store = new Map()) { + super(store); + } +} diff --git a/src/client/eppo-client.ts b/src/client/eppo-client.ts index 05d76c2..3e56d01 100644 --- a/src/client/eppo-client.ts +++ b/src/client/eppo-client.ts @@ -3,11 +3,9 @@ import { logger } from '../application-logger'; import { IAssignmentEvent, IAssignmentLogger } from '../assignment-logger'; import { BanditEvaluator } from '../bandit-evaluator'; import { IBanditEvent, IBanditLogger } from '../bandit-logger'; -import { - AssignmentCache, - LRUInMemoryAssignmentCache, - NonExpiringInMemoryAssignmentCache, -} from '../cache/abstract-assignment-cache'; +import { AssignmentCache } from '../cache/abstract-assignment-cache'; +import { LRUInMemoryAssignmentCache } from '../cache/lru-in-memory-assignment-cache'; +import { NonExpiringInMemoryAssignmentCache } from '../cache/non-expiring-in-memory-cache-assignment'; import ConfigurationRequestor from '../configuration-requestor'; import { IConfigurationStore } from '../configuration-store/configuration-store'; import { @@ -254,25 +252,6 @@ export default class EppoClient { return this.getBooleanAssignment(flagKey, subjectKey, subjectAttributes, defaultValue); } - /** - * Maps a subject to a boolean variation for a given experiment. - * - * @param flagKey feature flag identifier - * @param subjectKey an identifier of the experiment subject, for example a user ID. - * @param subjectAttributes optional attributes associated with the subject, for example name and email. - * @param defaultValue default value to return if the subject is not part of the experiment sample - * @returns a boolean variation value if the subject is part of the experiment sample, otherwise the default value - */ - public getBooleanAssignment( - flagKey: string, - subjectKey: string, - subjectAttributes: Attributes, - defaultValue: boolean, - ): boolean { - return this.getBooleanAssignmentDetails(flagKey, subjectKey, subjectAttributes, defaultValue) - .variation; - } - /** * Maps a subject to a boolean variation for a given experiment and provides additional details about the * variation assigned and the reason for the assignment. @@ -657,6 +636,25 @@ export default class EppoClient { return result; } + /** + * Maps a subject to a boolean variation for a given experiment. + * + * @param flagKey feature flag identifier + * @param subjectKey an identifier of the experiment subject, for example a user ID. + * @param subjectAttributes optional attributes associated with the subject, for example name and email. + * @param defaultValue default value to return if the subject is not part of the experiment sample + * @returns a boolean variation value if the subject is part of the experiment sample, otherwise the default value + */ + public getBooleanAssignment( + flagKey: string, + subjectKey: string, + subjectAttributes: Attributes, + defaultValue: boolean, + ): boolean { + return this.getBooleanAssignmentDetails(flagKey, subjectKey, subjectAttributes, defaultValue) + .variation; + } + private ensureActionsWithContextualAttributes( actions: BanditActions, ): Record { diff --git a/src/index.ts b/src/index.ts index a4b0eec..7bc3d9a 100644 --- a/src/index.ts +++ b/src/index.ts @@ -6,8 +6,6 @@ import { IBanditLogger, IBanditEvent } from './bandit-logger'; import { AbstractAssignmentCache, AssignmentCache, - NonExpiringInMemoryAssignmentCache, - LRUInMemoryAssignmentCache, AsyncMap, AssignmentCacheKey, AssignmentCacheValue, @@ -15,6 +13,8 @@ import { assignmentCacheKeyToString, assignmentCacheValueToString, } from './cache/abstract-assignment-cache'; +import { LRUInMemoryAssignmentCache } from './cache/lru-in-memory-assignment-cache'; +import { NonExpiringInMemoryAssignmentCache } from './cache/non-expiring-in-memory-cache-assignment'; import EppoClient, { FlagConfigurationRequestParameters, IAssignmentDetails, From e4d1d268a1240cb6e2d977c427e4a8e2d3ac8ae1 Mon Sep 17 00:00:00 2001 From: selki Date: Fri, 8 Nov 2024 18:21:32 +0000 Subject: [PATCH 04/13] fixed older unit tests --- src/client/eppo-client-with-bandits.spec.ts | 2 +- src/client/eppo-client.ts | 9 +++++++++ src/configuration-requestor.spec.ts | 2 +- 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/client/eppo-client-with-bandits.spec.ts b/src/client/eppo-client-with-bandits.spec.ts index 0610a90..9a8ccc8 100644 --- a/src/client/eppo-client-with-bandits.spec.ts +++ b/src/client/eppo-client-with-bandits.spec.ts @@ -163,7 +163,7 @@ describe('EppoClient Bandits E2E test', () => { expect(banditEvent.action).toBe('adidas'); expect(banditEvent.actionProbability).toBeCloseTo(0.099); expect(banditEvent.optimalityGap).toBe(7.1); - expect(banditEvent.modelVersion).toBe('v123'); + expect(banditEvent.modelVersion).toBe('123'); expect(banditEvent.subjectNumericAttributes).toStrictEqual({ age: 25 }); expect(banditEvent.subjectCategoricalAttributes).toStrictEqual({ country: 'USA', diff --git a/src/client/eppo-client.ts b/src/client/eppo-client.ts index 3e56d01..6f17d1a 100644 --- a/src/client/eppo-client.ts +++ b/src/client/eppo-client.ts @@ -4,6 +4,7 @@ import { IAssignmentEvent, IAssignmentLogger } from '../assignment-logger'; import { BanditEvaluator } from '../bandit-evaluator'; import { IBanditEvent, IBanditLogger } from '../bandit-logger'; import { AssignmentCache } from '../cache/abstract-assignment-cache'; +import { ExpiringLRUInMemoryAssignmentCache } from '../cache/expiring-lru-in-memory-assignment-cache'; import { LRUInMemoryAssignmentCache } from '../cache/lru-in-memory-assignment-cache'; import { NonExpiringInMemoryAssignmentCache } from '../cache/non-expiring-in-memory-cache-assignment'; import ConfigurationRequestor from '../configuration-requestor'; @@ -985,6 +986,14 @@ export default class EppoClient { this.banditAssignmentCache = new LRUInMemoryAssignmentCache(maxSize); } + /** + * @param {number} maxSize - Maximum cache size + * @param {number} timeout - TTL of cache entries + */ + public useExpiringInMemoryBanditAssignmentCache(maxSize: number, timeout?: number) { + this.banditAssignmentCache = new ExpiringLRUInMemoryAssignmentCache(maxSize, timeout); + } + public useCustomBanditAssignmentCache(cache: AssignmentCache) { this.banditAssignmentCache = cache; } diff --git a/src/configuration-requestor.spec.ts b/src/configuration-requestor.spec.ts index 746bb0f..b3bb05f 100644 --- a/src/configuration-requestor.spec.ts +++ b/src/configuration-requestor.spec.ts @@ -141,7 +141,7 @@ describe('ConfigurationRequestor', () => { const bannerBandit = banditModelStore.get('banner_bandit'); expect(bannerBandit?.banditKey).toBe('banner_bandit'); expect(bannerBandit?.modelName).toBe('falcon'); - expect(bannerBandit?.modelVersion).toBe('v123'); + expect(bannerBandit?.modelVersion).toBe('123'); const bannerModelData = bannerBandit?.modelData; expect(bannerModelData?.gamma).toBe(1); expect(bannerModelData?.defaultActionScore).toBe(0); From 001ae6eb7ca76283ccbbff5c17b11593340161c4 Mon Sep 17 00:00:00 2001 From: selki Date: Tue, 12 Nov 2024 12:06:02 +0000 Subject: [PATCH 05/13] renaming classes and file organizing --- ...expiring-lru-in-memory-assignment-cache.ts | 20 -------------- src/cache/lru-cache.spec.ts | 27 +------------------ src/cache/lru-cache.ts | 10 +++---- src/cache/tlru-cache.spec.ts | 26 ++++++++++++++++++ src/cache/tlru-cache.ts | 20 ++++++++++++++ ...> tlru-in-memory-assignment-cache.spec.ts} | 8 +++--- src/cache/tlru-in-memory-assignment-cache.ts | 17 ++++++++++++ src/client/eppo-client.ts | 8 +++--- 8 files changed, 77 insertions(+), 59 deletions(-) delete mode 100644 src/cache/expiring-lru-in-memory-assignment-cache.ts create mode 100644 src/cache/tlru-cache.spec.ts create mode 100644 src/cache/tlru-cache.ts rename src/cache/{expiring-lru-in-memory-assignment-cache.spec.ts => tlru-in-memory-assignment-cache.spec.ts} (79%) create mode 100644 src/cache/tlru-in-memory-assignment-cache.ts diff --git a/src/cache/expiring-lru-in-memory-assignment-cache.ts b/src/cache/expiring-lru-in-memory-assignment-cache.ts deleted file mode 100644 index a378c45..0000000 --- a/src/cache/expiring-lru-in-memory-assignment-cache.ts +++ /dev/null @@ -1,20 +0,0 @@ -import { AbstractAssignmentCache } from './abstract-assignment-cache'; -import { ExpiringLRUCache } from './lru-cache'; - -/** - * Variation of LRU caching mechanism that will automatically evict items after - * set time of milliseconds. - * - * It is used to limit the size of the cache. - * - * The primary use case is for server-side SDKs, where the cache is shared across - * multiple users. In this case, the cache size should be set to the maximum number - * of users that can be active at the same time. - * @param {number} maxSize - Maximum cache size - * @param {number} timeout - Time in milliseconds after cache will expire. - */ -export class ExpiringLRUInMemoryAssignmentCache extends AbstractAssignmentCache { - constructor(maxSize: number, timeout = 60_000) { - super(new ExpiringLRUCache(maxSize, timeout)); - } -} diff --git a/src/cache/lru-cache.spec.ts b/src/cache/lru-cache.spec.ts index 0623109..9812a5e 100644 --- a/src/cache/lru-cache.spec.ts +++ b/src/cache/lru-cache.spec.ts @@ -1,4 +1,4 @@ -import { ExpiringLRUCache, LRUCache } from './lru-cache'; +import { LRUCache } from './lru-cache'; describe('LRUCache', () => { let cache: LRUCache; @@ -62,28 +62,3 @@ describe('LRUCache', () => { expect(oneCache.get('b')).toBe('banana'); }); }); - -describe('Expiring LRU Cache', () => { - let cache: ExpiringLRUCache; - const expectedCacheTimeoutMs = 50; - - beforeEach(async () => { - cache = new ExpiringLRUCache(2, expectedCacheTimeoutMs); - }); - - afterAll(async () => { - jest.restoreAllMocks(); - }); - - it('should evict cache after timeout', async () => { - jest.useFakeTimers(); - jest.spyOn(global, 'setTimeout'); - - cache.set('a', 'apple'); - jest.advanceTimersByTime(expectedCacheTimeoutMs); - - expect(setTimeout).toHaveBeenCalledTimes(1); - expect(setTimeout).toHaveBeenLastCalledWith(expect.any(Function), expectedCacheTimeoutMs); - expect(cache.get('a')).toBeUndefined(); - }); -}); diff --git a/src/cache/lru-cache.ts b/src/cache/lru-cache.ts index 7103f89..4134bdc 100644 --- a/src/cache/lru-cache.ts +++ b/src/cache/lru-cache.ts @@ -99,12 +99,12 @@ export class LRUCache implements Map { } /** - * Variation of LRUCache that expires after set time in milliseconds + * Time-aware, least-recently-used (TLRU), variant of LRU where entries have valid lifetime. * @param {number} maxSize - Maximum cache size - * @param {number} timeout - Time in milliseconds after which cache entry will evict itself + * @param {number} ttl - Time in milliseconds after which cache entry will evict itself **/ -export class ExpiringLRUCache extends LRUCache { - constructor(readonly maxSize: number, readonly timeout: number) { +export class TLRUCache extends LRUCache { + constructor(readonly maxSize: number, readonly ttl: number) { super(maxSize); } @@ -112,7 +112,7 @@ export class ExpiringLRUCache extends LRUCache { const cache = super.set(key, value); setTimeout(() => { this.delete(key); - }, this.timeout); + }, this.ttl); return cache; } } diff --git a/src/cache/tlru-cache.spec.ts b/src/cache/tlru-cache.spec.ts new file mode 100644 index 0000000..0017b12 --- /dev/null +++ b/src/cache/tlru-cache.spec.ts @@ -0,0 +1,26 @@ +import { TLRUCache } from './lru-cache'; + +describe('TLRU Cache', () => { + let cache: TLRUCache; + const expectedCacheTimeoutMs = 50; + + beforeEach(async () => { + cache = new TLRUCache(2, expectedCacheTimeoutMs); + }); + + afterAll(async () => { + jest.restoreAllMocks(); + }); + + it('should evict cache after timeout', async () => { + jest.useFakeTimers(); + jest.spyOn(global, 'setTimeout'); + + cache.set('a', 'apple'); + jest.advanceTimersByTime(expectedCacheTimeoutMs); + + expect(setTimeout).toHaveBeenCalledTimes(1); + expect(setTimeout).toHaveBeenLastCalledWith(expect.any(Function), expectedCacheTimeoutMs); + expect(cache.get('a')).toBeUndefined(); + }); +}); diff --git a/src/cache/tlru-cache.ts b/src/cache/tlru-cache.ts new file mode 100644 index 0000000..5cb1c5a --- /dev/null +++ b/src/cache/tlru-cache.ts @@ -0,0 +1,20 @@ +import { LRUCache } from './lru-cache'; + +/** + * Time-aware, least-recently-used (TLRU), variant of LRU where entries have valid lifetime. + * @param {number} maxSize - Maximum cache size + * @param {number} ttl - Time in milliseconds after which cache entry will evict itself + **/ +export class TLRUCache extends LRUCache { + constructor(readonly maxSize: number, readonly ttl: number) { + super(maxSize); + } + + set(key: string, value: string): this { + const cache = super.set(key, value); + setTimeout(() => { + this.delete(key); + }, this.ttl); + return cache; + } +} diff --git a/src/cache/expiring-lru-in-memory-assignment-cache.spec.ts b/src/cache/tlru-in-memory-assignment-cache.spec.ts similarity index 79% rename from src/cache/expiring-lru-in-memory-assignment-cache.spec.ts rename to src/cache/tlru-in-memory-assignment-cache.spec.ts index 3648884..f818ebc 100644 --- a/src/cache/expiring-lru-in-memory-assignment-cache.spec.ts +++ b/src/cache/tlru-in-memory-assignment-cache.spec.ts @@ -1,12 +1,12 @@ -import { ExpiringLRUInMemoryAssignmentCache } from './expiring-lru-in-memory-assignment-cache'; +import { TLRUInMemoryAssignmentCache } from './tlru-in-memory-assignment-cache'; describe('ExpiringLRUInMemoryAssignmentCache', () => { - let cache: ExpiringLRUInMemoryAssignmentCache; + let cache: TLRUInMemoryAssignmentCache; const defaultTimout = 60_000; // 10 minutes beforeAll(() => { jest.useFakeTimers(); - cache = new ExpiringLRUInMemoryAssignmentCache(2); + cache = new TLRUInMemoryAssignmentCache(2); }); it(`assignment cache's timeout should default to 10 minutes `, () => { @@ -18,7 +18,7 @@ describe('ExpiringLRUInMemoryAssignmentCache', () => { it(`assignment cache's timeout value is used on construction`, () => { const expectedTimout = 88; - cache = new ExpiringLRUInMemoryAssignmentCache(2, expectedTimout); + cache = new TLRUInMemoryAssignmentCache(2, expectedTimout); const key1 = { subjectKey: 'a', flagKey: 'b', banditKey: 'c', actionKey: 'd' }; cache.set(key1); jest.advanceTimersByTime(expectedTimout); diff --git a/src/cache/tlru-in-memory-assignment-cache.ts b/src/cache/tlru-in-memory-assignment-cache.ts new file mode 100644 index 0000000..275ff3a --- /dev/null +++ b/src/cache/tlru-in-memory-assignment-cache.ts @@ -0,0 +1,17 @@ +import { AbstractAssignmentCache } from './abstract-assignment-cache'; +import { TLRUCache } from './tlru-cache'; + +/** + * Variation of LRU caching mechanism that will automatically evict items after + * set time of milliseconds. + * + * It is used to limit the size of the cache. + * + * @param {number} maxSize - Maximum cache size + * @param {number} ttl - Time in milliseconds after cache will expire. + */ +export class TLRUInMemoryAssignmentCache extends AbstractAssignmentCache { + constructor(maxSize: number, ttl = 60_000) { + super(new TLRUCache(maxSize, ttl)); + } +} diff --git a/src/client/eppo-client.ts b/src/client/eppo-client.ts index 6f17d1a..bd3f3cb 100644 --- a/src/client/eppo-client.ts +++ b/src/client/eppo-client.ts @@ -4,9 +4,9 @@ import { IAssignmentEvent, IAssignmentLogger } from '../assignment-logger'; import { BanditEvaluator } from '../bandit-evaluator'; import { IBanditEvent, IBanditLogger } from '../bandit-logger'; import { AssignmentCache } from '../cache/abstract-assignment-cache'; -import { ExpiringLRUInMemoryAssignmentCache } from '../cache/expiring-lru-in-memory-assignment-cache'; import { LRUInMemoryAssignmentCache } from '../cache/lru-in-memory-assignment-cache'; import { NonExpiringInMemoryAssignmentCache } from '../cache/non-expiring-in-memory-cache-assignment'; +import { TLRUInMemoryAssignmentCache } from '../cache/tlru-in-memory-assignment-cache'; import ConfigurationRequestor from '../configuration-requestor'; import { IConfigurationStore } from '../configuration-store/configuration-store'; import { @@ -983,15 +983,15 @@ export default class EppoClient { } public useLRUInMemoryBanditAssignmentCache(maxSize: number) { - this.banditAssignmentCache = new LRUInMemoryAssignmentCache(maxSize); + this.banditAssignmentCache = new TLRUInMemoryAssignmentCache(maxSize); } /** * @param {number} maxSize - Maximum cache size * @param {number} timeout - TTL of cache entries */ - public useExpiringInMemoryBanditAssignmentCache(maxSize: number, timeout?: number) { - this.banditAssignmentCache = new ExpiringLRUInMemoryAssignmentCache(maxSize, timeout); + public useTLRUInMemoryAssignmentCache(maxSize: number, timeout?: number) { + this.banditAssignmentCache = new TLRUInMemoryAssignmentCache(maxSize, timeout); } public useCustomBanditAssignmentCache(cache: AssignmentCache) { From c386303161a99747ef49cc4aac1a8e8b0f39197c Mon Sep 17 00:00:00 2001 From: selki Date: Tue, 12 Nov 2024 21:16:38 +0000 Subject: [PATCH 06/13] Fixed cache eviction for TLRU cache entries that were overwritten. Changed default ttl of TLRU assignment cache. --- src/cache/lru-cache.ts | 19 -------- src/cache/tlru-cache.spec.ts | 43 +++++++++++++++++-- src/cache/tlru-cache.ts | 42 ++++++++++++++++-- .../tlru-in-memory-assignment-cache.spec.ts | 26 ++++++----- src/cache/tlru-in-memory-assignment-cache.ts | 2 +- src/client/eppo-client.ts | 38 ++++++++-------- 6 files changed, 113 insertions(+), 57 deletions(-) diff --git a/src/cache/lru-cache.ts b/src/cache/lru-cache.ts index 4134bdc..d87d29c 100644 --- a/src/cache/lru-cache.ts +++ b/src/cache/lru-cache.ts @@ -97,22 +97,3 @@ export class LRUCache implements Map { return this; } } - -/** - * Time-aware, least-recently-used (TLRU), variant of LRU where entries have valid lifetime. - * @param {number} maxSize - Maximum cache size - * @param {number} ttl - Time in milliseconds after which cache entry will evict itself - **/ -export class TLRUCache extends LRUCache { - constructor(readonly maxSize: number, readonly ttl: number) { - super(maxSize); - } - - set(key: string, value: string): this { - const cache = super.set(key, value); - setTimeout(() => { - this.delete(key); - }, this.ttl); - return cache; - } -} diff --git a/src/cache/tlru-cache.spec.ts b/src/cache/tlru-cache.spec.ts index 0017b12..daa6ead 100644 --- a/src/cache/tlru-cache.spec.ts +++ b/src/cache/tlru-cache.spec.ts @@ -1,18 +1,19 @@ -import { TLRUCache } from './lru-cache'; +import { TLRUCache } from './tlru-cache'; describe('TLRU Cache', () => { let cache: TLRUCache; - const expectedCacheTimeoutMs = 50; + const expectedCacheTimeoutMs = 10; beforeEach(async () => { cache = new TLRUCache(2, expectedCacheTimeoutMs); }); - afterAll(async () => { + afterEach(async () => { jest.restoreAllMocks(); + jest.clearAllTimers(); }); - it('should evict cache after timeout', async () => { + it('should evict cache after timeout', () => { jest.useFakeTimers(); jest.spyOn(global, 'setTimeout'); @@ -23,4 +24,38 @@ describe('TLRU Cache', () => { expect(setTimeout).toHaveBeenLastCalledWith(expect.any(Function), expectedCacheTimeoutMs); expect(cache.get('a')).toBeUndefined(); }); + + /** + * This test assumes implementation which is not ideal, but that's + * the only way I know of how to go around timers in jest + **/ + it('should overwrite existing cache entry', () => { + jest.useFakeTimers(); + jest.spyOn(global, 'setTimeout'); + jest.spyOn(global, 'clearTimeout'); + + cache.set('a', 'apple'); + jest.advanceTimersByTime(expectedCacheTimeoutMs - 1); + cache.set('a', 'avocado'); + + expect(setTimeout).toHaveBeenCalledTimes(2); + expect(setTimeout).toHaveBeenLastCalledWith(expect.any(Function), expectedCacheTimeoutMs); + expect(clearTimeout).toHaveBeenCalledTimes(1); + // spin the clock by 5sec. After that time cache entry should be still valid. + jest.advanceTimersByTime(expectedCacheTimeoutMs / 2); + + // setting assertion in a weird way because calling cache.get() + // will reset eviction timer which will mess up next assertion + let avocadoInCache = false; + cache.forEach((value, key) => { + if (key === 'a' && value === 'avocado') { + avocadoInCache = true; + } + }); + expect(avocadoInCache).toBe(true); + + // after another spin of 5 sec, cache entry should evict itself + jest.advanceTimersByTime(expectedCacheTimeoutMs / 2); + expect(cache.has('a')).toBeFalsy(); + }); }); diff --git a/src/cache/tlru-cache.ts b/src/cache/tlru-cache.ts index 5cb1c5a..5ad27bf 100644 --- a/src/cache/tlru-cache.ts +++ b/src/cache/tlru-cache.ts @@ -6,15 +6,51 @@ import { LRUCache } from './lru-cache'; * @param {number} ttl - Time in milliseconds after which cache entry will evict itself **/ export class TLRUCache extends LRUCache { + private readonly cacheEntriesTimoutIds = new Map>(); constructor(readonly maxSize: number, readonly ttl: number) { super(maxSize); } - set(key: string, value: string): this { - const cache = super.set(key, value); - setTimeout(() => { + private clearCacheEntryTimeout(key: string): void { + const timeoutId = this.cacheEntriesTimoutIds.get(key); + clearTimeout(timeoutId); + this.cacheEntriesTimoutIds.delete(key); + } + + private setCacheEntryTimout(key: string): void { + const timeOutId = setTimeout(() => { this.delete(key); }, this.ttl); + + this.cacheEntriesTimoutIds.set(key, timeOutId); + } + + delete(key: string): boolean { + this.clearCacheEntryTimeout(key); + return super.delete(key); + } + + get(key: string): string | undefined { + const value = super.get(key); + + // Whenever we get a cache hit, we need to reset the timer + // for eviction, because it is now considered most recently + // accessed thus the timer should start over. Not doing that + // will cause a de-sync that will stop proper eviction + this.clearCacheEntryTimeout(key); + if (value) { + this.setCacheEntryTimout(key); + } + return value; + } + + set(key: string, value: string): this { + const cache = super.set(key, value); + if (this.cacheEntriesTimoutIds.has(key)) { + this.clearCacheEntryTimeout(key); + } + this.setCacheEntryTimout(key); + return cache; } } diff --git a/src/cache/tlru-in-memory-assignment-cache.spec.ts b/src/cache/tlru-in-memory-assignment-cache.spec.ts index f818ebc..c03d64e 100644 --- a/src/cache/tlru-in-memory-assignment-cache.spec.ts +++ b/src/cache/tlru-in-memory-assignment-cache.spec.ts @@ -2,36 +2,40 @@ import { TLRUInMemoryAssignmentCache } from './tlru-in-memory-assignment-cache'; describe('ExpiringLRUInMemoryAssignmentCache', () => { let cache: TLRUInMemoryAssignmentCache; - const defaultTimout = 60_000; // 10 minutes + const defaultTimout = 600_000; // 10 minutes beforeAll(() => { jest.useFakeTimers(); cache = new TLRUInMemoryAssignmentCache(2); }); + afterAll(() => { + jest.clearAllTimers(); + }); + it(`assignment cache's timeout should default to 10 minutes `, () => { - const key1 = { subjectKey: 'a', flagKey: 'b', banditKey: 'c', actionKey: 'd' }; - cache.set(key1); + const cacheEntry = { subjectKey: 'a', flagKey: 'b', banditKey: 'c', actionKey: 'd' }; + cache.set(cacheEntry); jest.advanceTimersByTime(defaultTimout); - expect(cache.has(key1)).toBeFalsy(); + expect(cache.has(cacheEntry)).toBeFalsy(); }); it(`assignment cache's timeout value is used on construction`, () => { const expectedTimout = 88; cache = new TLRUInMemoryAssignmentCache(2, expectedTimout); - const key1 = { subjectKey: 'a', flagKey: 'b', banditKey: 'c', actionKey: 'd' }; - cache.set(key1); + const cacheEntry = { subjectKey: 'a', flagKey: 'b', banditKey: 'c', actionKey: 'd' }; + cache.set(cacheEntry); jest.advanceTimersByTime(expectedTimout); - expect(cache.has(key1)).toBeFalsy(); + expect(cache.has(cacheEntry)).toBeFalsy(); }); it(`cache shouldn't be invalidated before timeout`, () => { - const key1 = { subjectKey: 'a', flagKey: 'b', banditKey: 'c', actionKey: 'd' }; - cache.set(key1); + const cacheEntry = { subjectKey: 'a', flagKey: 'b', banditKey: 'c', actionKey: 'd' }; + cache.set(cacheEntry); - expect(cache.has(key1)).toBeTruthy(); + expect(cache.has(cacheEntry)).toBeTruthy(); jest.advanceTimersByTime(defaultTimout); - expect(cache.has(key1)).toBeFalsy(); + expect(cache.has(cacheEntry)).toBeFalsy(); }); }); diff --git a/src/cache/tlru-in-memory-assignment-cache.ts b/src/cache/tlru-in-memory-assignment-cache.ts index 275ff3a..a17c2d7 100644 --- a/src/cache/tlru-in-memory-assignment-cache.ts +++ b/src/cache/tlru-in-memory-assignment-cache.ts @@ -11,7 +11,7 @@ import { TLRUCache } from './tlru-cache'; * @param {number} ttl - Time in milliseconds after cache will expire. */ export class TLRUInMemoryAssignmentCache extends AbstractAssignmentCache { - constructor(maxSize: number, ttl = 60_000) { + constructor(maxSize: number, ttl = 600_000) { super(new TLRUCache(maxSize, ttl)); } } diff --git a/src/client/eppo-client.ts b/src/client/eppo-client.ts index bd3f3cb..0c8cf76 100644 --- a/src/client/eppo-client.ts +++ b/src/client/eppo-client.ts @@ -253,6 +253,25 @@ export default class EppoClient { return this.getBooleanAssignment(flagKey, subjectKey, subjectAttributes, defaultValue); } + /** + * Maps a subject to a boolean variation for a given experiment. + * + * @param flagKey feature flag identifier + * @param subjectKey an identifier of the experiment subject, for example a user ID. + * @param subjectAttributes optional attributes associated with the subject, for example name and email. + * @param defaultValue default value to return if the subject is not part of the experiment sample + * @returns a boolean variation value if the subject is part of the experiment sample, otherwise the default value + */ + public getBooleanAssignment( + flagKey: string, + subjectKey: string, + subjectAttributes: Attributes, + defaultValue: boolean, + ): boolean { + return this.getBooleanAssignmentDetails(flagKey, subjectKey, subjectAttributes, defaultValue) + .variation; + } + /** * Maps a subject to a boolean variation for a given experiment and provides additional details about the * variation assigned and the reason for the assignment. @@ -637,25 +656,6 @@ export default class EppoClient { return result; } - /** - * Maps a subject to a boolean variation for a given experiment. - * - * @param flagKey feature flag identifier - * @param subjectKey an identifier of the experiment subject, for example a user ID. - * @param subjectAttributes optional attributes associated with the subject, for example name and email. - * @param defaultValue default value to return if the subject is not part of the experiment sample - * @returns a boolean variation value if the subject is part of the experiment sample, otherwise the default value - */ - public getBooleanAssignment( - flagKey: string, - subjectKey: string, - subjectAttributes: Attributes, - defaultValue: boolean, - ): boolean { - return this.getBooleanAssignmentDetails(flagKey, subjectKey, subjectAttributes, defaultValue) - .variation; - } - private ensureActionsWithContextualAttributes( actions: BanditActions, ): Record { From e5c85cce137dda7115ffb8719acf811814b25df2 Mon Sep 17 00:00:00 2001 From: selki Date: Tue, 12 Nov 2024 21:45:06 +0000 Subject: [PATCH 07/13] minor code cleanliness improvements --- src/cache/tlru-cache.ts | 36 +++++++++---------- .../tlru-in-memory-assignment-cache.spec.ts | 4 ++- src/cache/tlru-in-memory-assignment-cache.ts | 4 ++- src/constants.ts | 1 + 4 files changed, 25 insertions(+), 20 deletions(-) diff --git a/src/cache/tlru-cache.ts b/src/cache/tlru-cache.ts index 5ad27bf..fc04109 100644 --- a/src/cache/tlru-cache.ts +++ b/src/cache/tlru-cache.ts @@ -11,45 +11,45 @@ export class TLRUCache extends LRUCache { super(maxSize); } - private clearCacheEntryTimeout(key: string): void { - const timeoutId = this.cacheEntriesTimoutIds.get(key); - clearTimeout(timeoutId); - this.cacheEntriesTimoutIds.delete(key); + private clearCacheEntryTimeoutIfExists(key: string): void { + if (this.cacheEntriesTimoutIds.has(key)) { + const timeoutId = this.cacheEntriesTimoutIds.get(key); + clearTimeout(timeoutId); + this.cacheEntriesTimoutIds.delete(key); + } } - private setCacheEntryTimout(key: string): void { - const timeOutId = setTimeout(() => { + private setCacheEntryTimeout(key: string): void { + const timeoutId = setTimeout(() => { this.delete(key); }, this.ttl); - this.cacheEntriesTimoutIds.set(key, timeOutId); + this.cacheEntriesTimoutIds.set(key, timeoutId); } delete(key: string): boolean { - this.clearCacheEntryTimeout(key); + this.clearCacheEntryTimeoutIfExists(key); return super.delete(key); } get(key: string): string | undefined { const value = super.get(key); - // Whenever we get a cache hit, we need to reset the timer - // for eviction, because it is now considered most recently - // accessed thus the timer should start over. Not doing that - // will cause a de-sync that will stop proper eviction - this.clearCacheEntryTimeout(key); if (value) { - this.setCacheEntryTimout(key); + // Whenever we get a cache hit, we need to reset the timer + // for eviction, because it is now considered most recently + // accessed thus the timer should start over. Not doing that + // will cause a de-sync that will stop proper eviction + this.clearCacheEntryTimeoutIfExists(key); + this.setCacheEntryTimeout(key); } return value; } set(key: string, value: string): this { const cache = super.set(key, value); - if (this.cacheEntriesTimoutIds.has(key)) { - this.clearCacheEntryTimeout(key); - } - this.setCacheEntryTimout(key); + this.clearCacheEntryTimeoutIfExists(key); + this.setCacheEntryTimeout(key); return cache; } diff --git a/src/cache/tlru-in-memory-assignment-cache.spec.ts b/src/cache/tlru-in-memory-assignment-cache.spec.ts index c03d64e..886ca62 100644 --- a/src/cache/tlru-in-memory-assignment-cache.spec.ts +++ b/src/cache/tlru-in-memory-assignment-cache.spec.ts @@ -1,8 +1,10 @@ +import { DEFAULT_TLRU_TTL_MS } from '../constants'; + import { TLRUInMemoryAssignmentCache } from './tlru-in-memory-assignment-cache'; describe('ExpiringLRUInMemoryAssignmentCache', () => { let cache: TLRUInMemoryAssignmentCache; - const defaultTimout = 600_000; // 10 minutes + const defaultTimout = DEFAULT_TLRU_TTL_MS; // 10 minutes beforeAll(() => { jest.useFakeTimers(); diff --git a/src/cache/tlru-in-memory-assignment-cache.ts b/src/cache/tlru-in-memory-assignment-cache.ts index a17c2d7..854f76c 100644 --- a/src/cache/tlru-in-memory-assignment-cache.ts +++ b/src/cache/tlru-in-memory-assignment-cache.ts @@ -1,3 +1,5 @@ +import { DEFAULT_TLRU_TTL_MS } from '../constants'; + import { AbstractAssignmentCache } from './abstract-assignment-cache'; import { TLRUCache } from './tlru-cache'; @@ -11,7 +13,7 @@ import { TLRUCache } from './tlru-cache'; * @param {number} ttl - Time in milliseconds after cache will expire. */ export class TLRUInMemoryAssignmentCache extends AbstractAssignmentCache { - constructor(maxSize: number, ttl = 600_000) { + constructor(maxSize: number, ttl = DEFAULT_TLRU_TTL_MS) { super(new TLRUCache(maxSize, ttl)); } } diff --git a/src/constants.ts b/src/constants.ts index 4546722..f26a13b 100644 --- a/src/constants.ts +++ b/src/constants.ts @@ -12,3 +12,4 @@ export const NULL_SENTINEL = 'EPPO_NULL'; // number of logging events that may be queued while waiting for initialization export const MAX_EVENT_QUEUE_SIZE = 100; export const BANDIT_ASSIGNMENT_SHARDS = 10000; +export const DEFAULT_TLRU_TTL_MS = 600_000; From 4b02b3ee559b8b79f86e4a471076bd584959859d Mon Sep 17 00:00:00 2001 From: selki Date: Mon, 18 Nov 2024 13:21:09 +0000 Subject: [PATCH 08/13] memory optimization of tlru cache --- src/cache/abstract-assignment-cache.ts | 2 +- src/cache/lru-cache.ts | 4 +- src/cache/tlru-cache.spec.ts | 50 ++++++++++++++++----- src/cache/tlru-cache.ts | 61 +++++++++++++++++--------- 4 files changed, 84 insertions(+), 33 deletions(-) diff --git a/src/cache/abstract-assignment-cache.ts b/src/cache/abstract-assignment-cache.ts index 91298fd..118fbe2 100644 --- a/src/cache/abstract-assignment-cache.ts +++ b/src/cache/abstract-assignment-cache.ts @@ -58,7 +58,7 @@ export abstract class AbstractAssignmentCache> return this.get(entry) === assignmentCacheValueToString(entry); } - private get(key: AssignmentCacheKey): string | undefined { + get(key: AssignmentCacheKey): string | undefined { return this.delegate.get(assignmentCacheKeyToString(key)); } diff --git a/src/cache/lru-cache.ts b/src/cache/lru-cache.ts index d87d29c..507d905 100644 --- a/src/cache/lru-cache.ts +++ b/src/cache/lru-cache.ts @@ -12,12 +12,12 @@ * Source: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map */ export class LRUCache implements Map { - private readonly cache = new Map(); + protected readonly cache = new Map(); // eslint-disable-next-line @typescript-eslint/ban-ts-comment // @ts-ignore [Symbol.toStringTag]: string; - constructor(private readonly capacity: number) {} + constructor(protected readonly capacity: number) {} [Symbol.iterator](): IterableIterator<[string, string]> { return this.cache[Symbol.iterator](); diff --git a/src/cache/tlru-cache.spec.ts b/src/cache/tlru-cache.spec.ts index daa6ead..8772a94 100644 --- a/src/cache/tlru-cache.spec.ts +++ b/src/cache/tlru-cache.spec.ts @@ -13,15 +13,24 @@ describe('TLRU Cache', () => { jest.clearAllTimers(); }); - it('should evict cache after timeout', () => { + it('should evict cache after expiration', () => { jest.useFakeTimers(); - jest.spyOn(global, 'setTimeout'); cache.set('a', 'apple'); jest.advanceTimersByTime(expectedCacheTimeoutMs); - expect(setTimeout).toHaveBeenCalledTimes(1); - expect(setTimeout).toHaveBeenLastCalledWith(expect.any(Function), expectedCacheTimeoutMs); + expect(cache.get('a')).toBeUndefined(); + }); + + it('should evict all expired entries', () => { + jest.useFakeTimers(); + + cache.set('a', 'avocado'); + jest.advanceTimersByTime(expectedCacheTimeoutMs); + cache.set('b', 'banana'); + jest.advanceTimersByTime(expectedCacheTimeoutMs); + + expect(cache.get('b')).toBeUndefined(); expect(cache.get('a')).toBeUndefined(); }); @@ -31,16 +40,11 @@ describe('TLRU Cache', () => { **/ it('should overwrite existing cache entry', () => { jest.useFakeTimers(); - jest.spyOn(global, 'setTimeout'); - jest.spyOn(global, 'clearTimeout'); cache.set('a', 'apple'); jest.advanceTimersByTime(expectedCacheTimeoutMs - 1); cache.set('a', 'avocado'); - expect(setTimeout).toHaveBeenCalledTimes(2); - expect(setTimeout).toHaveBeenLastCalledWith(expect.any(Function), expectedCacheTimeoutMs); - expect(clearTimeout).toHaveBeenCalledTimes(1); // spin the clock by 5sec. After that time cache entry should be still valid. jest.advanceTimersByTime(expectedCacheTimeoutMs / 2); @@ -56,6 +60,32 @@ describe('TLRU Cache', () => { // after another spin of 5 sec, cache entry should evict itself jest.advanceTimersByTime(expectedCacheTimeoutMs / 2); - expect(cache.has('a')).toBeFalsy(); + expect(cache.get('a')).toBeUndefined(); + }); + + it('should check if a key exists', () => { + cache.set('a', 'apple'); + expect(cache.has('a')).toBeTruthy(); + expect(cache.has('b')).toBeFalsy(); + }); + + it('should handle the cache capacity of zero', () => { + const zeroCache = new TLRUCache(0, expectedCacheTimeoutMs); + zeroCache.set('a', 'apple'); + expect(zeroCache.get('a')).toBeFalsy(); + }); + + it('should handle the cache capacity of one', () => { + jest.useFakeTimers(); + const oneCache = new TLRUCache(1, expectedCacheTimeoutMs); + oneCache.set('a', 'apple'); + jest.advanceTimersByTime(expectedCacheTimeoutMs); + expect(oneCache.get('a')).toBeUndefined(); + + oneCache.set('a', 'avocado'); + expect(oneCache.get('a')).toBe('avocado'); + oneCache.set('b', 'banana'); + expect(oneCache.get('a')).toBeFalsy(); + expect(oneCache.get('b')).toBe('banana'); }); }); diff --git a/src/cache/tlru-cache.ts b/src/cache/tlru-cache.ts index fc04109..5d9ea6c 100644 --- a/src/cache/tlru-cache.ts +++ b/src/cache/tlru-cache.ts @@ -1,56 +1,77 @@ import { LRUCache } from './lru-cache'; /** - * Time-aware, least-recently-used (TLRU), variant of LRU where entries have valid lifetime. + * Time-aware, least-recently-used cache (TLRU). Variant of LRU where entries have valid lifetime. * @param {number} maxSize - Maximum cache size * @param {number} ttl - Time in milliseconds after which cache entry will evict itself + * @param {number} evictionInterval - Frequency of cache entries eviction check **/ export class TLRUCache extends LRUCache { - private readonly cacheEntriesTimoutIds = new Map>(); + private readonly cacheEntriesTTLRegistry = new Map(); constructor(readonly maxSize: number, readonly ttl: number) { super(maxSize); } - private clearCacheEntryTimeoutIfExists(key: string): void { - if (this.cacheEntriesTimoutIds.has(key)) { - const timeoutId = this.cacheEntriesTimoutIds.get(key); - clearTimeout(timeoutId); - this.cacheEntriesTimoutIds.delete(key); + private getCacheEntryEvictionTime(): Date { + return new Date(Date.now() + this.ttl); + } + + private clearCacheEntryEvictionTimeIfExists(key: string): void { + if (this.cacheEntriesTTLRegistry.has(key)) { + this.cacheEntriesTTLRegistry.delete(key); } } - private setCacheEntryTimeout(key: string): void { - const timeoutId = setTimeout(() => { - this.delete(key); - }, this.ttl); + private setCacheEntryEvictionTime(key: string): void { + this.cacheEntriesTTLRegistry.set(key, this.getCacheEntryEvictionTime()); + } - this.cacheEntriesTimoutIds.set(key, timeoutId); + private resetCacheEntryEvictionTime(key: string): void { + this.clearCacheEntryEvictionTimeIfExists(key); + this.setCacheEntryEvictionTime(key); + } + + private evictExpiredCacheEntries() { + const now = new Date(Date.now()); + let cacheKey: string; + let evictionDate: Date; + + // Not using this.cache.forEach so we can break the loop once + // we find the fist non-expired entry. Each entry after that + // is guaranteed to also be non-expired, because they are oldest->newest + for ([cacheKey, evictionDate] of this.cacheEntriesTTLRegistry.entries()) { + if (now >= evictionDate) { + this.delete(cacheKey); + } else { + break; + } + } } delete(key: string): boolean { - this.clearCacheEntryTimeoutIfExists(key); + this.clearCacheEntryEvictionTimeIfExists(key); return super.delete(key); } get(key: string): string | undefined { - const value = super.get(key); + this.evictExpiredCacheEntries(); - if (value) { + const value = super.get(key); + if (value !== undefined) { // Whenever we get a cache hit, we need to reset the timer // for eviction, because it is now considered most recently // accessed thus the timer should start over. Not doing that // will cause a de-sync that will stop proper eviction - this.clearCacheEntryTimeoutIfExists(key); - this.setCacheEntryTimeout(key); + this.resetCacheEntryEvictionTime(key); } return value; } set(key: string, value: string): this { - const cache = super.set(key, value); - this.clearCacheEntryTimeoutIfExists(key); - this.setCacheEntryTimeout(key); + this.evictExpiredCacheEntries(); + const cache = super.set(key, value); + this.resetCacheEntryEvictionTime(key); return cache; } } From 4bdfa7f6778cfdf08ba8004af26b34ae8f6f2243 Mon Sep 17 00:00:00 2001 From: selki Date: Mon, 18 Nov 2024 13:26:15 +0000 Subject: [PATCH 09/13] use better name for bandit cache method in EppoClient --- src/client/eppo-client.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/client/eppo-client.ts b/src/client/eppo-client.ts index 0c8cf76..26fec75 100644 --- a/src/client/eppo-client.ts +++ b/src/client/eppo-client.ts @@ -982,15 +982,11 @@ export default class EppoClient { this.banditAssignmentCache = new NonExpiringInMemoryAssignmentCache(); } - public useLRUInMemoryBanditAssignmentCache(maxSize: number) { - this.banditAssignmentCache = new TLRUInMemoryAssignmentCache(maxSize); - } - /** * @param {number} maxSize - Maximum cache size * @param {number} timeout - TTL of cache entries */ - public useTLRUInMemoryAssignmentCache(maxSize: number, timeout?: number) { + public useExpiringInMemoryBanditAssignmentCache(maxSize: number, timeout?: number) { this.banditAssignmentCache = new TLRUInMemoryAssignmentCache(maxSize, timeout); } From d4a14bd0ffdedf85d850643f55d9893f32cc6268 Mon Sep 17 00:00:00 2001 From: selki Date: Mon, 18 Nov 2024 13:29:23 +0000 Subject: [PATCH 10/13] fixed tests --- src/client/eppo-client-with-bandits.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/eppo-client-with-bandits.spec.ts b/src/client/eppo-client-with-bandits.spec.ts index 9a8ccc8..c32cba3 100644 --- a/src/client/eppo-client-with-bandits.spec.ts +++ b/src/client/eppo-client-with-bandits.spec.ts @@ -206,7 +206,7 @@ describe('EppoClient Bandits E2E test', () => { it('Flushed queued logging events when a logger is set', () => { client.useLRUInMemoryAssignmentCache(5); - client.useLRUInMemoryBanditAssignmentCache(5); + client.useExpiringInMemoryBanditAssignmentCache(5); client.setAssignmentLogger(null as unknown as IAssignmentLogger); client.setBanditLogger(null as unknown as IBanditLogger); const banditAssignment = client.getBanditAction( From 1cf739129df85ef38202f52d777006fcaab90782 Mon Sep 17 00:00:00 2001 From: selki Date: Thu, 21 Nov 2024 13:13:47 +0000 Subject: [PATCH 11/13] added more test for tlru cache, changed get, set, entries, keys, values behaviour --- src/cache/lru-cache.spec.ts | 15 ++++ ...on-expiring-in-memory-assignment-cache.ts} | 0 src/cache/tlru-cache.spec.ts | 82 +++++++++++++++++-- src/cache/tlru-cache.ts | 55 +++++++++++-- 4 files changed, 136 insertions(+), 16 deletions(-) rename src/cache/{abstract-assignment-cache.spec.ts => non-expiring-in-memory-assignment-cache.ts} (100%) diff --git a/src/cache/lru-cache.spec.ts b/src/cache/lru-cache.spec.ts index 9812a5e..b0e311a 100644 --- a/src/cache/lru-cache.spec.ts +++ b/src/cache/lru-cache.spec.ts @@ -61,4 +61,19 @@ describe('LRUCache', () => { expect(oneCache.get('a')).toBeFalsy(); expect(oneCache.get('b')).toBe('banana'); }); + + /** + This test case might be an overkill but in case Map() changes, + or we want to ditch it completely this will remind us that insertion + order is crucial for this cache to work properly + **/ + it('should preserve insertion order when inserting on capacity limit', () => { + cache.set('a', 'apple'); + cache.set('b', 'banana'); + cache.set('c', 'cherry'); + + const keys = Array.from(cache.keys()); + expect(keys[0]).toBe('b'); + expect(keys[1]).toBe('c'); + }); }); diff --git a/src/cache/abstract-assignment-cache.spec.ts b/src/cache/non-expiring-in-memory-assignment-cache.ts similarity index 100% rename from src/cache/abstract-assignment-cache.spec.ts rename to src/cache/non-expiring-in-memory-assignment-cache.ts diff --git a/src/cache/tlru-cache.spec.ts b/src/cache/tlru-cache.spec.ts index 8772a94..99666cb 100644 --- a/src/cache/tlru-cache.spec.ts +++ b/src/cache/tlru-cache.spec.ts @@ -22,7 +22,15 @@ describe('TLRU Cache', () => { expect(cache.get('a')).toBeUndefined(); }); - it('should evict all expired entries', () => { + it('should not evict cache before expiration', () => { + jest.useFakeTimers(); + + cache.set('a', 'apple'); + jest.advanceTimersByTime(expectedCacheTimeoutMs - 1); + expect(cache.get('a')).toBe('apple'); + }); + + it('should evict all expired entries on .entries() call', () => { jest.useFakeTimers(); cache.set('a', 'avocado'); @@ -30,14 +38,49 @@ describe('TLRU Cache', () => { cache.set('b', 'banana'); jest.advanceTimersByTime(expectedCacheTimeoutMs); - expect(cache.get('b')).toBeUndefined(); - expect(cache.get('a')).toBeUndefined(); + const cacheEntries = []; + + for (const entry of cache.entries()) { + cacheEntries.push(entry); + } + + expect(cacheEntries.length).toBe(0); + }); + + it('should evict all expired entries on .keys() call', () => { + jest.useFakeTimers(); + + cache.set('a', 'avocado'); + jest.advanceTimersByTime(expectedCacheTimeoutMs); + cache.set('b', 'banana'); + jest.advanceTimersByTime(expectedCacheTimeoutMs); + + const cacheKeys = []; + + for (const key of cache.keys()) { + cacheKeys.push(key); + } + + expect(cacheKeys.length).toBe(0); + }); + + it('should evict all expired entries on .values() call', () => { + jest.useFakeTimers(); + + cache.set('a', 'avocado'); + jest.advanceTimersByTime(expectedCacheTimeoutMs); + cache.set('b', 'banana'); + jest.advanceTimersByTime(expectedCacheTimeoutMs); + + const cacheValues = []; + + for (const value of cache.values()) { + cacheValues.push(value); + } + + expect(cacheValues.length).toBe(0); }); - /** - * This test assumes implementation which is not ideal, but that's - * the only way I know of how to go around timers in jest - **/ it('should overwrite existing cache entry', () => { jest.useFakeTimers(); @@ -88,4 +131,29 @@ describe('TLRU Cache', () => { expect(oneCache.get('a')).toBeFalsy(); expect(oneCache.get('b')).toBe('banana'); }); + + it('should evict oldest entry when capacity limit is reached', () => { + cache.set('a', 'apple'); + cache.set('b', 'banana'); + cache.set('c', 'cherry'); + + expect(cache.get('a')).toBeUndefined(); + expect(cache.has('b')).toBeTruthy(); + expect(cache.has('c')).toBeTruthy(); + }); + + /** + This test case might be an overkill but in case Map() changes, + or we want to ditch it completely this will remind us that insertion + order is crucial for this cache to work properly + **/ + it('should preserve insertion order when inserting on capacity limit', () => { + cache.set('a', 'apple'); + cache.set('b', 'banana'); + cache.set('c', 'cherry'); + + const keys = Array.from(cache.keys()); + expect(keys[0]).toBe('b'); + expect(keys[1]).toBe('c'); + }); }); diff --git a/src/cache/tlru-cache.ts b/src/cache/tlru-cache.ts index 5d9ea6c..d5a258e 100644 --- a/src/cache/tlru-cache.ts +++ b/src/cache/tlru-cache.ts @@ -22,6 +22,12 @@ export class TLRUCache extends LRUCache { } } + private isCacheEntryValid(key: string): boolean { + const now = new Date(Date.now()); + const evictionDate = this.cacheEntriesTTLRegistry.get(key); + return evictionDate !== undefined ? now < evictionDate : false; + } + private setCacheEntryEvictionTime(key: string): void { this.cacheEntriesTTLRegistry.set(key, this.getCacheEntryEvictionTime()); } @@ -32,15 +38,14 @@ export class TLRUCache extends LRUCache { } private evictExpiredCacheEntries() { - const now = new Date(Date.now()); let cacheKey: string; - let evictionDate: Date; // Not using this.cache.forEach so we can break the loop once // we find the fist non-expired entry. Each entry after that - // is guaranteed to also be non-expired, because they are oldest->newest - for ([cacheKey, evictionDate] of this.cacheEntriesTTLRegistry.entries()) { - if (now >= evictionDate) { + // is guaranteed to also be non-expired, because iteration happens + // in insertion order + for (cacheKey of this.cache.keys()) { + if (!this.isCacheEntryValid(cacheKey)) { this.delete(cacheKey); } else { break; @@ -48,16 +53,50 @@ export class TLRUCache extends LRUCache { } } + entries(): IterableIterator<[string, string]> { + this.evictExpiredCacheEntries(); + return super.entries(); + } + + keys(): IterableIterator { + this.evictExpiredCacheEntries(); + return super.keys(); + } + + values(): IterableIterator { + this.evictExpiredCacheEntries(); + return super.values(); + } + delete(key: string): boolean { this.clearCacheEntryEvictionTimeIfExists(key); return super.delete(key); } + // has(key: string): boolean { + // const hasValue = this.cache.has(key); + // + // if (!this.isCacheEntryValid(key)) { + // this.delete(key); + // return false; + // } + // + // return hasValue; + // } + get(key: string): string | undefined { - this.evictExpiredCacheEntries(); + if (!this.cache.has(key)) { + return undefined; + } + + const value = this.cache.get(key); - const value = super.get(key); if (value !== undefined) { + if (!this.isCacheEntryValid(key)) { + this.delete(key); + return undefined; + } + // Whenever we get a cache hit, we need to reset the timer // for eviction, because it is now considered most recently // accessed thus the timer should start over. Not doing that @@ -68,8 +107,6 @@ export class TLRUCache extends LRUCache { } set(key: string, value: string): this { - this.evictExpiredCacheEntries(); - const cache = super.set(key, value); this.resetCacheEntryEvictionTime(key); return cache; From 64c672939644303f63c3481750c0476831503956 Mon Sep 17 00:00:00 2001 From: selki Date: Fri, 22 Nov 2024 12:22:45 +0000 Subject: [PATCH 12/13] fixed get() behaviour of tlru cache --- src/cache/tlru-cache.spec.ts | 22 +++++++++++++++++++--- src/cache/tlru-cache.ts | 28 ++++++++++------------------ 2 files changed, 29 insertions(+), 21 deletions(-) diff --git a/src/cache/tlru-cache.spec.ts b/src/cache/tlru-cache.spec.ts index 99666cb..b4038a6 100644 --- a/src/cache/tlru-cache.spec.ts +++ b/src/cache/tlru-cache.spec.ts @@ -50,10 +50,12 @@ describe('TLRU Cache', () => { it('should evict all expired entries on .keys() call', () => { jest.useFakeTimers(); + cache = new TLRUCache(3, expectedCacheTimeoutMs); cache.set('a', 'avocado'); jest.advanceTimersByTime(expectedCacheTimeoutMs); cache.set('b', 'banana'); jest.advanceTimersByTime(expectedCacheTimeoutMs); + cache.set('c', 'cherry'); const cacheKeys = []; @@ -61,16 +63,19 @@ describe('TLRU Cache', () => { cacheKeys.push(key); } - expect(cacheKeys.length).toBe(0); + expect(cacheKeys.length).toBe(1); + expect(cache.get('c')).toBe('cherry'); }); it('should evict all expired entries on .values() call', () => { jest.useFakeTimers(); + cache = new TLRUCache(3, expectedCacheTimeoutMs); cache.set('a', 'avocado'); jest.advanceTimersByTime(expectedCacheTimeoutMs); cache.set('b', 'banana'); jest.advanceTimersByTime(expectedCacheTimeoutMs); + cache.set('c', 'cherry'); const cacheValues = []; @@ -78,7 +83,8 @@ describe('TLRU Cache', () => { cacheValues.push(value); } - expect(cacheValues.length).toBe(0); + expect(cacheValues.length).toBe(1); + expect(cache.get('c')).toBe('cherry'); }); it('should overwrite existing cache entry', () => { @@ -152,8 +158,18 @@ describe('TLRU Cache', () => { cache.set('b', 'banana'); cache.set('c', 'cherry'); - const keys = Array.from(cache.keys()); + let keys = Array.from(cache.keys()); expect(keys[0]).toBe('b'); expect(keys[1]).toBe('c'); + + cache = new TLRUCache(2, expectedCacheTimeoutMs); + cache.set('a', 'apple'); + cache.set('b', 'banana'); + cache.get('a'); + cache.set('c', 'cherry'); + + keys = Array.from(cache.keys()); + expect(keys[0]).toBe('a'); + expect(keys[1]).toBe('c'); }); }); diff --git a/src/cache/tlru-cache.ts b/src/cache/tlru-cache.ts index d5a258e..c16f14e 100644 --- a/src/cache/tlru-cache.ts +++ b/src/cache/tlru-cache.ts @@ -73,30 +73,22 @@ export class TLRUCache extends LRUCache { return super.delete(key); } - // has(key: string): boolean { - // const hasValue = this.cache.has(key); - // - // if (!this.isCacheEntryValid(key)) { - // this.delete(key); - // return false; - // } - // - // return hasValue; - // } + has(key: string): boolean { + if (!this.isCacheEntryValid(key)) { + this.delete(key); + return false; + } + return this.cache.has(key); + } get(key: string): string | undefined { - if (!this.cache.has(key)) { + if (!this.isCacheEntryValid(key)) { + this.delete(key); return undefined; } - const value = this.cache.get(key); - + const value = super.get(key); if (value !== undefined) { - if (!this.isCacheEntryValid(key)) { - this.delete(key); - return undefined; - } - // Whenever we get a cache hit, we need to reset the timer // for eviction, because it is now considered most recently // accessed thus the timer should start over. Not doing that From 1d1f7ee616f544e63620de38c3b3e8f4c6962596 Mon Sep 17 00:00:00 2001 From: selki Date: Mon, 25 Nov 2024 14:04:32 +0000 Subject: [PATCH 13/13] sdk version bump --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 2574db8..1e1f4fd 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@eppo/js-client-sdk-common", - "version": "4.3.0", + "version": "4.4.0", "description": "Eppo SDK for client-side JavaScript applications (base for both web and react native)", "main": "dist/index.js", "files": [