From 3e28c4ee9fc01e74617b4dcbb7b181f6ca9086a2 Mon Sep 17 00:00:00 2001 From: Sergei Zharinov Date: Thu, 23 Mar 2023 18:59:51 +0300 Subject: [PATCH] feat(cache): Fallback to older decorator results on error (#20795) --- lib/util/cache/package/decorator.spec.ts | 115 +++++++++++++++++++++-- lib/util/cache/package/decorator.ts | 60 ++++++++++-- lib/util/cache/package/types.ts | 5 + lib/util/decorator/index.ts | 6 ++ 4 files changed, 171 insertions(+), 15 deletions(-) diff --git a/lib/util/cache/package/decorator.spec.ts b/lib/util/cache/package/decorator.spec.ts index 1501b13098997c..d6b6f144f42e04 100644 --- a/lib/util/cache/package/decorator.spec.ts +++ b/lib/util/cache/package/decorator.spec.ts @@ -1,4 +1,5 @@ import os from 'os'; +import { GlobalConfig } from '../../../config/global'; import * as memCache from '../memory'; import { cache } from './decorator'; import * as packageCache from '.'; @@ -37,8 +38,8 @@ describe('util/cache/package/decorator', () => { expect(getValue).toHaveBeenCalledTimes(1); expect(setCache).toHaveBeenCalledOnceWith( 'some-namespace', - 'some-key', - '111', + 'cache-decorator:some-key', + { cachedAt: expect.any(String), value: '111' }, 30 ); }); @@ -75,7 +76,12 @@ describe('util/cache/package/decorator', () => { expect(await obj.fn(null)).toBeNull(); expect(getValue).toHaveBeenCalledTimes(1); - expect(setCache).toHaveBeenCalledOnceWith('namespace', 'key', null, 30); + expect(setCache).toHaveBeenCalledOnceWith( + 'namespace', + 'cache-decorator:key', + { cachedAt: expect.any(String), value: null }, + 30 + ); }); it('does not cache undefined', async () => { @@ -120,8 +126,8 @@ describe('util/cache/package/decorator', () => { expect(getValue).toHaveBeenCalledTimes(1); expect(setCache).toHaveBeenCalledOnceWith( 'some-namespace', - 'some-key', - '111', + 'cache-decorator:some-key', + { cachedAt: expect.any(String), value: '111' }, 30 ); }); @@ -140,6 +146,103 @@ describe('util/cache/package/decorator', () => { expect(await fn.value?.()).toBe('111'); expect(getValue).toHaveBeenCalledTimes(1); - expect(setCache).toHaveBeenCalledOnceWith('namespace', 'key', '111', 30); + expect(setCache).toHaveBeenCalledOnceWith( + 'namespace', + 'cache-decorator:key', + { cachedAt: expect.any(String), value: '111' }, + 30 + ); + }); + + describe('Fallbacks with hard TTL', () => { + class Class { + @cache({ + namespace: 'namespace', + key: 'key', + ttlMinutes: 1, + }) + + // Hard TTL is enabled only for `getReleases` and `getDigest` methods + public getReleases(): Promise { + return getValue(); + } + } + + beforeEach(() => { + jest.useFakeTimers({ advanceTimers: false }); + GlobalConfig.set({ cacheHardTtlMinutes: 2 }); + }); + + afterEach(() => { + jest.useRealTimers(); + GlobalConfig.reset(); + }); + + it('updates cached result', async () => { + const obj = new Class(); + + expect(await obj.getReleases()).toBe('111'); + expect(getValue).toHaveBeenCalledTimes(1); + + jest.advanceTimersByTime(60 * 1000 - 1); + expect(await obj.getReleases()).toBe('111'); + expect(getValue).toHaveBeenCalledTimes(1); + expect(setCache).toHaveBeenLastCalledWith( + 'namespace', + 'cache-decorator:key', + { cachedAt: expect.any(String), value: '111' }, + 2 + ); + + jest.advanceTimersByTime(1); + expect(await obj.getReleases()).toBe('222'); + expect(getValue).toHaveBeenCalledTimes(2); + expect(setCache).toHaveBeenLastCalledWith( + 'namespace', + 'cache-decorator:key', + { cachedAt: expect.any(String), value: '222' }, + 2 + ); + }); + + it('returns obsolete result on error', async () => { + const obj = new Class(); + + expect(await obj.getReleases()).toBe('111'); + expect(getValue).toHaveBeenCalledTimes(1); + expect(setCache).toHaveBeenLastCalledWith( + 'namespace', + 'cache-decorator:key', + { cachedAt: expect.any(String), value: '111' }, + 2 + ); + + jest.advanceTimersByTime(60 * 1000); + getValue.mockRejectedValueOnce(new Error('test')); + expect(await obj.getReleases()).toBe('111'); + expect(getValue).toHaveBeenCalledTimes(2); + expect(setCache).toHaveBeenCalledTimes(1); + }); + + it('drops obsolete value after hard TTL is out', async () => { + const obj = new Class(); + + expect(await obj.getReleases()).toBe('111'); + expect(getValue).toHaveBeenCalledTimes(1); + expect(setCache).toHaveBeenLastCalledWith( + 'namespace', + 'cache-decorator:key', + { cachedAt: expect.any(String), value: '111' }, + 2 + ); + + jest.advanceTimersByTime(2 * 60 * 1000 - 1); + getValue.mockRejectedValueOnce(new Error('test')); + expect(await obj.getReleases()).toBe('111'); + + jest.advanceTimersByTime(1); + getValue.mockRejectedValueOnce(new Error('test')); + await expect(obj.getReleases()).rejects.toThrow('test'); + }); }); }); diff --git a/lib/util/cache/package/decorator.ts b/lib/util/cache/package/decorator.ts index 492dae49e113a5..287d38a5a6f1b7 100644 --- a/lib/util/cache/package/decorator.ts +++ b/lib/util/cache/package/decorator.ts @@ -1,5 +1,9 @@ import is from '@sindresorhus/is'; +import { DateTime } from 'luxon'; +import { GlobalConfig } from '../../../config/global'; +import { logger } from '../../../logger'; import { Decorator, decorate } from '../../decorator'; +import type { DecoratorCachedRecord } from './types'; import * as packageCache from '.'; type HashFunction = (...args: T) => string; @@ -42,7 +46,7 @@ export function cache({ cacheable = () => true, ttlMinutes = 30, }: CacheParameters): Decorator { - return decorate(async ({ args, instance, callback }) => { + return decorate(async ({ args, instance, callback, methodName }) => { if (!cacheable.apply(instance, args)) { return callback(); } @@ -66,21 +70,59 @@ export function cache({ return callback(); } - const cachedResult = await packageCache.get( + finalKey = `cache-decorator:${finalKey}`; + const oldRecord = await packageCache.get( finalNamespace, finalKey ); - if (cachedResult !== undefined) { - return cachedResult; + const softTtl = ttlMinutes; + + const cacheHardTtlMinutes = GlobalConfig.get().cacheHardTtlMinutes ?? 0; + let hardTtl = softTtl; + if (methodName === 'getReleases' || methodName === 'getDigest') { + hardTtl = Math.max(softTtl, cacheHardTtlMinutes); } - const result = await callback(); + let oldData: unknown; + if (oldRecord) { + const now = DateTime.local(); + const cachedAt = DateTime.fromISO(oldRecord.cachedAt); + + const softDeadline = cachedAt.plus({ minutes: softTtl }); + if (now < softDeadline) { + return oldRecord.value; + } + + const hardDeadline = cachedAt.plus({ minutes: hardTtl }); + if (now < hardDeadline) { + oldData = oldRecord.value; + } + } - // only cache if we got a valid result - if (result !== undefined) { - await packageCache.set(finalNamespace, finalKey, result, ttlMinutes); + let newData: unknown; + if (oldData) { + try { + newData = (await callback()) as T | undefined; + } catch (err) { + logger.debug( + { err }, + 'Package cache decorator: callback error, returning old data' + ); + return oldData; + } + } else { + newData = (await callback()) as T | undefined; } - return result; + + if (!is.undefined(newData)) { + const newRecord: DecoratorCachedRecord = { + cachedAt: DateTime.local().toISO(), + value: newData, + }; + await packageCache.set(finalNamespace, finalKey, newRecord, hardTtl); + } + + return newData; }); } diff --git a/lib/util/cache/package/types.ts b/lib/util/cache/package/types.ts index d345b6d4d2670f..65383ab9d6d58b 100644 --- a/lib/util/cache/package/types.ts +++ b/lib/util/cache/package/types.ts @@ -10,3 +10,8 @@ export interface PackageCache { cleanup?(): Promise; } + +export interface DecoratorCachedRecord { + value: unknown; + cachedAt: string; +} diff --git a/lib/util/decorator/index.ts b/lib/util/decorator/index.ts index 45f4ad2075286d..87bd71aa3c0e85 100644 --- a/lib/util/decorator/index.ts +++ b/lib/util/decorator/index.ts @@ -23,6 +23,11 @@ export interface DecoratorParameters { * Current call context. */ instance: T; + + /** + * The decorated method name. + */ + methodName?: string; } /** @@ -48,6 +53,7 @@ export function decorate(fn: Handler): Decorator { args, instance: this, callback: () => value?.apply(this, args), + methodName: value?.name, }); }, });