From 73b0ddc29608c59d2d6ad745aa0ffec0868fdf0f Mon Sep 17 00:00:00 2001 From: Wan Qi Chen <495709+wa0x6e@users.noreply.github.com> Date: Mon, 30 Oct 2023 14:17:28 +0700 Subject: [PATCH 1/3] refactor: extract cache logic from getScores --- src/helpers/cache.ts | 18 ++++++++++++++++++ src/scores.test.ts | 43 ++++++++----------------------------------- src/scores.ts | 42 +++++++++++++++++------------------------- 3 files changed, 43 insertions(+), 60 deletions(-) diff --git a/src/helpers/cache.ts b/src/helpers/cache.ts index e6e91cda..329ffcc0 100644 --- a/src/helpers/cache.ts +++ b/src/helpers/cache.ts @@ -1,4 +1,5 @@ import redis from '../redis'; +import { get, set } from '../aws'; const VP_KEY_PREFIX = 'vp'; @@ -38,3 +39,20 @@ export async function cachedVp>( return { result, cache: false }; } + +export async function cachedScores(key: string, callback: () => Type, toCache = false) { + if (!toCache || !!process.env.AWS_REGION) { + return { scores: await callback(), cache: false }; + } + + const cache = await get(key); + + if (cache) { + return { scores: cache as Awaited, cache: true }; + } + + const scores = await callback(); + set(key, scores); + + return { scores, cache: false }; +} diff --git a/src/scores.test.ts b/src/scores.test.ts index a850c031..e71302a1 100644 --- a/src/scores.test.ts +++ b/src/scores.test.ts @@ -1,10 +1,10 @@ import scores from './scores'; -import { get, set } from './aws'; +import { cachedScores } from './helpers/cache'; import { getCurrentBlockNum, sha256 } from './utils'; import snapshot from '@snapshot-labs/strategies'; jest.mock('./utils'); -jest.mock('./aws'); +jest.mock('./helpers/cache'); jest.mock('@snapshot-labs/strategies'); describe('scores function', () => { @@ -24,7 +24,7 @@ describe('scores function', () => { }); it('should deduplicate requests', async () => { - (snapshot.utils.getScoresDirect as jest.Mock).mockResolvedValue(mockScores); + (cachedScores as jest.Mock).mockResolvedValue({ scores: mockScores, cache: false }); const firstCall = scores(null, mockArgs); const secondCall = scores(null, mockArgs); @@ -32,12 +32,11 @@ describe('scores function', () => { const secondResult = await secondCall; expect(firstResult).toEqual(secondResult); - expect(snapshot.utils.getScoresDirect).toHaveBeenCalledTimes(1); + expect(cachedScores).toHaveBeenCalledTimes(1); }); it('should return cached results', async () => { - process.env.AWS_REGION = 'us-west-1'; - (get as jest.Mock).mockResolvedValue(mockScores); + (cachedScores as jest.Mock).mockResolvedValue({ scores: mockScores, cache: true }); const result = await scores(null, mockArgs); @@ -46,24 +45,12 @@ describe('scores function', () => { scores: mockScores, state: 'final' }); - expect(get).toHaveBeenCalledWith(key); - }); - - it('should set cache if not cached before', async () => { - process.env.AWS_REGION = 'us-west-1'; - (get as jest.Mock).mockResolvedValue(null); // Not in cache - (snapshot.utils.getScoresDirect as jest.Mock).mockResolvedValue(mockScores); - - await scores(null, mockArgs); - - expect(set).toHaveBeenCalledWith(key, mockScores); + expect(cachedScores).toHaveBeenCalledWith(key, expect.anything(), true); }); it('should return uncached results when cache is not needed', async () => { - process.env.AWS_REGION = 'us-west-1'; (getCurrentBlockNum as jest.Mock).mockResolvedValue('latest'); - (get as jest.Mock).mockResolvedValue(null); // Not in cache - (snapshot.utils.getScoresDirect as jest.Mock).mockResolvedValue(mockScores); + (cachedScores as jest.Mock).mockResolvedValue({ scores: mockScores, cache: false }); // Not in cache const result = await scores(null, { ...mockArgs, snapshot: 'latest' }); // "latest" should bypass cache expect(result).toEqual({ @@ -71,21 +58,7 @@ describe('scores function', () => { scores: mockScores, state: 'pending' }); - expect(set).not.toHaveBeenCalled(); - }); - - it("shouldn't return cached results when cache is not available", async () => { - process.env.AWS_REGION = ''; - (getCurrentBlockNum as jest.Mock).mockResolvedValue(mockArgs.snapshot); - (snapshot.utils.getScoresDirect as jest.Mock).mockResolvedValue(mockScores); - const result = await scores(null, mockArgs); - - expect(result).toEqual({ - cache: false, - scores: mockScores, - state: 'final' - }); - expect(get).not.toHaveBeenCalled(); + expect(cachedScores).toHaveBeenCalledWith(key, expect.anything(), false); }); it('should restrict block number by `latest`', async () => { diff --git a/src/scores.ts b/src/scores.ts index 66c55fb6..6d76e65e 100644 --- a/src/scores.ts +++ b/src/scores.ts @@ -1,12 +1,13 @@ import snapshot from '@snapshot-labs/strategies'; -import { get, set } from './aws'; import { getCurrentBlockNum, sha256 } from './utils'; import serve from './requestDeduplicator'; +import { cachedScores } from './helpers/cache'; const broviderUrl = process.env.BROVIDER_URL || 'https://rpc.snapshot.org'; +type ScoresResult = ReturnType; + async function calculateScores(parent, args, key) { - const withCache = !!process.env.AWS_REGION; const { space = '', strategies, network, addresses } = args; let snapshotBlockNum = 'latest'; @@ -16,32 +17,23 @@ async function calculateScores(parent, args, key) { } const state = snapshotBlockNum === 'latest' ? 'pending' : 'final'; - - let scores; - - if (withCache && state === 'final') scores = await get(key); - - let cache = true; - if (!scores) { - cache = false; - scores = await snapshot.utils.getScoresDirect( - space, - strategies, - network, - snapshot.utils.getProvider(network, { broviderUrl }), - addresses, - snapshotBlockNum - ); - - if (withCache && state === 'final') { - set(key, scores); - } - } + const results = await cachedScores( + key, + async () => + await snapshot.utils.getScoresDirect( + space, + strategies, + network, + snapshot.utils.getProvider(network, { broviderUrl }), + addresses, + snapshotBlockNum + ), + state === 'final' + ); return { state, - cache, - scores + ...results }; } From 26fbe60a17e7d80bff5691da5f75e07fd06812d0 Mon Sep 17 00:00:00 2001 From: Wan Qi Chen <495709+wa0x6e@users.noreply.github.com> Date: Mon, 30 Oct 2023 17:33:40 +0700 Subject: [PATCH 2/3] chore: fix tests --- src/scores.test.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/scores.test.ts b/src/scores.test.ts index e71302a1..9a48bff9 100644 --- a/src/scores.test.ts +++ b/src/scores.test.ts @@ -4,7 +4,12 @@ import { getCurrentBlockNum, sha256 } from './utils'; import snapshot from '@snapshot-labs/strategies'; jest.mock('./utils'); -jest.mock('./helpers/cache'); +jest.mock('./helpers/cache', () => { + return { + cachedScores: jest.fn(), + cachedVp: jest.fn() + }; +}); jest.mock('@snapshot-labs/strategies'); describe('scores function', () => { From b78d40d6ad8f88d42bf86d97128ddca9e6dee168 Mon Sep 17 00:00:00 2001 From: Wan <495709+wa0x6e@users.noreply.github.com> Date: Mon, 6 Nov 2023 15:13:26 +0800 Subject: [PATCH 3/3] chore: instrument the cache layer (#942) --- src/helpers/cache.ts | 9 +++++++++ src/metrics.ts | 6 ++++++ 2 files changed, 15 insertions(+) diff --git a/src/helpers/cache.ts b/src/helpers/cache.ts index 92d08eb1..961e87e9 100644 --- a/src/helpers/cache.ts +++ b/src/helpers/cache.ts @@ -1,5 +1,6 @@ import redis from '../redis'; import { get, set } from '../aws'; +import { cacheActivitesCount } from '../metrics'; export const VP_KEY_PREFIX = 'vp'; @@ -15,6 +16,7 @@ export async function cachedVp>( toCache = true ) { if (!toCache || !redis) { + cacheActivitesCount.inc({ type: 'vp', status: 'skip' }); return { result: await callback(), cache: false }; } @@ -24,12 +26,15 @@ export async function cachedVp>( cache.vp = parseFloat(cache.vp); cache.vp_by_strategy = JSON.parse(cache.vp_by_strategy); + cacheActivitesCount.inc({ type: 'vp', status: 'hit' }); return { result: cache as Awaited, cache: true }; } const result = await callback(); + let cacheHitStatus = 'unqualified'; if (result.vp_state === 'final') { + cacheHitStatus = 'miss'; const multi = redis.multi(); multi.hSet(`${VP_KEY_PREFIX}:${key}`, 'vp', result.vp); multi.hSet(`${VP_KEY_PREFIX}:${key}`, 'vp_by_strategy', JSON.stringify(result.vp_by_strategy)); @@ -37,22 +42,26 @@ export async function cachedVp>( multi.exec(); } + cacheActivitesCount.inc({ type: 'vp', status: cacheHitStatus }); return { result, cache: false }; } export async function cachedScores(key: string, callback: () => Type, toCache = false) { if (!toCache || !!process.env.AWS_REGION) { + cacheActivitesCount.inc({ type: 'scores', status: 'skip' }); return { scores: await callback(), cache: false }; } const cache = await get(key); if (cache) { + cacheActivitesCount.inc({ type: 'scores', status: 'hit' }); return { scores: cache as Awaited, cache: true }; } const scores = await callback(); set(key, scores); + cacheActivitesCount.inc({ type: 'scores', status: 'miss' }); return { scores, cache: false }; } diff --git a/src/metrics.ts b/src/metrics.ts index 10b0d90d..36ff3ef9 100644 --- a/src/metrics.ts +++ b/src/metrics.ts @@ -30,3 +30,9 @@ export const requestDeduplicatorSize = new client.Gauge({ name: 'request_deduplicator_size', help: 'Total number of items in the deduplicator queue' }); + +export const cacheActivitesCount = new client.Gauge({ + name: 'cache_activites_count', + help: 'Number of requests to the cache layer', + labelNames: ['type', 'status'] +});