Skip to content

Commit dff6202

Browse files
authored
fix(cache): implement distributed lock for cache stampede prevention (JhaSourav07#2132)
## Description This PR introduces a structural architectural fix to prevent Distributed Cache Stampedes (Thundering Herd) when a highly requested user or organization dashboard cache expires. Prior to this PR, the DistributedCache had no cross-instance locking. If an L2 (Redis) cache expired during high traffic, every concurrent serverless instance would experience a cache miss and send identical GraphQL queries to GitHub at the same exact millisecond, easily triggering secondary rate limits and causing 500s. This PR solves the vulnerability by introducing a distributed Redis Mutex lock and formalizing a getOrSet pattern. **Key Changes** 1. Added getOrSet to DistributedCache: Encapsulates both L1 Promise-based deduplication and L2 Redis Mutex locking (SET lock:KEY 1 NX PX 10000). 2. Graceful Instance Polling: When an edge instance fails to acquire the mutex (meaning another instance is already fetching the data), it gracefully polls the Redis cache waiting for the data to populate rather than blindly spamming GitHub APIs. 3. Stale-While-Revalidate (SWR) Support: Upgraded getOrSet with an optional shouldFetch predicate. This enables fetchGitHubContributions to continue fetching "delta syncs" securely by retaining the 7-day stale cache while acquiring a lock. 4. Refactored lib/github.ts: Stripped out legacy, manual Map locks (pendingContributions, etc.) and fully delegated fetching logic to DistributedCache. Fixes JhaSourav07#1979 ## Pillar - [ ] 🎨 Pillar 1 — New Theme Design - [ ] 📐 Pillar 2 — Geometric SVG Improvement - [ ] 🕐 Pillar 3 — Timezone Logic Optimization - [x] 🛠️ Other (Bug fix, refactoring, docs) ## Visual Preview ## Checklist before requesting a review: - [x] I have read the `CONTRIBUTING.md` file. - [x] I have tested these changes locally (`localhost:3000/api/streak?user=YOUR_USERNAME`). - [x] I have run `npm run format` and `npm run lint` locally and resolved all errors (CI will fail otherwise). - [x] My commits follow the Conventional Commits format (e.g., `feat(themes): ...`, `fix(calculate): ...`). - [x] I have updated `README.md` if I added a new theme or URL parameter. - [x] I have started the repo. - [x] I have made sure that i have only one commit to merge in this PR. - [x] The SVG output matches the CommitPulse "premium quality" aesthetic standard (no raw elements, smooth animations, correct fonts). - [x] (Recommended) I joined the CommitPulse Discord community for contributor discussions, mentorship, and faster PR support.
2 parents 61f67f3 + 9ad4527 commit dff6202

3 files changed

Lines changed: 133 additions & 65 deletions

File tree

app/page.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
'use client';
22
import { trackUser } from '@/utils/tracking';
3+
34
import Link from 'next/link';
45
import { useRef, useState, useEffect } from 'react';
56
import { AnimatePresence, motion } from 'framer-motion';

lib/cache.ts

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,7 @@ export class DistributedCache<T> {
211211
private useRedis: boolean;
212212
private redisUrl: string = '';
213213
private redisToken: string = '';
214+
private localLocks = new Map<string, Promise<T>>();
214215

215216
constructor(maxSize?: number, cleanupIntervalMs?: number) {
216217
this.localCache = new TTLCache<T>(maxSize, cleanupIntervalMs);
@@ -369,4 +370,121 @@ export class DistributedCache<T> {
369370
destroy(): void {
370371
this.localCache.destroy();
371372
}
373+
374+
/**
375+
* Gets a value from the cache, or executes the load function if missing or stale.
376+
* Employs both an in-memory Promise lock (L1) and a Redis Mutex (L2) to prevent Cache Stampedes.
377+
*
378+
* @param key - Cache key.
379+
* @param loadFn - Async function to fetch the data. Receives the stale cached value if one exists.
380+
* @param ttlMs - Time to live in milliseconds.
381+
* @param shouldFetch - Optional predicate to force fetching even if a cache value exists (e.g. for stale delta sync).
382+
*/
383+
async getOrSet(
384+
key: string,
385+
loadFn: (cached: T | null) => Promise<T>,
386+
ttlMs: number,
387+
shouldFetch?: (cached: T) => boolean
388+
): Promise<T> {
389+
// 1. L1 & L2 Cache Check
390+
const cached = await this.get(key);
391+
392+
// If we have a cache hit and we don't need to force a refresh, return it early.
393+
if (cached !== null && (!shouldFetch || !shouldFetch(cached))) {
394+
return cached;
395+
}
396+
397+
// 2. L1 Promise Deduping (Local Lock)
398+
const pendingLocal = this.localLocks.get(key);
399+
if (pendingLocal) return pendingLocal;
400+
401+
const executeAndLock = async () => {
402+
if (!this.useRedis) {
403+
// Fallback: Local execution only
404+
const data = await loadFn(cached);
405+
await this.set(key, data, ttlMs);
406+
return data;
407+
}
408+
409+
const lockKey = `lock:${key}`;
410+
const maxPollTime = 8000; // Give up polling after 8 seconds to stay within serverless limits
411+
const pollInterval = 400;
412+
const start = Date.now();
413+
414+
while (Date.now() - start < maxPollTime) {
415+
try {
416+
// Attempt to acquire Redis Mutex
417+
const lockRes = await fetch(`${this.redisUrl}/`, {
418+
method: 'POST',
419+
headers: {
420+
Authorization: `Bearer ${this.redisToken}`,
421+
'Content-Type': 'application/json',
422+
},
423+
body: JSON.stringify(['SET', lockKey, '1', 'NX', 'PX', 10000]),
424+
});
425+
426+
if (lockRes.ok) {
427+
const lockData = await lockRes.json();
428+
if (lockData.result === 'OK') {
429+
// Lock acquired! Execute loadFn.
430+
try {
431+
const freshData = await loadFn(cached);
432+
await this.set(key, freshData, ttlMs);
433+
434+
// Release lock early
435+
await fetch(`${this.redisUrl}/`, {
436+
method: 'POST',
437+
headers: {
438+
Authorization: `Bearer ${this.redisToken}`,
439+
'Content-Type': 'application/json',
440+
},
441+
body: JSON.stringify(['DEL', lockKey]),
442+
}).catch(() => {});
443+
444+
return freshData;
445+
} catch (err) {
446+
// Release lock on error so others can retry
447+
await fetch(`${this.redisUrl}/`, {
448+
method: 'POST',
449+
headers: {
450+
Authorization: `Bearer ${this.redisToken}`,
451+
'Content-Type': 'application/json',
452+
},
453+
body: JSON.stringify(['DEL', lockKey]),
454+
}).catch(() => {});
455+
throw err;
456+
}
457+
}
458+
}
459+
} catch (err) {
460+
// Redis network error during locking. Fallback to direct execution.
461+
console.error(`[DistributedCache] Lock error for "${key}":`, err);
462+
const fallbackData = await loadFn(cached);
463+
await this.set(key, fallbackData, ttlMs);
464+
return fallbackData;
465+
}
466+
467+
// Lock not acquired. Wait and poll L2 cache.
468+
await new Promise((resolve) => setTimeout(resolve, pollInterval));
469+
const doubleCheck = await this.get(key);
470+
// If doubleCheck satisfies the condition, return it
471+
if (doubleCheck !== null && (!shouldFetch || !shouldFetch(doubleCheck))) {
472+
return doubleCheck;
473+
}
474+
}
475+
476+
// Timed out waiting for lock. Fallback to direct execution to avoid hanging the client.
477+
const finalFallback = await loadFn(cached);
478+
await this.set(key, finalFallback, ttlMs);
479+
return finalFallback;
480+
};
481+
482+
// Execute with local Promise lock
483+
const promise = executeAndLock().finally(() => {
484+
this.localLocks.delete(key);
485+
});
486+
this.localLocks.set(key, promise);
487+
488+
return promise;
489+
}
372490
}

lib/github.ts

Lines changed: 14 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -251,10 +251,6 @@ const contributionsCache = new DistributedCache<ExtendedContributionData>(1000);
251251
const profileCache = new DistributedCache<GitHubUserProfile>(1000);
252252
const reposCache = new DistributedCache<GitHubRepo[]>(500);
253253
const contributedReposCache = new DistributedCache<Record<string, unknown>[]>(500);
254-
const pendingContributions = new Map<string, Promise<ExtendedContributionData>>();
255-
const pendingProfiles = new Map<string, Promise<GitHubUserProfile>>();
256-
const pendingRepos = new Map<string, Promise<GitHubRepo[]>>();
257-
const pendingContributedRepos = new Map<string, Promise<Record<string, unknown>[]>>();
258254

259255
interface GitHubUserProfile {
260256
login: string;
@@ -300,25 +296,6 @@ export function clearGitHubApiCacheForTests(): void {
300296
profileCache.clear();
301297
reposCache.clear();
302298
contributedReposCache.clear();
303-
pendingContributions.clear();
304-
pendingProfiles.clear();
305-
pendingRepos.clear();
306-
pendingContributedRepos.clear();
307-
}
308-
309-
function dedupeRequest<T>(
310-
pendingRequests: Map<string, Promise<T>>,
311-
key: string,
312-
load: () => Promise<T>
313-
): Promise<T> {
314-
const pending = pendingRequests.get(key);
315-
if (pending) return pending;
316-
317-
const request = load().finally(() => {
318-
pendingRequests.delete(key);
319-
});
320-
pendingRequests.set(key, request);
321-
return request;
322299
}
323300

324301
function getGitHubToken(): string {
@@ -397,31 +374,21 @@ export async function fetchGitHubContributions(
397374
options: FetchOptions = {}
398375
): Promise<ExtendedContributionData> {
399376
const key = cacheKey('contributions', username, options.from, options.to);
377+
const LONG_CACHE_TTL = 7 * 24 * 60 * 60 * 1000;
400378

401-
if (options.bypassCache) {
402-
return fetchContributionsUncached(username, key, options, null);
403-
}
404-
405-
// Wrap the entire cache-check + fetch pipeline in dedupeRequest so that
406-
// concurrent calls for the same key share a single in-flight promise.
407-
// This closes the TOCTOU window where two requests both miss the cache
408-
// (during the async DistributedCache.get) and each fires its own API call.
409-
const load = async (): Promise<ExtendedContributionData> => {
410-
const cached = await contributionsCache.get(key);
411-
379+
const shouldFetch = (cached: ExtendedContributionData) => {
412380
const now = Date.now();
413-
const isStale = cached?.calendar.lastSyncedAt
381+
return cached?.calendar.lastSyncedAt
414382
? now - new Date(cached.calendar.lastSyncedAt).getTime() > GITHUB_CACHE_TTL_MS
415383
: true;
384+
};
416385

417-
if (cached && !isStale) {
418-
return cached;
419-
}
420-
386+
const load = async (cached: ExtendedContributionData | null) => {
421387
return fetchContributionsUncached(username, key, options, cached);
422388
};
423389

424-
return dedupeRequest(pendingContributions, key, load);
390+
if (options.bypassCache) return load(null);
391+
return contributionsCache.getOrSet(key, load, LONG_CACHE_TTL, shouldFetch);
425392
}
426393

427394
async function fetchContributionsUncached(
@@ -579,17 +546,12 @@ export async function fetchUserProfile(
579546
const key = cacheKey('profile', username);
580547
const encodedUsername = encodeURIComponent(username);
581548

582-
if (options.bypassCache) {
583-
return fetchProfileUncached(encodedUsername, key, options);
584-
}
585-
586-
const load = async (): Promise<GitHubUserProfile> => {
587-
const cached = await profileCache.get(key);
588-
if (cached) return cached;
549+
const load = async () => {
589550
return fetchProfileUncached(encodedUsername, key, options);
590551
};
591552

592-
return dedupeRequest(pendingProfiles, key, load);
553+
if (options.bypassCache) return load();
554+
return profileCache.getOrSet(key, load, GITHUB_CACHE_TTL_MS);
593555
}
594556

595557
async function fetchProfileUncached(
@@ -627,17 +589,12 @@ export async function fetchUserRepos(
627589
const key = cacheKey('repos', username);
628590
const encodedUsername = encodeURIComponent(username);
629591

630-
if (options.bypassCache) {
631-
return fetchReposUncached(encodedUsername, key, options);
632-
}
633-
634-
const load = async (): Promise<GitHubRepo[]> => {
635-
const cached = await reposCache.get(key);
636-
if (cached) return cached;
592+
const load = async () => {
637593
return fetchReposUncached(encodedUsername, key, options);
638594
};
639595

640-
return dedupeRequest(pendingRepos, key, load);
596+
if (options.bypassCache) return load();
597+
return reposCache.getOrSet(key, load, GITHUB_CACHE_TTL_MS);
641598
}
642599

643600
async function fetchReposUncached(
@@ -945,10 +902,6 @@ export async function fetchContributedRepos(
945902
options: FetchOptions = {}
946903
): Promise<Record<string, unknown>[]> {
947904
const key = cacheKey('repos:contributed', username);
948-
if (!options.bypassCache) {
949-
const cached = await contributedReposCache.get(key);
950-
if (cached) return cached;
951-
}
952905

953906
const load = async () => {
954907
const query = `
@@ -983,15 +936,11 @@ export async function fetchContributedRepos(
983936
if (!res.ok) return [];
984937
const data = await res.json();
985938
const result = data?.data?.user?.repositoriesContributedTo?.nodes || [];
986-
987-
if (!options.bypassCache) {
988-
await contributedReposCache.set(key, result, GITHUB_CACHE_TTL_MS);
989-
}
990939
return result;
991940
};
992941

993942
if (options.bypassCache) return load();
994-
return dedupeRequest(pendingContributedRepos, key, load);
943+
return contributedReposCache.getOrSet(key, load, GITHUB_CACHE_TTL_MS);
995944
}
996945

997946
export interface DeveloperScoreInput {

0 commit comments

Comments
 (0)