Skip to content

Add graceful Redis failure handling with aggressive timeouts #10

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
bmcdorman wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Add graceful Redis failure handling with aggressive timeouts #10

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a29acc4
Add graceful Redis failure handling with aggressive timeouts
bmcdorman Oct 9, 2025
1f0231f
Address review feedback: fix retry config and stale cache issues
bmcdorman Oct 9, 2025
79bc749
Trigger PR update
bmcdorman Oct 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 40 additions & 11 deletions src/RedisCache.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,36 +4,65 @@ import Redis, { RedisOptions } from 'ioredis';
import Selector from './model/Selector';

class RedisCache implements Cache {
private static DEFAULT_TTL = 60 * 60 * 24 * 7;
// 1 hour TTL to limit stale data window if cache invalidation fails during Redis issues
private static DEFAULT_TTL = 60 * 60;

private redis_: Redis;

constructor(options: RedisOptions) {
this.redis_ = new Redis(options);
this.redis_ = new Redis({
...options,
connectTimeout: 500, // 500ms to connect
commandTimeout: 200, // 200ms per command
// retryStrategy controls retries for initial connection and reconnection attempts
// maxRetriesPerRequest controls retries for individual commands (GET, SET, etc)
retryStrategy: (times) => {
// Stop reconnection attempts after 2 tries
if (times > 2) return null;
Copy link
Member

@navzam navzam Oct 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this check needed if maxRetriesPerRequest is 0?

Copy link
Member

@navzam navzam Oct 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this check needed if maxRetriesPerRequest is 0?

return 50; // Wait only 50ms between reconnection attempts
},
maxRetriesPerRequest: 0, // Don't retry commands - fail immediately
enableOfflineQueue: false, // Don't queue commands when disconnected
});

this.redis_.on('error', (err) => {
console.error('Redis error (app will continue without cache):', err.message);
});
}

private static key_ = ({ collection, id }: Selector): string => {
return `${collection}/${id}`;
};

async get(selector: Selector): Promise<object | null> {
const data = await this.redis_.get(RedisCache.key_(selector));
if (!data) return null;

return JSON.parse(data);
try {
const data = await this.redis_.get(RedisCache.key_(selector));
if (!data) return null;
return JSON.parse(data);
} catch (err) {
console.error('Redis GET failed, continuing without cache:', err);
return null;
}
}

async set(selector: Selector, value: object | null): Promise<void> {
if (!value) {
// Cache-aside pattern: Only populate cache on reads, not writes
// This prevents stale data if cache write fails but DB write succeeds
// Instead, we just invalidate the cache entry on writes
try {
await this.redis_.del(RedisCache.key_(selector));
return;
} catch (err) {
console.error('Redis cache invalidation failed, continuing without cache:', err);
// Best effort - if invalidation fails, TTL will eventually clear stale data
}

await this.redis_.setex(RedisCache.key_(selector), RedisCache.DEFAULT_TTL, JSON.stringify(value));
}

async remove(selector: Selector): Promise<void> {
await this.redis_.del(RedisCache.key_(selector));
try {
await this.redis_.del(RedisCache.key_(selector));
} catch (err) {
console.error('Redis DEL failed, continuing without cache:', err);
}
}
}

Expand Down
32 changes: 10 additions & 22 deletions src/db.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,16 +60,12 @@ class Db {

const cacheSelector: Selector = { ...selector, collection: fCollection };

try {
const cached = await this.cache_.get(cacheSelector);
const cached = await this.cache_.get(cacheSelector);

if (cached) return {
type: 'success',
value: cached as T,
};
} catch (e) {
console.error(e);
}
if (cached) return {
type: 'success',
value: cached as T,
};

const doc = await firestore
.collection(fCollection)
Expand All @@ -82,11 +78,12 @@ class Db {
message: `Document "${selector.id}" not found.`,
};

// Populate the cache with fresh data from Firestore (cache-aside pattern)
try {
// Update the cache
await this.cache_.set(cacheSelector, doc.data());
} catch (e) {
console.error(e);
console.error('Failed to populate cache after read:', e);
// Continue - cache is optional
}

return {
Expand Down Expand Up @@ -120,12 +117,7 @@ class Db {
await docRef.set(value, { mergeFields: keysToReplace });
} else {
await docRef.set(value);

try {
await this.cache_.set(cacheSelector, value);
} catch (e) {
console.error(e);
}
await this.cache_.set(cacheSelector, value);
}

return {
Expand All @@ -143,11 +135,7 @@ class Db {

const cacheSelector: Selector = { ...selector, collection: fCollection };

try {
await this.cache_.remove(cacheSelector);
} catch (e) {
console.error(e);
}
await this.cache_.remove(cacheSelector);

await firestore
.collection(fCollection)
Expand Down