fix(server): restore absolute TTL in PromiseMemoizer (#21480)
## Context `PromiseMemoizer` sits in front of the staged lookup (local cache → Redis hash validation → Redis data → DB recompute) of both `CoreEntityCacheService` and `WorkspaceCacheService` (10s TTL each). The Redis hash check is the **only** cross-instance invalidation mechanism — there is no pub/sub — and it runs only when the memo entry expires. The TTL is currently **sliding**: every read refreshes `lastUsed`, and eviction compares against time-since-last-read. So any entry read more often than every 10s on a given instance never revalidates, and that instance serves stale data for as long as traffic continues. Affected data: auth-context entities (workspace, user, userWorkspace), API key revocations, role/permission maps, RLS predicates, feature flags, and all metadata maps. Observed manifestation: after `activateWorkspace`, a sibling instance kept serving a `PENDING_CREATION` workspace snapshot (kept alive indefinitely by the client's own polling), stranding signup on a permanent loading skeleton at `/create/profile` (#21461). Same staleness class as #20322 and the CI flakes investigated in #21435. ## Why it was sliding #11444 (April 2025) deliberately changed the TTL from absolute to sliding because the memoizer's then-consumer was the TypeORM datasource storage: absolute expiry was destroying datasources that were actively in use (`onDelete` → `destroy()`), causing worker `Connection terminated` errors. That consumer no longer exists — datasources moved to `GlobalWorkspaceOrmManager`, and neither remaining consumer passes `onDelete` or holds resources needing keep-alive. ## Fix Restore absolute expiry: `expiresAt` is set at write time and never refreshed on read. Every instance now re-enters the staged lookup (and thus the Redis hash validation) at least once per TTL, restoring the designed ≤10s cross-instance staleness ceiling. Concurrent dedup (`pending` map) and `onDelete` plumbing are unchanged. ## Test plan - New regression test: reads at half-TTL intervals must not extend an entry's lifetime (fails on the sliding implementation, passes now). - Full `promise-memoizer.storage.spec.ts` and `workspace-cache.service.spec.ts` suites pass (25 tests); lint and format clean. <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/twentyhq/twenty/pull/21480?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. -->
This commit is contained in:
+23
@@ -83,6 +83,29 @@ describe('PromiseMemoizer', () => {
|
||||
expect(mockFactory).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it('should re-execute factory after TTL expires even under continuous reads', async () => {
|
||||
mockFactory.mockResolvedValue('test-value');
|
||||
|
||||
const startTime = Date.now();
|
||||
|
||||
await memoizer.memoizePromiseAndExecute('test-key-1', mockFactory);
|
||||
|
||||
for (const elapsedTime of [
|
||||
TTL_MS / 2,
|
||||
TTL_MS,
|
||||
(3 * TTL_MS) / 2,
|
||||
2 * TTL_MS,
|
||||
]) {
|
||||
jest
|
||||
.spyOn(global.Date, 'now')
|
||||
.mockImplementation(() => startTime + elapsedTime + 1);
|
||||
|
||||
await memoizer.memoizePromiseAndExecute('test-key-1', mockFactory);
|
||||
}
|
||||
|
||||
expect(mockFactory).toHaveBeenCalledTimes(3);
|
||||
});
|
||||
|
||||
it('should handle null values', async () => {
|
||||
mockFactory.mockResolvedValue(null);
|
||||
|
||||
|
||||
@@ -8,7 +8,7 @@ type AsyncFactoryCallback<T> = () => Promise<T | null>;
|
||||
const ONE_HOUR_IN_MS = 3600_000;
|
||||
|
||||
export class PromiseMemoizer<T> {
|
||||
private cache = new Map<CacheKey, { value: T; lastUsed: number }>();
|
||||
private cache = new Map<CacheKey, { value: T; expiresAt: number }>();
|
||||
private pending = new Map<CacheKey, Promise<T | null>>();
|
||||
private ttlMs: number;
|
||||
|
||||
@@ -26,8 +26,6 @@ export class PromiseMemoizer<T> {
|
||||
const cachedEntry = this.cache.get(cacheKey);
|
||||
|
||||
if (cachedEntry) {
|
||||
cachedEntry.lastUsed = Date.now();
|
||||
|
||||
return cachedEntry.value;
|
||||
}
|
||||
|
||||
@@ -42,7 +40,10 @@ export class PromiseMemoizer<T> {
|
||||
const value = await factory();
|
||||
|
||||
if (value) {
|
||||
this.cache.set(cacheKey, { value, lastUsed: Date.now() });
|
||||
this.cache.set(cacheKey, {
|
||||
value,
|
||||
expiresAt: Date.now() + this.ttlMs,
|
||||
});
|
||||
}
|
||||
|
||||
return value;
|
||||
@@ -60,7 +61,7 @@ export class PromiseMemoizer<T> {
|
||||
const now = Date.now();
|
||||
|
||||
for (const [cacheKey, cachedEntry] of this.cache.entries()) {
|
||||
if (cachedEntry.lastUsed < now - this.ttlMs) {
|
||||
if (cachedEntry.expiresAt <= now) {
|
||||
await this.clearKey(cacheKey, onDelete);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user