Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 6 additions & 2 deletions src/api/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import {
} from "fastify-zod-openapi";
import { type ZodOpenApiVersion } from "zod-openapi";
import { withTags } from "./components/index.js";
import RedisModule from "ioredis";

/** BEGIN EXTERNAL PLUGINS */
import fastifyIp from "fastify-ip";
Expand Down Expand Up @@ -61,6 +60,7 @@ import mobileWalletV2Route from "./routes/v2/mobileWallet.js";
import membershipV2Plugin from "./routes/v2/membership.js";
import { docsHtml, securitySchemes } from "./docs.js";
import syncIdentityPlugin from "./routes/syncIdentity.js";
import { createRedisModule } from "./redis.js";
/** END ROUTES */

export const instanceId = randomUUID();
Expand Down Expand Up @@ -309,7 +309,11 @@ Otherwise, email [infra@acm.illinois.edu](mailto:infra@acm.illinois.edu) for sup
) as SecretConfig;
};
await app.refreshSecretConfig();
app.redisClient = new RedisModule.default(app.secretConfig.redis_url);
app.redisClient = await createRedisModule(
app.secretConfig.redis_url,
app.secretConfig.fallback_redis_url,
app.log,
);
}
if (isRunningInLambda) {
await app.register(fastifyIp.default, {
Expand Down
76 changes: 76 additions & 0 deletions src/api/redis.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
import RedisModule from "ioredis";
import { ValidLoggers } from "./types.js";

type RedisMode = "read-write" | "read-only" | "down";

async function checkRedisMode(url: string): Promise<RedisMode> {
let testModule: RedisModule.default | null = null;
try {
testModule = new RedisModule.default(url);

// Test connectivity
await testModule.ping();

// Test write capability
const testKey = `health-check:${Date.now()}`;
try {
await testModule.set(testKey, "test", "EX", 1);
return "read-write";
} catch (writeError) {
return "read-only";
}
} catch (error) {
return "down";
} finally {
if (testModule) {
await testModule.quit().catch(() => {});
}
}
}

export async function createRedisModule(
primaryUrl: string,
fallbackUrl: string,
logger: ValidLoggers,
) {
const primaryMode = await checkRedisMode(primaryUrl);
if (Math.random() < 0.01) {
// Upstash will complain if we never hit the instance
// Fire a hit every so often on the fallback
// Don't block on it
// Given we create a client once ever hour, we should hit the node at least once every 4 days at least
checkRedisMode(fallbackUrl);
}
if (primaryMode === "read-write") {
logger.info("Using primary Redis in read-write mode");
return new RedisModule.default(primaryUrl);
}

const fallbackMode = await checkRedisMode(fallbackUrl);

if (fallbackMode === "read-write") {
logger.warn(
`Primary Redis is ${primaryMode}, using fallback in read-write mode`,
);
return new RedisModule.default(fallbackUrl);
}

if (primaryMode === "read-only") {
logger.warn(
"Both Redis instances are read-only. Using primary in read-only mode",
);
return new RedisModule.default(primaryUrl);
}

if (fallbackMode === "read-only") {
logger.error(
"Primary Redis is down and Fallback Redis is down, using primary anyway",
);
return new RedisModule.default(primaryUrl);
}
Comment on lines +65 to +70
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Fix incorrect log message.

The log message states "Fallback Redis is down" but this branch is entered when fallbackMode === "read-only", not "down". This will mislead engineers during incidents when they're trying to understand the Redis failover state.

Apply this diff:

   if (fallbackMode === "read-only") {
     logger.error(
-      "Primary Redis is down and Fallback Redis is down, using primary anyway",
+      "Primary Redis is down and fallback is read-only; failing with primary to surface write failures immediately",
     );
     return new RedisModule.default(primaryUrl);
   }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (fallbackMode === "read-only") {
logger.error(
"Primary Redis is down and Fallback Redis is down, using primary anyway",
);
return new RedisModule.default(primaryUrl);
}
if (fallbackMode === "read-only") {
logger.error(
"Primary Redis is down and fallback is read-only; failing with primary to surface write failures immediately",
);
return new RedisModule.default(primaryUrl);
}
🤖 Prompt for AI Agents
In src/api/redis.ts around lines 65 to 70, the error message incorrectly says
"Fallback Redis is down" while the branch is taken when fallbackMode ===
"read-only"; update the log text to accurately reflect the state (e.g., "Primary
Redis is down and Fallback Redis is in read-only mode, using primary anyway") so
engineers are not misled during incidents; simply replace the logger.error
string with the corrected message and keep the existing return new
RedisModule.default(primaryUrl) behavior.


logger.error(
"Both primary and fallback Redis instances are down. Creating client on primary anyway.",
);
return new RedisModule.default(primaryUrl);
}
7 changes: 6 additions & 1 deletion src/api/sqs/handlers/createOrgGithubTeam.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import { Modules } from "common/modules.js";
import { retryDynamoTransactionWithBackoff } from "api/utils.js";
import { SKIP_EXTERNAL_ORG_LEAD_UPDATE } from "common/overrides.js";
import { getOrgByName } from "@acm-uiuc/js-shared";
import { createRedisModule } from "api/redis.js";

export const createOrgGithubTeamHandler: SQSHandlerFunction<
AvailableSQSFunctions.CreateOrgGithubTeam
Expand All @@ -28,7 +29,11 @@ export const createOrgGithubTeamHandler: SQSHandlerFunction<
logger,
commonConfig: { region: genericConfig.AwsRegion },
});
const redisClient = new RedisModule.default(secretConfig.redis_url);
const redisClient = await createRedisModule(
secretConfig.redis_url,
secretConfig.fallback_redis_url,
logger,
);
try {
const { orgName, githubTeamName, githubTeamDescription } = payload;
const orgImmutableId = getOrgByName(orgName)!.id;
Expand Down
7 changes: 6 additions & 1 deletion src/api/sqs/handlers/provisionNewMember.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { getAuthorizedClients, getSecretConfig } from "../utils.js";
import { emailMembershipPassHandler } from "./emailMembershipPassHandler.js";
import RedisModule from "ioredis";
import { setKey } from "api/functions/redisCache.js";
import { createRedisModule } from "api/redis.js";

export const provisionNewMemberHandler: SQSHandlerFunction<
AvailableSQSFunctions.ProvisionNewMember
Expand All @@ -30,7 +31,11 @@ export const provisionNewMemberHandler: SQSHandlerFunction<
logger,
commonConfig,
});
const redisClient = new RedisModule.default(secretConfig.redis_url);
const redisClient = await createRedisModule(
secretConfig.redis_url,
secretConfig.fallback_redis_url,
logger,
);
const netId = email.replace("@illinois.edu", "");
const cacheKey = `membership:${netId}:acmpaid`;
logger.info("Got authorized clients and Entra ID token.");
Expand Down
1 change: 1 addition & 0 deletions src/common/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,7 @@ export type SecretConfig = {
stripe_endpoint_secret: string;
stripe_links_endpoint_secret: string;
redis_url: string;
fallback_redis_url: string;
encryption_key: string;
UIN_HASHING_SECRET_PEPPER: string;
github_pat: string;
Expand Down
Loading