Skip to content

Commit

Permalink
feat(cache): Fallback to older decorator results on error (#20795)
Browse files Browse the repository at this point in the history
  • Loading branch information
zharinov authored Mar 23, 2023
1 parent 49d6230 commit 3e28c4e
Show file tree
Hide file tree
Showing 4 changed files with 171 additions and 15 deletions.
115 changes: 109 additions & 6 deletions lib/util/cache/package/decorator.spec.ts
Original file line number Diff line number Diff line change
@@ -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 '.';
Expand Down Expand Up @@ -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
);
});
Expand Down Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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
);
});
Expand All @@ -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<string> {
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');
});
});
});
60 changes: 51 additions & 9 deletions lib/util/cache/package/decorator.ts
Original file line number Diff line number Diff line change
@@ -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<T extends any[] = any[]> = (...args: T) => string;
Expand Down Expand Up @@ -42,7 +46,7 @@ export function cache<T>({
cacheable = () => true,
ttlMinutes = 30,
}: CacheParameters): Decorator<T> {
return decorate(async ({ args, instance, callback }) => {
return decorate(async ({ args, instance, callback, methodName }) => {
if (!cacheable.apply(instance, args)) {
return callback();
}
Expand All @@ -66,21 +70,59 @@ export function cache<T>({
return callback();
}

const cachedResult = await packageCache.get<unknown>(
finalKey = `cache-decorator:${finalKey}`;
const oldRecord = await packageCache.get<DecoratorCachedRecord>(
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;
});
}
5 changes: 5 additions & 0 deletions lib/util/cache/package/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,8 @@ export interface PackageCache {

cleanup?(): Promise<void>;
}

export interface DecoratorCachedRecord {
value: unknown;
cachedAt: string;
}
6 changes: 6 additions & 0 deletions lib/util/decorator/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,11 @@ export interface DecoratorParameters<T, U extends any[] = any[]> {
* Current call context.
*/
instance: T;

/**
* The decorated method name.
*/
methodName?: string;
}

/**
Expand All @@ -48,6 +53,7 @@ export function decorate<T>(fn: Handler<T>): Decorator<T> {
args,
instance: this,
callback: () => value?.apply(this, args),
methodName: value?.name,
});
},
});
Expand Down

0 comments on commit 3e28c4e

Please sign in to comment.