From ef04b09f75058cce47e0b35eebef533e82772b2d Mon Sep 17 00:00:00 2001 From: Eric Peterson Date: Fri, 12 Nov 2021 15:01:37 +0100 Subject: [PATCH] Make cache readTimeout configurable. Signed-off-by: Eric Peterson --- docs/features/techdocs/configuration.md | 6 +++++ plugins/techdocs-backend/config.d.ts | 9 +++++++ .../src/cache/TechDocsCache.test.ts | 27 +++++++++++++++++-- .../src/cache/TechDocsCache.ts | 26 ++++++++++++++++-- .../techdocs-backend/src/service/router.ts | 2 +- 5 files changed, 65 insertions(+), 5 deletions(-) diff --git a/docs/features/techdocs/configuration.md b/docs/features/techdocs/configuration.md index 3234cdc034..983a8dd3e7 100644 --- a/docs/features/techdocs/configuration.md +++ b/docs/features/techdocs/configuration.md @@ -144,6 +144,12 @@ techdocs: # to storage using the techdocs-cli, allowing long TTLs (e.g. 1 month/year) ttl: 3600000 + # (Optional) The time (in milliseconds) that the TechDocs backend will wait + # for a cache service to respond before continuing on as though the cached + # object was not found (e.g. when the cache sercice is unavailable). The + # default value is 1000 + readTimeout: 500 + # (Optional and Legacy) TechDocs makes API calls to techdocs-backend using this URL. e.g. get docs of an entity, get metadata, etc. # You don't have to specify this anymore. diff --git a/plugins/techdocs-backend/config.d.ts b/plugins/techdocs-backend/config.d.ts index 1767120a35..8d531e0536 100644 --- a/plugins/techdocs-backend/config.d.ts +++ b/plugins/techdocs-backend/config.d.ts @@ -241,6 +241,15 @@ export interface Config { * createRouter method in your backend. */ ttl: number; + + /** + * The time (in milliseconds) that the TechDocs backend will wait for + * a cache service to respond before continuing on as though the cached + * object was not found (e.g. when the cache sercice is unavailable). + * + * Defaults to 1000 milliseconds. + */ + readTimeout?: number; }; /** diff --git a/plugins/techdocs-backend/src/cache/TechDocsCache.test.ts b/plugins/techdocs-backend/src/cache/TechDocsCache.test.ts index e4308b7c3b..17056fc442 100644 --- a/plugins/techdocs-backend/src/cache/TechDocsCache.test.ts +++ b/plugins/techdocs-backend/src/cache/TechDocsCache.test.ts @@ -15,6 +15,7 @@ */ import { CacheClient, getVoidLogger } from '@backstage/backend-common'; +import { ConfigReader } from '@backstage/config'; import { CacheInvalidationError, TechDocsCache } from './TechDocsCache'; const cached = (str: string): string => { @@ -31,7 +32,7 @@ describe('TechDocsCache', () => { set: jest.fn(), delete: jest.fn(), }; - CacheUnderTest = new TechDocsCache({ + CacheUnderTest = TechDocsCache.fromConfig(new ConfigReader({}), { cache: MockClient, logger: getVoidLogger(), }); @@ -55,7 +56,7 @@ describe('TechDocsCache', () => { expect(actual).toBe(undefined); }); - it('returns undefined if no response after 1s', async () => { + it('returns undefined if no response after 1s by default', async () => { const expectedPath = 'some/index.html'; MockClient.get.mockImplementationOnce(() => { return new Promise(resolve => { @@ -66,6 +67,28 @@ describe('TechDocsCache', () => { expect(actual).toBe(undefined); }); + it('returns undefined if no response after configured readTimeout', async () => { + const expectedPath = 'some/index.html'; + MockClient.get.mockImplementationOnce(() => { + return new Promise(resolve => { + setTimeout(() => resolve(cached('value')), 20); + }); + }); + + CacheUnderTest = TechDocsCache.fromConfig( + new ConfigReader({ + techdocs: { cache: { readTimeout: 10 } }, + }), + { + cache: MockClient, + logger: getVoidLogger(), + }, + ); + + const actual = await CacheUnderTest.get(expectedPath); + expect(actual).toBe(undefined); + }); + it('returns data if cache get returns it', async () => { const expectedPath = 'some/index.html'; MockClient.get.mockResolvedValueOnce(cached('expected value')); diff --git a/plugins/techdocs-backend/src/cache/TechDocsCache.ts b/plugins/techdocs-backend/src/cache/TechDocsCache.ts index 83497608cf..b12142dda5 100644 --- a/plugins/techdocs-backend/src/cache/TechDocsCache.ts +++ b/plugins/techdocs-backend/src/cache/TechDocsCache.ts @@ -14,6 +14,8 @@ * limitations under the License. */ import { CacheClient } from '@backstage/backend-common'; +import { assertError } from '@backstage/errors'; +import { Config } from '@backstage/config'; import { Logger } from 'winston'; export class CacheInvalidationError extends Error { @@ -28,10 +30,29 @@ export class CacheInvalidationError extends Error { export class TechDocsCache { protected readonly cache: CacheClient; protected readonly logger: Logger; + protected readonly readTimeout: number; - constructor({ cache, logger }: { cache: CacheClient; logger: Logger }) { + private constructor({ + cache, + logger, + readTimeout, + }: { + cache: CacheClient; + logger: Logger; + readTimeout: number; + }) { this.cache = cache; this.logger = logger; + this.readTimeout = readTimeout; + } + + static fromConfig( + config: Config, + { cache, logger }: { cache: CacheClient; logger: Logger }, + ) { + const readTimeout = + config.getOptionalNumber('techdocs.cache.readTimeout') || 1000; + return new TechDocsCache({ cache, logger, readTimeout }); } async get(path: string): Promise { @@ -40,7 +61,7 @@ export class TechDocsCache { // temporarily unreachable. const response = (await Promise.race([ this.cache.get(path), - new Promise(cancelAfter => setTimeout(cancelAfter, 1000)), + new Promise(cancelAfter => setTimeout(cancelAfter, this.readTimeout)), ])) as string | undefined; if (response !== undefined) { @@ -51,6 +72,7 @@ export class TechDocsCache { this.logger.debug(`Cache miss: ${path}`); return response; } catch (e) { + assertError(e); this.logger.warn(`Error getting cache entry ${path}: ${e.message}`); this.logger.debug(e.stack); return undefined; diff --git a/plugins/techdocs-backend/src/service/router.ts b/plugins/techdocs-backend/src/service/router.ts index ba3fc6ea87..8f48342683 100644 --- a/plugins/techdocs-backend/src/service/router.ts +++ b/plugins/techdocs-backend/src/service/router.ts @@ -91,7 +91,7 @@ export async function createRouter( const defaultTtl = config.getOptionalNumber('techdocs.cache.ttl'); if (isOutOfTheBoxOption(options) && options.cache && defaultTtl) { const cacheClient = options.cache.getClient({ defaultTtl }); - cache = new TechDocsCache({ cache: cacheClient, logger }); + cache = TechDocsCache.fromConfig(config, { cache: cacheClient, logger }); } const scmIntegrations = ScmIntegrations.fromConfig(config);